linux-ext4 - Re: xfstests-bld generic/231 failure on 4.18-rc4 and later

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Sun, 29 Jul 2018 13:21:29 -0400
From:   Eric Whitney <enwlinux@...il.com>
To:     "Theodore Y. Ts'o" <tytso@....edu>
Cc:     Eric Whitney <enwlinux@...il.com>, linux-ext4@...r.kernel.org
Subject: Re: xfstests-bld generic/231 failure on 4.18-rc4 and later

* Theodore Y. Ts'o <tytso@....edu>:
> On Thu, Jul 26, 2018 at 01:16:17PM -0400, Eric Whitney wrote:
> > As mentioned in today's ext4 concall, I've been seeing generic/231 failures
> > when running kvm-xfstests on recent 4.18 kernels, beginning with rc4 and
> > continuing through rc5 and rc6.  To get the failure, it's necessary to
> > run generic/230 and 231 in sequence.  Running generic/231 alone and
> > repeatedly does not reproduce the failure for me.
> > 
> > The failure bisects to a patch in 4.18-rc4:
> > (8844618d8aa7) ext4: only look at the bg_flags field if it is valid
> 
> Thanks, I found the problem with that patch.  Patch follows...
> 
> commit e21d00b2ed3f480224b3b2a620326cc5ac1ad5f2
> Author: Theodore Ts'o <tytso@....edu>
> Date:   Sun Jul 28 23:12:04 2018 -0400
> 
>     ext4: fix check to prevent initializing reserved inodes
>     
>     Commit 8844618d8aa7: "ext4: only look at the bg_flags field if it is
>     valid" will complain if block group zero does not have the
>     EXT4_BG_INODE_ZEROED flag set.  Unfortunately, this is not correct,
>     since a freshly created file system has this flag cleared.  It gets
>     almost immediately after the file system is mounted read-write --- but
>     the following somewhat unlikely sequence will end up triggering a
>     false positive report of a corrupted file system:
>     
>        mkfs.ext4 /dev/vdc
>        mount -o ro /dev/vdc /vdc
>        mount -o remount,rw /dev/vdc
>     
>     Instead, when initializing the inode table for block group zero, test
>     to make sure that itable_unused count is not too large, since that is
>     the case that will result in some or all of the reserved inodes
>     getting cleared.
>     
>     This fixes the failures reported by Eric Whiteney when running
>     generic/230 and generic/231 in the the nojournal test case.
>     
>     Fixes: 8844618d8aa7 ("ext4: only look at the bg_flags field if it is valid")
>     Reported-by: Eric Whitney <enwlinux@...il.com>
>     Signed-off-by: Theodore Ts'o <tytso@....edu>
> 
> diff --git a/fs/ext4/ialloc.c b/fs/ext4/ialloc.c
> index 9429242cf4f7..dffd21de2694 100644
> --- a/fs/ext4/ialloc.c
> +++ b/fs/ext4/ialloc.c
> @@ -1388,7 +1388,10 @@ int ext4_init_inode_table(struct super_block *sb, ext4_group_t group,
>  			    ext4_itable_unused_count(sb, gdp)),
>  			    sbi->s_inodes_per_block);
>  
> -	if ((used_blks < 0) || (used_blks > sbi->s_itb_per_group)) {
> +	if ((used_blks < 0) || (used_blks > sbi->s_itb_per_group) ||
> +	    ((group == 0) && ((EXT4_INODES_PER_GROUP(sb) -
> +			       ext4_itable_unused_count(sb, gdp)) <
> +			      EXT4_FIRST_INO(sb)))) {
>  		ext4_error(sb, "Something is wrong with group %u: "
>  			   "used itable blocks: %d; "
>  			   "itable unused count: %u",
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index 92de019c117d..d4a218ba626c 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -3162,14 +3162,8 @@ static ext4_group_t ext4_has_uninit_itable(struct super_block *sb)
>  		if (!gdp)
>  			continue;
>  
> -		if (gdp->bg_flags & cpu_to_le16(EXT4_BG_INODE_ZEROED))
> -			continue;
> -		if (group != 0)
> +		if (!(gdp->bg_flags & cpu_to_le16(EXT4_BG_INODE_ZEROED)))
>  			break;
> -		ext4_error(sb, "Inode table for bg 0 marked as "
> -			   "needing zeroing");
> -		if (sb_rdonly(sb))
> -			return ngroups;
>  	}
>  
>  	return group;

After applying that patch to 4.18-rc6, I'm no longer seeing failures with
kvm-xfstests in the nojournal test case.  Looks good.

Tested-by: Eric Whitney <enwlinux@...il.com>

Thanks!
Eric