diff mbox

[04/12] e2fsck: do not early terminate extra space check

Message ID 20170626134348.1240-4-tahsin@google.com
State Accepted, archived
Headers show

Commit Message

Tahsin Erdogan June 26, 2017, 1:43 p.m. UTC
When check_inode_extra_space() detects a problem with the value of
i_extra_isize, it adjusts it and then returns without further validation
of contents in the inode body. Change this so that it will proceed to
check inline extended attributes.

Signed-off-by: Tahsin Erdogan <tahsin@google.com>
---
 e2fsck/pass1.c | 1 -
 1 file changed, 1 deletion(-)

Comments

Andreas Dilger June 26, 2017, 9:23 p.m. UTC | #1
> On Jun 26, 2017, at 7:43 AM, Tahsin Erdogan <tahsin@google.com> wrote:
> 
> When check_inode_extra_space() detects a problem with the value of
> i_extra_isize, it adjusts it and then returns without further validation
> of contents in the inode body. Change this so that it will proceed to
> check inline extended attributes.
> 
> Signed-off-by: Tahsin Erdogan <tahsin@google.com>
> ---
> e2fsck/pass1.c | 1 -
> 1 file changed, 1 deletion(-)
> 
> diff --git a/e2fsck/pass1.c b/e2fsck/pass1.c
> index 32152f3ec926..1532fd2067f2 100644
> --- a/e2fsck/pass1.c
> +++ b/e2fsck/pass1.c
> @@ -582,7 +582,6 @@ static void check_inode_extra_space(e2fsck_t ctx, struct problem_context *pctx)
> 			inode->i_extra_isize = (inode->i_extra_isize + 3) & ~3;
> 		e2fsck_write_inode_full(ctx, pctx->ino, pctx->inode,
> 					EXT2_INODE_SIZE(sb), "pass1");
> -		return;
> 	}
> 
> 	/* check if there is no place for an EA header */

The problem is that if i_extra_isize is changed, then the EA magic and data
will no longer be aligned properly, so there isn't anything to check?


Cheers, Andreas
Tahsin Erdogan June 26, 2017, 11:57 p.m. UTC | #2
>> @@ -582,7 +582,6 @@ static void check_inode_extra_space(e2fsck_t ctx, struct problem_context *pctx)
>>                       inode->i_extra_isize = (inode->i_extra_isize + 3) & ~3;
>>               e2fsck_write_inode_full(ctx, pctx->ino, pctx->inode,
>>                                       EXT2_INODE_SIZE(sb), "pass1");
>> -             return;
>>       }
>>
>>       /* check if there is no place for an EA header */
>
> The problem is that if i_extra_isize is changed, then the EA magic and data
> will no longer be aligned properly, so there isn't anything to check?
>
In the rest of the function, inline extended attributes and
i_*time_extra fields are checked. If i_extra_size moves from its
original location, magic field won't be found and inline extended
attribute check will be skipped. But if it happens to be corrected to
its original location, then we should check the ea contents. Also, we
should check the time extra fields.

I don't really see a good reason to return early. Please let me know
if I missing something.
diff mbox

Patch

diff --git a/e2fsck/pass1.c b/e2fsck/pass1.c
index 32152f3ec926..1532fd2067f2 100644
--- a/e2fsck/pass1.c
+++ b/e2fsck/pass1.c
@@ -582,7 +582,6 @@  static void check_inode_extra_space(e2fsck_t ctx, struct problem_context *pctx)
 			inode->i_extra_isize = (inode->i_extra_isize + 3) & ~3;
 		e2fsck_write_inode_full(ctx, pctx->ino, pctx->inode,
 					EXT2_INODE_SIZE(sb), "pass1");
-		return;
 	}
 
 	/* check if there is no place for an EA header */