All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 1/2] blk-crypto: fix NULL deref and UAF in __blk_crypto_evict_key()
@ 2026-10-14  7:18 Baokun Li
  2026-10-14  7:18 ` [PATCH v2 2/2] blk-crypto: fix missing smp_load_acquire() for tfms_inited[] Baokun Li
  2026-10-14 15:51 ` [PATCH v2 1/2] blk-crypto: fix NULL deref and UAF in __blk_crypto_evict_key() Eric Biggers
  0 siblings, 2 replies; 4+ messages in thread
From: Baokun Li @ 2026-10-14  7:18 UTC (permalink / raw)
  To: axboe, linux-block
  Cc: ebiggers, linux-fscrypt, hch, dm-devel, mpatocka, sweettea-kernel,
	satyat, libaokun, stable, Sashiko

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


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

* [PATCH v2 2/2] blk-crypto: fix missing smp_load_acquire() for tfms_inited[]
  2026-10-14  7:18 [PATCH v2 1/2] blk-crypto: fix NULL deref and UAF in __blk_crypto_evict_key() Baokun Li
@ 2026-10-14  7:18 ` Baokun Li
  2026-10-14 16:11   ` Eric Biggers
  2026-10-14 15:51 ` [PATCH v2 1/2] blk-crypto: fix NULL deref and UAF in __blk_crypto_evict_key() Eric Biggers
  1 sibling, 1 reply; 4+ messages in thread
From: Baokun Li @ 2026-10-14  7:18 UTC (permalink / raw)
  To: axboe, linux-block
  Cc: ebiggers, linux-fscrypt, hch, dm-devel, mpatocka, sweettea-kernel,
	satyat, libaokun, stable

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


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

* Re: [PATCH v2 1/2] blk-crypto: fix NULL deref and UAF in __blk_crypto_evict_key()
  2026-10-14  7:18 [PATCH v2 1/2] blk-crypto: fix NULL deref and UAF in __blk_crypto_evict_key() Baokun Li
  2026-10-14  7:18 ` [PATCH v2 2/2] blk-crypto: fix missing smp_load_acquire() for tfms_inited[] Baokun Li
@ 2026-10-14 15:51 ` Eric Biggers
  1 sibling, 0 replies; 4+ messages in thread
From: Eric Biggers @ 2026-10-14 15:51 UTC (permalink / raw)
  To: Baokun Li
  Cc: axboe, linux-block, linux-fscrypt, hch, dm-devel, mpatocka,
	sweettea-kernel, satyat, stable, Sashiko

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;

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

* Re: [PATCH v2 2/2] blk-crypto: fix missing smp_load_acquire() for tfms_inited[]
  2026-10-14  7:18 ` [PATCH v2 2/2] blk-crypto: fix missing smp_load_acquire() for tfms_inited[] Baokun Li
@ 2026-10-14 16:11   ` Eric Biggers
  0 siblings, 0 replies; 4+ messages in thread
From: Eric Biggers @ 2026-10-14 16:11 UTC (permalink / raw)
  To: Baokun Li
  Cc: axboe, linux-block, linux-fscrypt, hch, dm-devel, mpatocka,
	sweettea-kernel, satyat, stable

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

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

end of thread, other threads:[~2026-10-14 16:12 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-14  7:18 [PATCH v2 1/2] blk-crypto: fix NULL deref and UAF in __blk_crypto_evict_key() Baokun Li
2026-10-14  7:18 ` [PATCH v2 2/2] blk-crypto: fix missing smp_load_acquire() for tfms_inited[] Baokun Li
2026-10-14 16:11   ` Eric Biggers
2026-10-14 15:51 ` [PATCH v2 1/2] blk-crypto: fix NULL deref and UAF in __blk_crypto_evict_key() Eric Biggers

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.