Skip to content

Commit 2b4eae9

Browse files
committed
fscrypt: don't evict dirty inodes after removing key
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: b1c0ec3 ("fscrypt: add FS_IOC_REMOVE_ENCRYPTION_KEY ioctl") Cc: <[email protected]> # v5.4+ Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Eric Biggers <[email protected]>
1 parent 98d54f8 commit 2b4eae9

File tree

1 file changed

+9
-0
lines changed

1 file changed

+9
-0
lines changed

fs/crypto/keysetup.c

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -538,6 +538,15 @@ int fscrypt_drop_inode(struct inode *inode)
538538
return 0;
539539
mk = ci->ci_master_key->payload.data[0];
540540

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

0 commit comments

Comments
 (0)