linux-kernel - Re: ext4 regression (was Re: [PATCH 4.19 000/105] 4.19.45-stable review)

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]

Date:   Wed, 22 May 2019 01:05:11 -0400
From:   "Theodore Ts'o" <tytso@....edu>
To:     Naresh Kamboju <naresh.kamboju@...aro.org>
Cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        open list <linux-kernel@...r.kernel.org>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Guenter Roeck <linux@...ck-us.net>,
        Shuah Khan <shuah@...nel.org>, patches@...nelci.org,
        Ben Hutchings <ben.hutchings@...ethink.co.uk>,
        lkft-triage@...ts.linaro.org,
        linux- stable <stable@...r.kernel.org>,
        linux-ext4@...r.kernel.org,
        Arthur Marsh <arthur.marsh@...ernode.on.net>,
        Richard Weinberger <richard.weinberger@...il.com>,
        ltp@...ts.linux.it, Jan Stancek <jstancek@...hat.com>
Subject: Re: ext4 regression (was Re: [PATCH 4.19 000/105] 4.19.45-stable
 review)

On Tue, May 21, 2019 at 11:27:21PM +0530, Naresh Kamboju wrote:
> Steps to reproduce is,
> running LTP three test cases in sequence on x86 device.
> # cd ltp/runtest
> # cat syscalls ( only three test case)
> open12 open12
> madvise06 madvise06
> poll02 poll02
> #
> 
> as Dan referring to,
> 
> LTP is run using '/opt/ltp/runltp -d /scratch -f syscalls', where the
> syscalls file has been replaced with three test case names, and
> /scratch is an ext4 SATA drive. /scratch is created using 'mkfs -t ext4
> /dev/disk/by-id/ata-TOSHIBA_MG03ACA100_37O9KGKWF' and mounted to
> /scratch.

I'm still having trouble reproducing the problem.  I've followed the
above exactly, and it doesn't trigger on my system.  I think I know
what is happening, but even given my theory, I'm still not able to
trigger it.  So, I'm not 100% sure this is the appropriate fix.  If
you can reproduce it, can you see if this patch, applied on top of the
Linus's tip, fixes the problem for you?

					- Ted

commit 3ad7621bfff343b16d59ed418f6d4420d4ec3e63
Author: Theodore Ts'o <tytso@....edu>
Date:   Tue May 21 17:01:01 2019 -0400

    ext4: don't perform block validity checks on the journal inode
    
    Since the journal inode is already checked when we added it to the
    block validity's system zone, if we check it again, we'll just trigger
    a failure.
    
    This was causing failures like this:
    
    [   53.897001] EXT4-fs error (device sda): ext4_find_extent:909: inode
    #8: comm jbd2/sda-8: pblk 121667583 bad header/extent: invalid extent entries - magic f30a, entries 8, max 340(340), depth 0(0)
    [   53.931430] jbd2_journal_bmap: journal block not found at offset 49 on sda-8
    [   53.938480] Aborting journal on device sda-8.
    
    ... but only if the system was under enough memory pressure that
    logical->physical mapping for the journal inode gets pushed out of the
    extent cache.  (This is why it wasn't noticed earlier.)
    
    Fixes: 345c0dbf3a30 ("ext4: protect journal inode's blocks using block_validity")
    Reported-by: Dan Rue <dan.rue@...aro.org>
    Signed-off-by: Theodore Ts'o <tytso@....edu>

diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index f2c62e2a0c98..d40ed940001e 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -518,10 +518,14 @@ __read_extent_tree_block(const char *function, unsigned int line,
 	}
 	if (buffer_verified(bh) && !(flags & EXT4_EX_FORCE_CACHE))
 		return bh;
-	err = __ext4_ext_check(function, line, inode,
-			       ext_block_hdr(bh), depth, pblk);
-	if (err)
-		goto errout;
+	if (!ext4_has_feature_journal(inode->i_sb) ||
+	    (inode->i_ino !=
+	     le32_to_cpu(EXT4_SB(inode->i_sb)->s_es->s_journal_inum))) {
+		err = __ext4_ext_check(function, line, inode,
+				       ext_block_hdr(bh), depth, pblk);
+		if (err)
+			goto errout;
+	}
 	set_buffer_verified(bh);
 	/*
 	 * If this is a leaf block, cache all of its entries