Lustre devel - Jan 2007 - [Bug 11608] e2fsck segfaults on EA corruption

Please don''t reply to lustre-devel. Instead, comment in Bugzilla by
using the following link:
https://bugzilla.lustre.org/show_bug.cgi?id=11608



Created an attachment (id=9412)
Please don''t reply to lustre-devel. Instead, comment in Bugzilla by
using the following link:
 --> (https://bugzilla.lustre.org/attachment.cgi?id=9412&action=view)
address ea segfault

Please don''t reply to lustre-devel. Instead, comment in Bugzilla by
using the following link:
https://bugzilla.lustre.org/show_bug.cgi?id=11608

           What    |Removed                     |Added
----------------------------------------------------------------------------
OtherBugsDependingO|                            |4536
              nThis|                            |


Blocks on LLNL tracker

Please don''t reply to lustre-devel. Instead, comment in Bugzilla by
using the following link:
https://bugzilla.lustre.org/show_bug.cgi?id=11608



(From update of attachment 9412)
While this seems like a good patch, there should in fact already be checking
for the coverage of the EAs  via region_create() and region_allocate().

The created region is given as fs->blocksize in size, and region_create()
should verify both that the EA is inside the block and that it doesn''t
overlap
any other EA.

Please don''t reply to lustre-devel. Instead, comment in Bugzilla by
using the following link:
https://bugzilla.lustre.org/show_bug.cgi?id=11608

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #9412|                            |review+
               Flag|                            |


(From update of attachment 9412)
>--- e2fsprogs/e2fsck/pass1.c	2007-01-24 11:10:49.000000000 -0800
>+++ e2fsprogs+chaos/e2fsck/pass1.c	2007-01-24 11:10:27.000000000 -0800
>@@ -1276,6 +1276,11 @@ static int check_ext_attr(e2fsck_t ctx, 
>+                if (entry->e_value_offs + entry->e_value_size >
fs->blocksize) {
>+                        if (fix_problem(ctx, PR_1_EA_BAD_VALUE, pctx))
>+                                goto clear_extattr;
>+			break;
>+                }
> 		if (entry->e_value_size &&
> 		    region_allocate(region, entry->e_value_offs,
> 				    EXT2_EXT_ATTR_SIZE(entry->e_value_size))) {
Ah, I see it now - the call to region_allocate() is only done if e_value_size
is non-zero, on the assumption that it only makes sense to allocate a region
that has some data in it.  The region_allocate() call itself will validate
start and length, but I also see that the "start" 

So, it seems we should either have an explicit check as in this patch or
allocate a zero-length region.	I think this patch is the most straight forward
approach to detecting this condition.

Girish, can you please add this to the CFS patch series (always before the
lfsck patch, as the other patches may be accepted upstream but the lfsck patch
will not be).