From 5e4a5dc82033d5b47e98b285906b366a864bb8a5 Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Fri, 9 Jun 2023 23:11:39 -0700 Subject: [PATCH] BACKPORT: blk-crypto: use dynamic lock class for blk_crypto_profile::lock When a device-mapper device is passing through the inline encryption support of an underlying device, calls to blk_crypto_evict_key() take the blk_crypto_profile::lock of the device-mapper device, then take the blk_crypto_profile::lock of the underlying device (nested). This isn't a real deadlock, but it causes a lockdep report because there is only one lock class for all instances of this lock. Lockdep subclasses don't really work here because the hierarchy of block devices is dynamic and could have more than 2 levels. Instead, register a dynamic lock class for each blk_crypto_profile, and associate that with the lock. This avoids false-positive lockdep reports like the following: ============================================ WARNING: possible recursive locking detected 6.4.0-rc5 #2 Not tainted -------------------------------------------- fscryptctl/1421 is trying to acquire lock: ffffff80829ca418 (&profile->lock){++++}-{3:3}, at: __blk_crypto_evict_key+0x44/0x1c0 but task is already holding lock: ffffff8086b68ca8 (&profile->lock){++++}-{3:3}, at: __blk_crypto_evict_key+0xc8/0x1c0 other info that might help us debug this: Possible unsafe locking scenario: CPU0 ---- lock(&profile->lock); lock(&profile->lock); *** DEADLOCK *** May be due to missing lock nesting notation Fixes: 1b2628397058 ("block: Keyslot Manager for Inline Encryption") Reported-by: Bart Van Assche Signed-off-by: Eric Biggers Reviewed-by: Bart Van Assche Link: https://lore.kernel.org/r/20230610061139.212085-1-ebiggers@kernel.org Signed-off-by: Jens Axboe Bug: 286427075 (cherry picked from commit 2fb48d88e77f29bf9d278f25bcfe82cf59a0e09b) (added '#ifdef CONFIG_LOCKDEP' to keep the KMI tooling happy) Change-Id: I21c0f941a36663c956a5c89324813bbaac0633ef Signed-off-by: Eric Biggers --- block/blk-crypto-profile.c | 16 +++++++++++++++- include/linux/blk-crypto-profile.h | 3 +++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/block/blk-crypto-profile.c b/block/blk-crypto-profile.c index fe550725c777..7cdef1bee6f7 100644 --- a/block/blk-crypto-profile.c +++ b/block/blk-crypto-profile.c @@ -79,7 +79,18 @@ int blk_crypto_profile_init(struct blk_crypto_profile *profile, unsigned int slot_hashtable_size; memset(profile, 0, sizeof(*profile)); + + /* + * profile->lock of an underlying device can nest inside profile->lock + * of a device-mapper device, so use a dynamic lock class to avoid + * false-positive lockdep reports. + */ +#ifdef CONFIG_LOCKDEP + lockdep_register_key(&profile->lockdep_key); + __init_rwsem(&profile->lock, "&profile->lock", &profile->lockdep_key); +#else init_rwsem(&profile->lock); +#endif if (num_slots == 0) return 0; @@ -89,7 +100,7 @@ int blk_crypto_profile_init(struct blk_crypto_profile *profile, profile->slots = kvcalloc(num_slots, sizeof(profile->slots[0]), GFP_KERNEL); if (!profile->slots) - return -ENOMEM; + goto err_destroy; profile->num_slots = num_slots; @@ -443,6 +454,9 @@ void blk_crypto_profile_destroy(struct blk_crypto_profile *profile) { if (!profile) return; +#ifdef CONFIG_LOCKDEP + lockdep_unregister_key(&profile->lockdep_key); +#endif kvfree(profile->slot_hashtable); kvfree_sensitive(profile->slots, sizeof(profile->slots[0]) * profile->num_slots); diff --git a/include/linux/blk-crypto-profile.h b/include/linux/blk-crypto-profile.h index 8b30d04ef008..794f608a8994 100644 --- a/include/linux/blk-crypto-profile.h +++ b/include/linux/blk-crypto-profile.h @@ -131,6 +131,9 @@ struct blk_crypto_profile { * keyslots while ensuring that they can't be changed concurrently. */ struct rw_semaphore lock; +#ifdef CONFIG_LOCKDEP + struct lock_class_key lockdep_key; +#endif /* List of idle slots, with least recently used slot at front */ wait_queue_head_t idle_slots_wait_queue;