All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3] fscrypt: Fix missing barrier in fscrypt_find_mode_key()
@ 2026-10-23 16:07 Baokun Li
  2026-10-23 19:38 ` Eric Biggers
  0 siblings, 1 reply; 3+ messages in thread
From: Baokun Li @ 2026-10-23 16:07 UTC (permalink / raw)
  To: linux-fscrypt; +Cc: ebiggers, tytso, jaegeuk, stable

fscrypt_find_mode_key() walks an RCU list and hands out a per-mode
key without an acquire:

  setup task
  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  setup_per_mode_enc_key()
   ├─ fscrypt_prepare_key()
   │  └─ fscrypt_prepare_inline_crypt_key()
   │     └─ blk_crypto_start_using_key()
   │        └─ blk_crypto_fallback_start_using_mode()
   │           ├─ init profile, keyslots, tfms[]
   │           └─ smp_store_release(tfms_inited[mode])  # init order
   └─ list_add_tail_rcu(&new_node->link, ...)  # release -> finder

  finding task (later submits the I/O itself)
  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  fscrypt_find_mode_key()
   ├─ list_for_each_entry_rcu(node, ...)
   └─ return &node->key                 # no acquire, chain breaks
  ...
  blk_crypto_fallback_bio_prep()        # WRITE path
   ├─ tfms_inited[mode]                 # addr-dep, reads true
   ├─ blk_crypto_fallback_profile       # no dependency
   ├─ blk_crypto_keyslots               # no dependency
   └─ slotp->tfms[mode]                 # no dependency

RCU publishes the node and the key reachable from it, so
tfms_inited[mode] reads true through the address dependency.  The
independent fallback globals are not covered by it and can be read as
uninitialized.

On a weakly ordered machine, a WRITE bio can crash in
blk_crypto_get_keyslot() or crypto_sync_skcipher_setkey().

Other tasks reach the key through fscrypt_inode_info's
cmpxchg_release()/smp_load_acquire(), which is already ordered after
the list walk; only the finding task's own later I/O was not.
dm-inlinecrypt reaches its key only through srcu_dereference(md->map),
with no acquire anywhere on the I/O path.

Fix this by upgrading the control dependency to an acquire with
smp_acquire__after_ctrl_dep() once a matching entry has been found.

Fixes: dd015b566d50 ("fscrypt: Fix key setup in edge case with multiple data unit sizes")
Cc: stable@vger.kernel.org
Suggested-by: Eric Biggers <ebiggers@kernel.org>
Link: https://patch.msgid.link/20261014161153.GC322435@quark
Signed-off-by: Baokun Li <libaokun@linux.alibaba.com>
---
 fs/crypto/keysetup.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/fs/crypto/keysetup.c b/fs/crypto/keysetup.c
index 892044ebcaca..984ef43982a3 100644
--- a/fs/crypto/keysetup.c
+++ b/fs/crypto/keysetup.c
@@ -205,8 +205,14 @@ fscrypt_find_mode_key(struct fscrypt_master_key *mk, u8 hkdf_context,
 		if (node->hkdf_context == hkdf_context &&
 		    node->mode_num == mode_num &&
 		    node->data_unit_bits == ci->ci_data_unit_bits &&
-		    fscrypt_is_key_prepared(&node->key, ci))
+		    fscrypt_is_key_prepared(&node->key, ci)) {
+			/*
+			 * Pairs with list_add_tail_rcu() in
+			 * setup_per_mode_enc_key().
+			 */
+			smp_acquire__after_ctrl_dep();
 			return &node->key;
+		}
 	}
 	return NULL;
 }

base-commit: 734bef548a96e8774d6eef5dfa2026c6edc4e80b
-- 
2.43.7


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v3] fscrypt: Fix missing barrier in fscrypt_find_mode_key()
  2026-10-23 16:07 [PATCH v3] fscrypt: Fix missing barrier in fscrypt_find_mode_key() Baokun Li
@ 2026-10-23 19:38 ` Eric Biggers
  2026-10-24  9:53   ` Baokun Li
  0 siblings, 1 reply; 3+ messages in thread
From: Eric Biggers @ 2026-10-23 19:38 UTC (permalink / raw)
  To: Baokun Li; +Cc: linux-fscrypt, tytso, jaegeuk, stable

On Sat, Oct 24, 2026 at 12:07:47AM +0800, Baokun Li wrote:
> dm-inlinecrypt reaches its key only through srcu_dereference(md->map),
> with no acquire anywhere on the I/O path.

The above seems like a stray sentence that should be deleted.

> Suggested-by: Eric Biggers <ebiggers@kernel.org>
> Link: https://patch.msgid.link/20261014161153.GC322435@quark

Nit: lore.kernel.org instead of patch.msgid.link, since it links to a
discussion (in response to a patch) rather than directly to a patch.

> diff --git a/fs/crypto/keysetup.c b/fs/crypto/keysetup.c
> index 892044ebcaca..984ef43982a3 100644
> --- a/fs/crypto/keysetup.c
> +++ b/fs/crypto/keysetup.c
> @@ -205,8 +205,14 @@ fscrypt_find_mode_key(struct fscrypt_master_key *mk, u8 hkdf_context,
>  		if (node->hkdf_context == hkdf_context &&
>  		    node->mode_num == mode_num &&
>  		    node->data_unit_bits == ci->ci_data_unit_bits &&
> -		    fscrypt_is_key_prepared(&node->key, ci))
> +		    fscrypt_is_key_prepared(&node->key, ci)) {
> +			/*
> +			 * Pairs with list_add_tail_rcu() in
> +			 * setup_per_mode_enc_key().
> +			 */
> +			smp_acquire__after_ctrl_dep();
>  			return &node->key;
> +		}
>  	}

I think a better comment would be:

			/*
			 * Upgrade the RCU dereference to a full ACQUIRE, so
			 * that blk-crypto global state is seen as initialized.
			 */

That helps convey why the pre-existing RCU dereference isn't enough.

Other than that this patch looks good.  I can fix all the above when
applying, if you agree, or you can just resend.  Thanks!

- Eric

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v3] fscrypt: Fix missing barrier in fscrypt_find_mode_key()
  2026-10-23 19:38 ` Eric Biggers
