Skip to content

Commit a075bac

Browse files
committed
fsverity: don't drop pagecache at end of FS_IOC_ENABLE_VERITY
The full pagecache drop at the end of FS_IOC_ENABLE_VERITY is causing performance problems and is hindering adoption of fsverity. It was intended to solve a race condition where unverified pages might be left in the pagecache. But actually it doesn't solve it fully. Since the incomplete solution for this race condition has too much performance impact for it to be worth it, let's remove it for now. Fixes: 3fda4c6 ("fs-verity: implement FS_IOC_ENABLE_VERITY ioctl") Cc: [email protected] Reviewed-by: Victor Hsieh <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Eric Biggers <[email protected]>
1 parent f959325 commit a075bac

File tree

1 file changed

+13
-12
lines changed

1 file changed

+13
-12
lines changed

fs/verity/enable.c

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
#include "fsverity_private.h"
99

1010
#include <linux/mount.h>
11-
#include <linux/pagemap.h>
1211
#include <linux/sched/signal.h>
1312
#include <linux/uaccess.h>
1413

@@ -367,25 +366,27 @@ int fsverity_ioctl_enable(struct file *filp, const void __user *uarg)
367366
goto out_drop_write;
368367

369368
err = enable_verity(filp, &arg);
370-
if (err)
371-
goto out_allow_write_access;
372369

373370
/*
374-
* Some pages of the file may have been evicted from pagecache after
375-
* being used in the Merkle tree construction, then read into pagecache
376-
* again by another process reading from the file concurrently. Since
377-
* these pages didn't undergo verification against the file digest which
378-
* fs-verity now claims to be enforcing, we have to wipe the pagecache
379-
* to ensure that all future reads are verified.
371+
* We no longer drop the inode's pagecache after enabling verity. This
372+
* used to be done to try to avoid a race condition where pages could be
373+
* evicted after being used in the Merkle tree construction, then
374+
* re-instantiated by a concurrent read. Such pages are unverified, and
375+
* the backing storage could have filled them with different content, so
376+
* they shouldn't be used to fulfill reads once verity is enabled.
377+
*
378+
* But, dropping the pagecache has a big performance impact, and it
379+
* doesn't fully solve the race condition anyway. So for those reasons,
380+
* and also because this race condition isn't very important relatively
381+
* speaking (especially for small-ish files, where the chance of a page
382+
* being used, evicted, *and* re-instantiated all while enabling verity
383+
* is quite small), we no longer drop the inode's pagecache.
380384
*/
381-
filemap_write_and_wait(inode->i_mapping);
382-
invalidate_inode_pages2(inode->i_mapping);
383385

384386
/*
385387
* allow_write_access() is needed to pair with deny_write_access().
386388
* Regardless, the filesystem won't allow writing to verity files.
387389
*/
388-
out_allow_write_access:
389390
allow_write_access(filp);
390391
out_drop_write:
391392
mnt_drop_write_file(filp);

0 commit comments

Comments
 (0)