Skip to content

Commit 97c6327

Browse files
committed
fscrypt: use smp_load_acquire() for fscrypt_prepared_key
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. fscrypt_prepared_key includes a pointer to a crypto_skcipher object, which is internal to and is allocated by the crypto subsystem. By using READ_ONCE() for it, we're relying on internal implementation details of the crypto subsystem. Remove this fragile assumption by using smp_load_acquire() instead. (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: 5fee360 ("fscrypt: add inline encryption support") Cc: Satya Tangirala <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Eric Biggers <[email protected]>
1 parent bd0d97b commit 97c6327

File tree

3 files changed

+17
-10
lines changed

3 files changed

+17
-10
lines changed

fs/crypto/fscrypt_private.h

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -351,13 +351,16 @@ fscrypt_is_key_prepared(struct fscrypt_prepared_key *prep_key,
351351
const struct fscrypt_info *ci)
352352
{
353353
/*
354-
* The READ_ONCE() here pairs with the smp_store_release() in
355-
* fscrypt_prepare_key(). (This only matters for the per-mode keys,
356-
* which are shared by multiple inodes.)
354+
* The two smp_load_acquire()'s here pair with the smp_store_release()'s
355+
* in fscrypt_prepare_inline_crypt_key() and fscrypt_prepare_key().
356+
* I.e., in some cases (namely, if this prep_key is a per-mode
357+
* encryption key) another task can publish blk_key or tfm concurrently,
358+
* executing a RELEASE barrier. We need to use smp_load_acquire() here
359+
* to safely ACQUIRE the memory the other task published.
357360
*/
358361
if (fscrypt_using_inline_encryption(ci))
359-
return READ_ONCE(prep_key->blk_key) != NULL;
360-
return READ_ONCE(prep_key->tfm) != NULL;
362+
return smp_load_acquire(&prep_key->blk_key) != NULL;
363+
return smp_load_acquire(&prep_key->tfm) != NULL;
361364
}
362365

363366
#else /* CONFIG_FS_ENCRYPTION_INLINE_CRYPT */
@@ -391,7 +394,7 @@ static inline bool
391394
fscrypt_is_key_prepared(struct fscrypt_prepared_key *prep_key,
392395
const struct fscrypt_info *ci)
393396
{
394-
return READ_ONCE(prep_key->tfm) != NULL;
397+
return smp_load_acquire(&prep_key->tfm) != NULL;
395398
}
396399
#endif /* !CONFIG_FS_ENCRYPTION_INLINE_CRYPT */
397400

fs/crypto/inline_crypt.c

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -176,8 +176,10 @@ int fscrypt_prepare_inline_crypt_key(struct fscrypt_prepared_key *prep_key,
176176
}
177177
}
178178
/*
179-
* Pairs with READ_ONCE() in fscrypt_is_key_prepared(). (Only matters
180-
* for the per-mode keys, which are shared by multiple inodes.)
179+
* Pairs with the smp_load_acquire() in fscrypt_is_key_prepared().
180+
* I.e., here we publish ->blk_key with a RELEASE barrier so that
181+
* concurrent tasks can ACQUIRE it. Note that this concurrency is only
182+
* possible for per-mode keys, not for per-file keys.
181183
*/
182184
smp_store_release(&prep_key->blk_key, blk_key);
183185
return 0;

fs/crypto/keysetup.c

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -129,8 +129,10 @@ int fscrypt_prepare_key(struct fscrypt_prepared_key *prep_key,
129129
if (IS_ERR(tfm))
130130
return PTR_ERR(tfm);
131131
/*
132-
* Pairs with READ_ONCE() in fscrypt_is_key_prepared(). (Only matters
133-
* for the per-mode keys, which are shared by multiple inodes.)
132+
* Pairs with the smp_load_acquire() in fscrypt_is_key_prepared().
133+
* I.e., here we publish ->tfm with a RELEASE barrier so that
134+
* concurrent tasks can ACQUIRE it. Note that this concurrency is only
135+
* possible for per-mode keys, not for per-file keys.
134136
*/
135137
smp_store_release(&prep_key->tfm, tfm);
136138
return 0;

0 commit comments

Comments
 (0)