[PATCH v2 1/2] blk-crypto: fix NULL deref and UAF in __blk_crypto_evict_key()

[PATCH v2 1/2] blk-crypto: fix NULL deref and UAF in __blk_crypto_evict_key()

[Baokun Li]

Sashiko reported a possible NULL pointer dereference, triggered by the
following flow:

  dm-inlinecrypt ctor            # first use of the fallback
   ├─ blk_crypto_init_key()      # succeeds, ctx->key.size != 0
   ├─ blk_crypto_start_using_key()
   │   ├─ hw-wrapped key: -EOPNOTSUPP
   │   │    # profile never allocated (NULL)
   │   └─ raw key: fallback_start_using_mode()
   │      └─ blk_crypto_fallback_init() fails
   │         └─ kfree(profile)   # not NULLed
   ├─ goto bad
   └─ inlinecrypt_dtr()          # evicts whenever key.size is set
      └─ blk_crypto_evict_key()
         └─ blk_crypto_config_supported_natively() = false
            └─ blk_crypto_fallback_evict_key()
               └─ __blk_crypto_evict_key(blk_crypto_fallback_profile)
                  └─ profile->num_slots # BOOM!!!

This issue can be reproduced using the following commands:

  dd if=/dev/zero of=/tmp/backing bs=1M count=64
  LOOP=$(losetup -f --show /tmp/backing)
  SZ=$(blockdev --getsz $LOOP)
  KEY=$(head -c 64 /dev/urandom | od -An -tx1 | tr -d ' \n')
  dmsetup create ic --table "0 $SZ inlinecrypt aes-xts-plain64 \
    $KEY 0 $LOOP 0 1 keytype:hw-wrapped"

