Skip to content

Commit

Permalink
BACKPORT: fsverity: don't drop pagecache at end of FS_IOC_ENABLE_VERITY
Browse files Browse the repository at this point in the history
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: stable@vger.kernel.org
Reviewed-by: Victor Hsieh <victorhsieh@google.com>
Link: https://lore.kernel.org/r/20230314235332.50270-1-ebiggers@kernel.org
Signed-off-by: Eric Biggers <ebiggers@google.com>

Bug: 273320626
(cherry picked from commit a075bacde257f755bea0e53400c9f1cdd1b8e8e6)
Change-Id: I28dacf122bba5ac816f9b748dcbaa82dc1072fed
Signed-off-by: Eric Biggers <ebiggers@google.com>
  • Loading branch information
ebiggers authored and roberto-sartori-gl committed Mar 18, 2024
1 parent 955e6ff commit 1d155fc
Showing 1 changed file with 13 additions and 12 deletions.
25 changes: 13 additions & 12 deletions fs/verity/enable.c
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
#include <crypto/hash.h>
#include <linux/backing-dev.h>
#include <linux/mount.h>
#include <linux/pagemap.h>
#include <linux/sched/signal.h>
#include <linux/uaccess.h>

Expand Down Expand Up @@ -389,25 +388,27 @@ int fsverity_ioctl_enable(struct file *filp, const void __user *uarg)
goto out_drop_write;

err = enable_verity(filp, &arg);
if (err)
goto out_allow_write_access;

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

/*
* allow_write_access() is needed to pair with deny_write_access().
* Regardless, the filesystem won't allow writing to verity files.
*/
out_allow_write_access:
allow_write_access(filp);
out_drop_write:
mnt_drop_write_file(filp);
Expand Down

0 comments on commit 1d155fc

Please sign in to comment.