linux-ext4 - Re: [e2fsprogs PATCH] tune2fs: don't recover journal if device is busy.

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20180224222352.GC14111@thunk.org>
Date:   Sat, 24 Feb 2018 17:23:52 -0500
From:   Theodore Ts'o <tytso@....edu>
To:     "Darrick J. Wong" <darrick.wong@...cle.com>
Cc:     NeilBrown <neilb@...e.com>, linux-ext4@...r.kernel.org,
        lustre <lustre-devel@...ts.lustre.org>
Subject: Re: [e2fsprogs PATCH] tune2fs: don't recover journal if device is
 busy.

On Sun, Feb 11, 2018 at 06:16:09PM -0800, Darrick J. Wong wrote:
> > Note: it seems wrong to recover the journal *after* making
> > changes to the superblock - there is a good chance that
> > recovering the journal will over-write those changes.
> > This is what was happening that lead me to this problem.
> > Shouldn't journal recovery happen *first*??
> 
> Yes.  Oops. :/
> 
> This whole hunk ought to move up to be right after
> ext2fs_check_if_mounted, I think.

After I did that, the test t_replay_and_set started failing.  The
problem is that the test deliberately corrupts all of the inode and
block bitmaps by writing bogus journal entries.  When tune2fs replays
the journal, it ends up smashing the inode and block bitmaps; but then
when it tries to rewrite checksums, the fact that inode bitmap is
completely zeroed out means all of the inode entries are also cleared
out.   Oops!

This normally isn't supposed to happen because check_fsck_needed() is
not supposed to allow us to do dangerous things that require rewriting
checksums unless the file system is freshly checked.

But the way the test was constructed the superblock's last mount time
is still 0, since the file system was never mounted.  By definition,
though, if there is a journal to be replayed, the file system has been
mounted, and hence must be checked.  Once fixed I this to force
s_lastcheck to be always less than s_mtimea after replaying the
journal, then the t_replay_and_set test would fail because it's not
safe to run "tune2fs -O ^metadata_csum" without running e2fsck first.

So I'll change the test to set the file system label instead doing
something more dangerous like clear the metadata checksum feature.
Was there someone who really wanted to be able to execute "tune2fs -O
^metadata_csum" on a file system with a dirty journal w/o running
e2fsck first?

						- Ted