And the following oops is triggered:

  ==================================================================
  blk-crypto: loop0: no support for wrapped keys
  Oops: general protection fault, probably for non-canonical address
        0xdffffc000000000b: 0000 [#1] SMP KASAN PTI
  KASAN: null-ptr-deref in range [0x0000000000000058-0x000000000000005f]
  CPU: 5 UID: 0 PID: 901 Comm: dmsetup 7.3.0-rc3 #1 PREEMPT(lazy)
  RIP: 0010:__blk_crypto_evict_key+0x2e/0x3b0
  Call Trace:
   blk_crypto_evict_key+0x24c/0x320
   inlinecrypt_dtr+0xf6/0x160
   inlinecrypt_ctr+0x2d3/0xbd0
   dm_table_add_target+0x661/0xf40
   table_load+0x1c4/0x490
   ctl_ioctl+0x32f/0x5f0
   dm_ctl_ioctl+0xe/0x20
  ==================================================================

dm-inlinecrypt is the only caller that can evict a key that was never
started, as fscrypt frees such a key without evicting it.

Fix this by publishing blk_crypto_fallback_profile only after
blk_crypto_fallback_init() succeeds, and by returning early from
__blk_crypto_evict_key() when the profile is NULL.  The eviction path
can then only observe either a NULL profile or a fully initialized
profile that is never freed.  A key whose start failed cannot be in a
keyslot, so there is nothing to evict.

The profile pointer is published and read locklessly, so use
smp_store_release()/smp_load_acquire() to guarantee that a reader in
the eviction path that observes the pointer also observes the
initialized profile.

The bug has been latent since commit c984ff1423ae ("blk-crypto:
dynamically allocate fallback profile") was merged in v6.5, and only
became actually triggerable when commit e7f57d2c47e2 ("dm-inlinecrypt:
add target for inline block device encryption") was merged in v7.2.

Fixes: c984ff1423ae ("blk-crypto: dynamically allocate fallback profile")
Cc: stable@vger.kernel.org
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://patch.msgid.link/sashiko-outbox-167341@kernel.org
Signed-off-by: Baokun Li <libaokun@linux.alibaba.com>
---
 block/blk-crypto-fallback.c | 31 +++++++++++++++++++------------
 block/blk-crypto-profile.c  |  3 +++
 2 files changed, 22 insertions(+), 12 deletions(-)

diff --git a/block/blk-crypto-fallback.c b/block/blk-crypto-fallback.c
index 2a8f40a65158..b5dbb092caea 100644
--- a/block/blk-crypto-fallback.c
+++ b/block/blk-crypto-fallback.c
@@ -524,12 +524,18 @@ bool blk_crypto_fallback_bio_prep(struct bio *bio)
 
 int blk_crypto_fallback_evict_key(const struct blk_crypto_key *key)
 {
-	return __blk_crypto_evict_key(blk_crypto_fallback_profile, key);
+	struct blk_crypto_profile *profile;
+
+	/* Pairs with smp_store_release() in blk_crypto_fallback_init(). */
+	profile = smp_load_acquire(&blk_crypto_fallback_profile);
+
+	return __blk_crypto_evict_key(profile, key);
 }
 
 static bool blk_crypto_fallback_inited;
 static int blk_crypto_fallback_init(void)
 {
+	struct blk_crypto_profile *profile;
 	int i;
 	int err;
 
@@ -543,26 +549,25 @@ static int blk_crypto_fallback_init(void)
 		goto out;
 
 	/* Dynamic allocation is needed because of lockdep_register_key(). */
-	blk_crypto_fallback_profile = kzalloc_obj(*blk_crypto_fallback_profile);
-	if (!blk_crypto_fallback_profile) {
+	profile = kzalloc_obj(*profile);
+	if (!profile) {
 		err = -ENOMEM;
 		goto fail_free_bioset;
 	}
 
-	err = blk_crypto_profile_init(blk_crypto_fallback_profile,
-				      blk_crypto_num_keyslots);
+	err = blk_crypto_profile_init(profile, blk_crypto_num_keyslots);
 	if (err)
 		goto fail_free_profile;
 	err = -ENOMEM;
 
-	blk_crypto_fallback_profile->ll_ops = blk_crypto_fallback_ll_ops;
-	blk_crypto_fallback_profile->max_dun_bytes_supported = BLK_CRYPTO_MAX_IV_SIZE;
-	blk_crypto_fallback_profile->key_types_supported = BLK_CRYPTO_KEY_TYPE_RAW;
+	profile->ll_ops = blk_crypto_fallback_ll_ops;
+	profile->max_dun_bytes_supported = BLK_CRYPTO_MAX_IV_SIZE;
+	profile->key_types_supported = BLK_CRYPTO_KEY_TYPE_RAW;
 
 	/* All blk-crypto modes have a crypto API fallback. */
 	for (i = 0; i < BLK_ENCRYPTION_MODE_MAX; i++)
-		blk_crypto_fallback_profile->modes_supported[i] = 0xFFFFFFFF;
-	blk_crypto_fallback_profile->modes_supported[BLK_ENCRYPTION_MODE_INVALID] = 0;
+		profile->modes_supported[i] = 0xFFFFFFFF;
+	profile->modes_supported[BLK_ENCRYPTION_MODE_INVALID] = 0;
 
 	blk_crypto_wq = alloc_workqueue("blk_crypto_wq",
 					WQ_UNBOUND | WQ_HIGHPRI |
@@ -590,6 +595,8 @@ static int blk_crypto_fallback_init(void)
 	if (!bio_fallback_crypt_ctx_pool)
 		goto fail_free_crypt_ctx_cache;
 
+	/* Pairs with smp_load_acquire() in blk_crypto_fallback_evict_key(). */
+	smp_store_release(&blk_crypto_fallback_profile, profile);
 	blk_crypto_fallback_inited = true;
 
 	return 0;
@@ -602,9 +609,9 @@ static int blk_crypto_fallback_init(void)
 fail_free_wq:
 	destroy_workqueue(blk_crypto_wq);
 fail_destroy_profile:
-	blk_crypto_profile_destroy(blk_crypto_fallback_profile);
+	blk_crypto_profile_destroy(profile);
 fail_free_profile:
-	kfree(blk_crypto_fallback_profile);
+	kfree(profile);
 fail_free_bioset:
 	bioset_exit(&enc_bio_set);
 out:
diff --git a/block/blk-crypto-profile.c b/block/blk-crypto-profile.c
index 53126c091b0b..1c5feca3b856 100644
--- a/block/blk-crypto-profile.c
+++ b/block/blk-crypto-profile.c
@@ -346,6 +346,9 @@ int __blk_crypto_evict_key(struct blk_crypto_profile *profile,
 	struct blk_crypto_keyslot *slot;
 	int err;
 
+	if (!profile)
+		return 0;
+
 	if (profile->num_slots == 0) {
 		if (profile->ll_ops.keyslot_evict) {
 			blk_crypto_hw_enter(profile);
-- 
2.43.7

[PATCH v2 2/2] blk-crypto: fix missing smp_load_acquire() for tfms_inited[]

[Baokun Li]

blk_crypto_fallback_start_using_mode() sets tfms_inited[] with
smp_store_release(), but blk_crypto_fallback_bio_prep() reads it
without smp_load_acquire() and without any lock protection.

Most callers are covered by blk-crypto itself: a task whose key needs
the fallback acquires the flag in the fast path of
blk_crypto_fallback_start_using_mode(), and its slow path is
serialized by tfms_init_lock.

But the problem can still be actually triggered: a task can inherit a
key without ever calling blk_crypto_start_using_key() itself, e.g. on
the lockless per-mode key cache hit in setup_per_mode_enc_key().  And
bios reach the dm target via srcu_dereference()/rcu_dereference(),
neither of which provides an acquire for tfms_inited[].

As a result, on a weakly ordered machine, the flag can be seen as
false and fail I/O, or as true while the tfms[] and fallback profile
initialization ordered before it is still invisible, and the reads
that follow can crash in the fallback I/O path.

Fix this by pairing the read with the release in
blk_crypto_fallback_start_using_mode() using smp_load_acquire(), and
hoist bc->bc_key->crypto_cfg into a local variable.

The plain read dates back to commit 488f6682c832 ("block:
blk-crypto-fallback for Inline Encryption") in v5.8, but was masked by
the smp_load_acquire() in fscrypt_is_key_prepared(); commit dd015b566d50
("fscrypt: Fix key setup in edge case with multiple data unit sizes")
in v7.2 replaced that with RCU publish/subscribe.

Fixes: 488f6682c832 ("block: blk-crypto-fallback for Inline Encryption")
Cc: stable@vger.kernel.org
Signed-off-by: Baokun Li <libaokun@linux.alibaba.com>
---
 block/blk-crypto-fallback.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/block/blk-crypto-fallback.c b/block/blk-crypto-fallback.c
index b5dbb092caea..31f76d117593 100644
--- a/block/blk-crypto-fallback.c
+++ b/block/blk-crypto-fallback.c
@@ -488,15 +488,17 @@ static void blk_crypto_fallback_decrypt_endio(struct bio *bio)
 bool blk_crypto_fallback_bio_prep(struct bio *bio)
 {
 	struct bio_crypt_ctx *bc = bio->bi_crypt_context;
+	const struct blk_crypto_config *cfg = &bc->bc_key->crypto_cfg;
 	struct bio_fallback_crypt_ctx *f_ctx;
 
-	if (WARN_ON_ONCE(!tfms_inited[bc->bc_key->crypto_cfg.crypto_mode])) {
+	/* Pairs with the release in blk_crypto_fallback_start_using_mode(). */
+	if (WARN_ON_ONCE(!smp_load_acquire(&tfms_inited[cfg->crypto_mode]))) {
 		/* User didn't call blk_crypto_start_using_key() first */
 		bio_io_error(bio);
 		return false;
 	}
 
-	if (bc->bc_key->crypto_cfg.key_type != BLK_CRYPTO_KEY_TYPE_RAW) {
+	if (cfg->key_type != BLK_CRYPTO_KEY_TYPE_RAW) {
 		bio_endio_status(bio, BLK_STS_NOTSUPP);
 		return false;
 	}
-- 
2.43.7

Re: [PATCH v2 1/2] blk-crypto: fix NULL deref and UAF in __blk_crypto_evict_key()

[Eric Biggers]

On Wed, Oct 14, 2026 at 03:18:45PM +0800, Baokun Li wrote:
> Sashiko reported a possible NULL pointer dereference, triggered by the
> following flow:
> 
>   dm-inlinecrypt ctor            # first use of the fallback
>    ├─ blk_crypto_init_key()      # succeeds, ctx->key.size != 0
>    ├─ blk_crypto_start_using_key()
>    │   ├─ hw-wrapped key: -EOPNOTSUPP
>    │   │    # profile never allocated (NULL)
>    │   └─ raw key: fallback_start_using_mode()
>    │      └─ blk_crypto_fallback_init() fails
>    │         └─ kfree(profile)   # not NULLed
>    ├─ goto bad
>    └─ inlinecrypt_dtr()          # evicts whenever key.size is set
>       └─ blk_crypto_evict_key()
>          └─ blk_crypto_config_supported_natively() = false
>             └─ blk_crypto_fallback_evict_key()
>                └─ __blk_crypto_evict_key(blk_crypto_fallback_profile)
>                   └─ profile->num_slots # BOOM!!!
> 
> This issue can be reproduced using the following commands:
> 
>   dd if=/dev/zero of=/tmp/backing bs=1M count=64
>   LOOP=$(losetup -f --show /tmp/backing)
>   SZ=$(blockdev --getsz $LOOP)
>   KEY=$(head -c 64 /dev/urandom | od -An -tx1 | tr -d ' \n')
>   dmsetup create ic --table "0 $SZ inlinecrypt aes-xts-plain64 \
>     $KEY 0 $LOOP 0 1 keytype:hw-wrapped"
> 
> And the following oops is triggered:
> 
>   ==================================================================
>   blk-crypto: loop0: no support for wrapped keys
>   Oops: general protection fault, probably for non-canonical address
>         0xdffffc000000000b: 0000 [#1] SMP KASAN PTI
>   KASAN: null-ptr-deref in range [0x0000000000000058-0x000000000000005f]
>   CPU: 5 UID: 0 PID: 901 Comm: dmsetup 7.3.0-rc3 #1 PREEMPT(lazy)
>   RIP: 0010:__blk_crypto_evict_key+0x2e/0x3b0
>   Call Trace:
>    blk_crypto_evict_key+0x24c/0x320
>    inlinecrypt_dtr+0xf6/0x160
>    inlinecrypt_ctr+0x2d3/0xbd0
>    dm_table_add_target+0x661/0xf40
>    table_load+0x1c4/0x490
>    ctl_ioctl+0x32f/0x5f0
>    dm_ctl_ioctl+0xe/0x20
>   ==================================================================
> 
> dm-inlinecrypt is the only caller that can evict a key that was never
> started, as fscrypt frees such a key without evicting it.
> 
> Fix this by publishing blk_crypto_fallback_profile only after
> blk_crypto_fallback_init() succeeds, and by returning early from
> __blk_crypto_evict_key() when the profile is NULL.  The eviction path
> can then only observe either a NULL profile or a fully initialized
> profile that is never freed.  A key whose start failed cannot be in a
> keyslot, so there is nothing to evict.
> 
> The profile pointer is published and read locklessly, so use
> smp_store_release()/smp_load_acquire() to guarantee that a reader in
> the eviction path that observes the pointer also observes the
> initialized profile.
> 
> The bug has been latent since commit c984ff1423ae ("blk-crypto:
> dynamically allocate fallback profile") was merged in v6.5, and only
> became actually triggerable when commit e7f57d2c47e2 ("dm-inlinecrypt:
> add target for inline block device encryption") was merged in v7.2.
> 
> Fixes: c984ff1423ae ("blk-crypto: dynamically allocate fallback profile")
> Cc: stable@vger.kernel.org
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Closes: https://patch.msgid.link/sashiko-outbox-167341@kernel.org
> Signed-off-by: Baokun Li <libaokun@linux.alibaba.com>
> ---
>  block/blk-crypto-fallback.c | 31 +++++++++++++++++++------------
>  block/blk-crypto-profile.c  |  3 +++
>  2 files changed, 22 insertions(+), 12 deletions(-)

Wouldn't it be a lot simpler to just make
blk_crypto_fallback_evict_key() check the existing tfms_inited flag?

	/*
	 * Pairs with smp_store_release() in
	 * blk_crypto_fallback_start_using_mode()
	 */
	if (!smp_load_acquire(&tfms_inited[key->crypto_cfg.crypto_mode]))
		return 0;

Re: [PATCH v2 2/2] blk-crypto: fix missing smp_load_acquire() for tfms_inited[]

[Eric Biggers]

On Wed, Oct 14, 2026 at 03:18:46PM +0800, Baokun Li wrote:
>
[trim LLM-generated text]

> diff --git a/block/blk-crypto-fallback.c b/block/blk-crypto-fallback.c
> index b5dbb092caea..31f76d117593 100644
> --- a/block/blk-crypto-fallback.c
> +++ b/block/blk-crypto-fallback.c
> @@ -488,15 +488,17 @@ static void blk_crypto_fallback_decrypt_endio(struct bio *bio)
>  bool blk_crypto_fallback_bio_prep(struct bio *bio)
>  {
>  	struct bio_crypt_ctx *bc = bio->bi_crypt_context;
> +	const struct blk_crypto_config *cfg = &bc->bc_key->crypto_cfg;
>  	struct bio_fallback_crypt_ctx *f_ctx;
>  
> -	if (WARN_ON_ONCE(!tfms_inited[bc->bc_key->crypto_cfg.crypto_mode])) {
> +	/* Pairs with the release in blk_crypto_fallback_start_using_mode(). */
> +	if (WARN_ON_ONCE(!smp_load_acquire(&tfms_inited[cfg->crypto_mode]))) {
>  		/* User didn't call blk_crypto_start_using_key() first */
>  		bio_io_error(bio);
>  		return false;
>  	}

No, anyone submitting encrypted I/O is required to ensure that
blk_crypto_start_using_key() was called first, in which case the
additional barrier here during I/O submission isn't needed.  If the
caller didn't ensure that, then it's a bug, and we WARN.  This patch is
inconsistent with that.

I think this can be considered a bug report that the ordering of
fscrypt_find_mode_key() is too weak now, as it just uses
list_for_each_entry_rcu().  It looks like it should do
smp_acquire__after_ctrl_dep() after it finds the right entry in the
list.

- Eric

Re: [PATCH v2 1/2] blk-crypto: fix NULL deref and UAF in __blk_crypto_evict_key()

[Baokun Li]

On 2026/10/14 23:51, Eric Biggers wrote:
> On Wed, Oct 14, 2026 at 03:18:45PM +0800, Baokun Li wrote:
>> Sashiko reported a possible NULL pointer dereference, triggered by the
>> following flow:
>>
>>   dm-inlinecrypt ctor            # first use of the fallback
>>    ├─ blk_crypto_init_key()      # succeeds, ctx->key.size != 0
>>    ├─ blk_crypto_start_using_key()
>>    │   ├─ hw-wrapped key: -EOPNOTSUPP
>>    │   │    # profile never allocated (NULL)
>>    │   └─ raw key: fallback_start_using_mode()
>>    │      └─ blk_crypto_fallback_init() fails
>>    │         └─ kfree(profile)   # not NULLed
>>    ├─ goto bad
>>    └─ inlinecrypt_dtr()          # evicts whenever key.size is set
>>       └─ blk_crypto_evict_key()
>>          └─ blk_crypto_config_supported_natively() = false
>>             └─ blk_crypto_fallback_evict_key()
>>                └─ __blk_crypto_evict_key(blk_crypto_fallback_profile)
>>                   └─ profile->num_slots # BOOM!!!
>>
>> This issue can be reproduced using the following commands:
>>
>>   dd if=/dev/zero of=/tmp/backing bs=1M count=64
>>   LOOP=$(losetup -f --show /tmp/backing)
>>   SZ=$(blockdev --getsz $LOOP)
>>   KEY=$(head -c 64 /dev/urandom | od -An -tx1 | tr -d ' \n')
>>   dmsetup create ic --table "0 $SZ inlinecrypt aes-xts-plain64 \
>>     $KEY 0 $LOOP 0 1 keytype:hw-wrapped"
>>
>> And the following oops is triggered:
>>
>>   ==================================================================
>>   blk-crypto: loop0: no support for wrapped keys
>>   Oops: general protection fault, probably for non-canonical address
>>         0xdffffc000000000b: 0000 [#1] SMP KASAN PTI
>>   KASAN: null-ptr-deref in range [0x0000000000000058-0x000000000000005f]
>>   CPU: 5 UID: 0 PID: 901 Comm: dmsetup 7.3.0-rc3 #1 PREEMPT(lazy)
>>   RIP: 0010:__blk_crypto_evict_key+0x2e/0x3b0
>>   Call Trace:
>>    blk_crypto_evict_key+0x24c/0x320
>>    inlinecrypt_dtr+0xf6/0x160
>>    inlinecrypt_ctr+0x2d3/0xbd0
>>    dm_table_add_target+0x661/0xf40
>>    table_load+0x1c4/0x490
>>    ctl_ioctl+0x32f/0x5f0
>>    dm_ctl_ioctl+0xe/0x20
>>   ==================================================================
>>
>> dm-inlinecrypt is the only caller that can evict a key that was never
>> started, as fscrypt frees such a key without evicting it.
>>
>> Fix this by publishing blk_crypto_fallback_profile only after
>> blk_crypto_fallback_init() succeeds, and by returning early from
>> __blk_crypto_evict_key() when the profile is NULL.  The eviction path
>> can then only observe either a NULL profile or a fully initialized
>> profile that is never freed.  A key whose start failed cannot be in a
>> keyslot, so there is nothing to evict.
>>
>> The profile pointer is published and read locklessly, so use
>> smp_store_release()/smp_load_acquire() to guarantee that a reader in
>> the eviction path that observes the pointer also observes the
>> initialized profile.
>>
>> The bug has been latent since commit c984ff1423ae ("blk-crypto:
>> dynamically allocate fallback profile") was merged in v6.5, and only
>> became actually triggerable when commit e7f57d2c47e2 ("dm-inlinecrypt:
>> add target for inline block device encryption") was merged in v7.2.
>>
>> Fixes: c984ff1423ae ("blk-crypto: dynamically allocate fallback profile")
>> Cc: stable@vger.kernel.org
>> Reported-by: Sashiko <sashiko-bot@kernel.org>
>> Closes: https://patch.msgid.link/sashiko-outbox-167341@kernel.org
>> Signed-off-by: Baokun Li <libaokun@linux.alibaba.com>
>> ---
>>  block/blk-crypto-fallback.c | 31 +++++++++++++++++++------------
>>  block/blk-crypto-profile.c  |  3 +++
>>  2 files changed, 22 insertions(+), 12 deletions(-)
> Wouldn't it be a lot simpler to just make
> blk_crypto_fallback_evict_key() check the existing tfms_inited flag?
>
> 	/*
> 	 * Pairs with smp_store_release() in
> 	 * blk_crypto_fallback_start_using_mode()
> 	 */
> 	if (!smp_load_acquire(&tfms_inited[key->crypto_cfg.crypto_mode]))
> 		return 0;

Indeed, this is a simpler and more straightforward approach.
I will use it in the next iteration.

Thank you for your suggestion!

Regards,
Baokun

Re: [PATCH v2 2/2] blk-crypto: fix missing smp_load_acquire() for tfms_inited[]

[Baokun Li]

Hi Eric,

Thank you for your feedback!

On 2026/10/15 00:11, Eric Biggers wrote:
> On Wed, Oct 14, 2026 at 03:18:46PM +0800, Baokun Li wrote:
> [trim LLM-generated text]
>
>> diff --git a/block/blk-crypto-fallback.c b/block/blk-crypto-fallback.c
>> index b5dbb092caea..31f76d117593 100644
>> --- a/block/blk-crypto-fallback.c
>> +++ b/block/blk-crypto-fallback.c
>> @@ -488,15 +488,17 @@ static void blk_crypto_fallback_decrypt_endio(struct bio *bio)
>>  bool blk_crypto_fallback_bio_prep(struct bio *bio)
>>  {
>>  	struct bio_crypt_ctx *bc = bio->bi_crypt_context;
>> +	const struct blk_crypto_config *cfg = &bc->bc_key->crypto_cfg;
>>  	struct bio_fallback_crypt_ctx *f_ctx;
>>  
>> -	if (WARN_ON_ONCE(!tfms_inited[bc->bc_key->crypto_cfg.crypto_mode])) {
>> +	/* Pairs with the release in blk_crypto_fallback_start_using_mode(). */
>> +	if (WARN_ON_ONCE(!smp_load_acquire(&tfms_inited[cfg->crypto_mode]))) {
>>  		/* User didn't call blk_crypto_start_using_key() first */
>>  		bio_io_error(bio);
>>  		return false;
>>  	}
> No, anyone submitting encrypted I/O is required to ensure that
> blk_crypto_start_using_key() was called first, in which case the
> additional barrier here during I/O submission isn't needed.  If the
> caller didn't ensure that, then it's a bug, and we WARN.  This patch is
> inconsistent with that.

You're right that the flag can't be observed as false here: the index
comes from bc->bc_key, so that load is address-dependent on the key
pointer, and a submitter that reached the key through a published
structure will in practice only ever see 'true'.  The WARN is fine.

The barrier wasn't meant to make the flag read reliable, though -- it
was for the loads *after* the if:

  if (!tfms_inited[mode])          <- addr-dep on bc->bc_key, ordered
          ...
  blk_crypto_fallback_profile      <- no dependency on anything
  blk_crypto_keyslots              <- no dependency on anything
  slotp->tfms[mode]                <- no dependency on anything

i.e. to make observing 'true' imply that those are initialized.  My
subject and commit message claimed the opposite, which is what made the
patch look inconsistent with the WARN next to it.  Sorry about that.

> I think this can be considered a bug report that the ordering of
> fscrypt_find_mode_key() is too weak now, as it just uses
> list_for_each_entry_rcu().  It looks like it should do
> smp_acquire__after_ctrl_dep() after it finds the right entry in the
> list.

That's what I was about to do for v3, but while looking for other
callers that inherit a key without calling start_using_key()
themselves, I found that dm-inlinecrypt has the same shape, so fixing
fscrypt alone wouldn't cover it -- only the constructor calls
start_using_key(), and the task submitting I/O reaches the key through
md->map:

  dmsetup (ctr)                      I/O task
  ---------------------------------- -------------------------------
  blk_crypto_start_using_key()
   fallback_start_using_mode()
    init profile, keyslots, tfms[]
    smp_store_release(tfms_inited[m])
  rcu_assign_pointer(md->map, t)
                                     srcu_dereference(md->map)
                                      -> ti->private -> &ctx->key
                                     inlinecrypt_map() -> bio_prep()
                                       tfms_inited[m]  addr-dep, ok
                                       profile/keyslots/tfms[]  not

dm's own grace periods would cover this, but all three are conditional:

  drivers/md/dm.c:2533       if (old_map) dm_sync_table(md);
  drivers/md/dm.c:3015,3039  if (map) synchronize_srcu(&md->io_barrier);
  drivers/md/dm-ioctl.c:1651 add_disk() only when type == DM_TYPE_NONE

so e.g. this sequence has no happens-before:

  DM_DEV_CREATE
  DM_TABLE_LOAD (linear)       add_disk(), md->type set, not resumed
                                 -> a task opens the device here
  DM_TABLE_CLEAR
  DM_TABLE_LOAD (inlinecrypt)  ctr sets tfms_inited[]; no add_disk()
  DM_DEV_SUSPEND (resume)      old_map == NULL, all three skipped
                                 -> the already-open task does I/O

This needs a weakly ordered machine and a deliberate ioctl order, so
it's not urgent.  Which would you prefer?

  1. Leave it for now and fix only fscrypt_find_mode_key().

  2. A separate smp_rmb() in inlinecrypt_map(), paired with the
     rcu_assign_pointer(md->map) in __bind().  Keeps the cost on
     dm-inlinecrypt only, but means a second site to get right.

  3. Keep the barrier in blk_crypto_fallback_bio_prep() after all,
     documented as ordering the loads after the check rather than the
     flag itself.  One site covers both callers and any future one,
     at the cost of blk-crypto no longer relying on the caller for
     this part.


Thanks,
Baokun

Re: [PATCH v2 2/2] blk-crypto: fix missing smp_load_acquire() for tfms_inited[]

[Eric Biggers]

On Fri, Oct 16, 2026 at 01:36:17AM +0800, Baokun Li wrote:
> That's what I was about to do for v3, but while looking for other
> callers that inherit a key without calling start_using_key()
> themselves, I found that dm-inlinecrypt has the same shape, so fixing
> fscrypt alone wouldn't cover it -- only the constructor calls
> start_using_key(), and the task submitting I/O reaches the key through
> md->map:
> 
>   dmsetup (ctr)                      I/O task
>   ---------------------------------- -------------------------------
>   blk_crypto_start_using_key()
>    fallback_start_using_mode()
>     init profile, keyslots, tfms[]
>     smp_store_release(tfms_inited[m])
>   rcu_assign_pointer(md->map, t)
>                                      srcu_dereference(md->map)
>                                       -> ti->private -> &ctx->key
>                                      inlinecrypt_map() -> bio_prep()
>                                        tfms_inited[m]  addr-dep, ok
>                                        profile/keyslots/tfms[]  not
> 
> dm's own grace periods would cover this, but all three are conditional:
> 
>   drivers/md/dm.c:2533       if (old_map) dm_sync_table(md);
>   drivers/md/dm.c:3015,3039  if (map) synchronize_srcu(&md->io_barrier);
>   drivers/md/dm-ioctl.c:1651 add_disk() only when type == DM_TYPE_NONE
> 
> so e.g. this sequence has no happens-before:
> 
>   DM_DEV_CREATE
>   DM_TABLE_LOAD (linear)       add_disk(), md->type set, not resumed
>                                  -> a task opens the device here
>   DM_TABLE_CLEAR
>   DM_TABLE_LOAD (inlinecrypt)  ctr sets tfms_inited[]; no add_disk()
>   DM_DEV_SUSPEND (resume)      old_map == NULL, all three skipped
>                                  -> the already-open task does I/O
> 
> This needs a weakly ordered machine and a deliberate ioctl order, so
> it's not urgent.  Which would you prefer?
> 
>   1. Leave it for now and fix only fscrypt_find_mode_key().
> 
>   2. A separate smp_rmb() in inlinecrypt_map(), paired with the
>      rcu_assign_pointer(md->map) in __bind().  Keeps the cost on
>      dm-inlinecrypt only, but means a second site to get right.
> 
>   3. Keep the barrier in blk_crypto_fallback_bio_prep() after all,
>      documented as ordering the loads after the check rather than the
>      flag itself.  One site covers both callers and any future one,
>      at the cost of blk-crypto no longer relying on the caller for
>      this part.

So the device-mapper core doesn't order target_type::ctr before
target_type::map?  Nearly every dm target's map function relies on the
constructor having run first.  If this is an issue, it isn't specific to
dm-inlinecrypt and it will need to be fixed in the device-mapper core.

- Eric