@ 2026-10-24  9:53   ` Baokun Li
  0 siblings, 0 replies; 3+ messages in thread
From: Baokun Li @ 2026-10-24  9:53 UTC (permalink / raw)
  To: Eric Biggers; +Cc: linux-fscrypt, tytso, jaegeuk, stable

On 2026/10/24 03:38, Eric Biggers wrote:
> On Sat, Oct 24, 2026 at 12:07:47AM +0800, Baokun Li wrote:
>> dm-inlinecrypt reaches its key only through srcu_dereference(md->map),
>> with no acquire anywhere on the I/O path.
> 
> The above seems like a stray sentence that should be deleted.
> 

Okay.

>> Suggested-by: Eric Biggers <ebiggers@kernel.org>
>> Link: https://patch.msgid.link/20261014161153.GC322435@quark
> 
> Nit: lore.kernel.org instead of patch.msgid.link, since it links to a
> discussion (in response to a patch) rather than directly to a patch.
> 

That makes sense; I hadn't noticed that difference before.

>> diff --git a/fs/crypto/keysetup.c b/fs/crypto/keysetup.c
>> index 892044ebcaca..984ef43982a3 100644
>> --- a/fs/crypto/keysetup.c
>> +++ b/fs/crypto/keysetup.c
>> @@ -205,8 +205,14 @@ fscrypt_find_mode_key(struct fscrypt_master_key *mk, u8 hkdf_context,
>>  		if (node->hkdf_context == hkdf_context &&
>>  		    node->mode_num == mode_num &&
>>  		    node->data_unit_bits == ci->ci_data_unit_bits &&
>> -		    fscrypt_is_key_prepared(&node->key, ci))
>> +		    fscrypt_is_key_prepared(&node->key, ci)) {
>> +			/*
>> +			 * Pairs with list_add_tail_rcu() in
>> +			 * setup_per_mode_enc_key().
>> +			 */
>> +			smp_acquire__after_ctrl_dep();
>>  			return &node->key;
>> +		}
>>  	}
> 
> I think a better comment would be:
> 
> 			/*
> 			 * Upgrade the RCU dereference to a full ACQUIRE, so
> 			 * that blk-crypto global state is seen as initialized.
> 			 */
> 
> That helps convey why the pre-existing RCU dereference isn't enough.
> 

Yeah, that looks much clearer.

> Other than that this patch looks good.  I can fix all the above when
> applying, if you agree, or you can just resend.  Thanks!
>

Thank you for your review!

I'll send v2 right away.


Cheers,
Baokun


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-10-24  9:53 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-23 16:07 [PATCH v3] fscrypt: Fix missing barrier in fscrypt_find_mode_key() Baokun Li
2026-10-23 19:38 ` Eric Biggers
2026-10-24  9:53   ` Baokun Li

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.