linux-ext4 - Re: [PATCH 3/7] ext4: shutdown should not prevent get_write

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: <20180322153811.GG2852@thunk.org>
Date:   Thu, 22 Mar 2018 11:38:12 -0400
From:   "Theodore Y. Ts'o" <tytso@....edu>
To:     Jan Kara <jack@...e.cz>
Cc:     Ext4 Developers List <linux-ext4@...r.kernel.org>,
        stable@...r.kernel.org
Subject: Re: [PATCH 3/7] ext4: shutdown should not prevent get_write_access

On Wed, Mar 07, 2018 at 10:42:01AM +0100, Jan Kara wrote:
> On Mon 19-02-18 21:30:34, Theodore Ts'o wrote:
> > The ext4 forced shutdown flag needs to prevent new handles from being
> > started, but it needs to allow existing handles to complete.  So the
> > forced shutdown flag should not force ext4_journal_get_write_access to
> > fail.
> > 
> > Signed-off-by: Theodore Ts'o <tytso@....edu>
> > Cc: stable@...r.kernel.org
> 
> OK, if you want the semantics of ext4 shutdown to be that running
> handles should be allowed to complete, I see where you are going with this
> patch. However there are more problems with this semantics than just
> __ext4_journal_get_write_access(). Just for example
> ext4_reserve_inode_write() will bail in case the fs got shutdown and thus
> inode changes won't be properly added to the running handle. Also places
> that rely on nested transactions being possible will not work because
> ext4_journal_start_sb() will refuse to get refcount of a running handle
> (ext4_journal_check_start() fails) in case fs got shutdown. And I may have
> missed other cases.

We have three cases in ext4_shutdown:

EXT4_GOING_FLAGS_DEFUALT:
    Freezes the block device, sets the EXT4_FLAGS_SHUTDOWN flag, and then
    unfreezes the block device:

EXT4_GOING_FLAGS_LOGFLUSH:
    Sets the EXT4_FLAGS_SHUTDOWN flag, forces a commit, and then aborts
    the journal.

EXT4_GOING_FLAGS_NOLOGFLUSH
    Sets the EXT4_FLAGS_SHUTDOWN flag, aborts the journal.

> The above problems are a reason why I though the semantics of ext4 shutdown
> was terminate the fs *now* - effectively a software equivalent of power
> off. That is much easier to implement since we just have to make sure no
> running handle makes it to the journal... Since I've said Google is using
> ext4 shutdown - is there any reason why you need the "running handles are
> allowed to finish" semantics? After all it seems it's just a race whether
> some handle makes it before the cut off or not...

Good points.  I'll have to look at what happens if we just drop the
EXT4_FLAGS_SHUTDOWN flag altogether.

							- Ted