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: [email protected]
Reported-by: Sashiko <[email protected]>
Closes: https://patch.msgid.link/[email protected]
Signed-off-by: Baokun Li <[email protected]>
---
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
