lkml.org 
[lkml]   [2020]   [Mar]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 5.5 109/151] fscrypt: dont evict dirty inodes after removing key
    Date
    From: Eric Biggers <ebiggers@google.com>

    commit 2b4eae95c7361e0a147b838715c8baa1380a428f upstream.

    After FS_IOC_REMOVE_ENCRYPTION_KEY removes a key, it syncs the
    filesystem and tries to get and put all inodes that were unlocked by the
    key so that unused inodes get evicted via fscrypt_drop_inode().
    Normally, the inodes are all clean due to the sync.

    However, after the filesystem is sync'ed, userspace can modify and close
    one of the files. (Userspace is *supposed* to close the files before
    removing the key. But it doesn't always happen, and the kernel can't
    assume it.) This causes the inode to be dirtied and have i_count == 0.
    Then, fscrypt_drop_inode() failed to consider this case and indicated
    that the inode can be dropped, causing the write to be lost.

    On f2fs, other problems such as a filesystem freeze could occur due to
    the inode being freed while still on f2fs's dirty inode list.

    Fix this bug by making fscrypt_drop_inode() only drop clean inodes.

    I've written an xfstest which detects this bug on ext4, f2fs, and ubifs.

    Fixes: b1c0ec3599f4 ("fscrypt: add FS_IOC_REMOVE_ENCRYPTION_KEY ioctl")
    Cc: <stable@vger.kernel.org> # v5.4+
    Link: https://lore.kernel.org/r/20200305084138.653498-1-ebiggers@kernel.org
    Signed-off-by: Eric Biggers <ebiggers@google.com>
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

    ---
    fs/crypto/keysetup.c | 9 +++++++++
    1 file changed, 9 insertions(+)

    --- a/fs/crypto/keysetup.c
    +++ b/fs/crypto/keysetup.c
    @@ -515,6 +515,15 @@ int fscrypt_drop_inode(struct inode *ino
    mk = ci->ci_master_key->payload.data[0];

    /*
    + * With proper, non-racy use of FS_IOC_REMOVE_ENCRYPTION_KEY, all inodes
    + * protected by the key were cleaned by sync_filesystem(). But if
    + * userspace is still using the files, inodes can be dirtied between
    + * then and now. We mustn't lose any writes, so skip dirty inodes here.
    + */
    + if (inode->i_state & I_DIRTY_ALL)
    + return 0;
    +
    + /*
    * Note: since we aren't holding ->mk_secret_sem, the result here can
    * immediately become outdated. But there's no correctness problem with
    * unnecessarily evicting. Nor is there a correctness problem with not

    \
     
     \ /
      Last update: 2020-03-17 12:16    [W:4.215 / U:0.032 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site