summaryrefslogtreecommitdiffstats
path: root/fs/verity
diff options
context:
space:
mode:
authorEric Biggers <ebiggers@google.com>2020-07-22 00:59:20 +0200
committerEric Biggers <ebiggers@google.com>2020-07-22 01:02:41 +0200
commitf3db0bed458314a835ccef5ccb130270c5b2cf04 (patch)
tree0a9b6eacb0dea29a4a18bf24ae56e29a67d3de08 /fs/verity
parentLinux 5.8-rc6 (diff)
downloadlinux-f3db0bed458314a835ccef5ccb130270c5b2cf04.tar.xz
linux-f3db0bed458314a835ccef5ccb130270c5b2cf04.zip
fs-verity: use smp_load_acquire() for ->i_verity_info
Normally smp_store_release() or cmpxchg_release() is paired with smp_load_acquire(). Sometimes smp_load_acquire() can be replaced with the more lightweight READ_ONCE(). However, for this to be safe, all the published memory must only be accessed in a way that involves the pointer itself. This may not be the case if allocating the object also involves initializing a static or global variable, for example. fsverity_info::tree_params.hash_alg->tfm is a crypto_ahash object that's internal to and is allocated by the crypto subsystem. So by using READ_ONCE() for ->i_verity_info, we're relying on internal implementation details of the crypto subsystem. Remove this fragile assumption by using smp_load_acquire() instead. Also fix the cmpxchg logic to correctly execute an ACQUIRE barrier when losing the cmpxchg race, since cmpxchg doesn't guarantee a memory barrier on failure. (Note: I haven't seen any real-world problems here. This change is just fixing the code to be guaranteed correct and less fragile.) Fixes: fd2d1acfcadf ("fs-verity: add the hook for file ->open()") Link: https://lore.kernel.org/r/20200721225920.114347-6-ebiggers@kernel.org Signed-off-by: Eric Biggers <ebiggers@google.com>
Diffstat (limited to 'fs/verity')
-rw-r--r--fs/verity/open.c15
1 files changed, 12 insertions, 3 deletions
diff --git a/fs/verity/open.c b/fs/verity/open.c
index d007db0c9304..bfe0280c14e4 100644
--- a/fs/verity/open.c
+++ b/fs/verity/open.c
@@ -221,11 +221,20 @@ out:
void fsverity_set_info(struct inode *inode, struct fsverity_info *vi)
{
/*
- * Multiple processes may race to set ->i_verity_info, so use cmpxchg.
- * This pairs with the READ_ONCE() in fsverity_get_info().
+ * Multiple tasks may race to set ->i_verity_info, so use
+ * cmpxchg_release(). This pairs with the smp_load_acquire() in
+ * fsverity_get_info(). I.e., here we publish ->i_verity_info with a
+ * RELEASE barrier so that other tasks can ACQUIRE it.
*/
- if (cmpxchg(&inode->i_verity_info, NULL, vi) != NULL)
+ if (cmpxchg_release(&inode->i_verity_info, NULL, vi) != NULL) {
+ /* Lost the race, so free the fsverity_info we allocated. */
fsverity_free_info(vi);
+ /*
+ * Afterwards, the caller may access ->i_verity_info directly,
+ * so make sure to ACQUIRE the winning fsverity_info.
+ */
+ (void)fsverity_get_info(inode);
+ }
}
void fsverity_free_info(struct fsverity_info *vi)