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: Windows password security audit tool. GUI, reports in PDF.

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

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

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;