Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

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 <bvanassche@acm.org>
Signed-off-by: Eric Biggers <ebiggers@google.com>
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
Link: https://lore.kernel.org/r/20230610061139.212085-1-ebiggers@kernel.org
Signed-off-by: Jens Axboe <axboe@kernel.dk>

authored by

Eric Biggers and committed by
Jens Axboe
2fb48d88 7eb1e476

+11 -2
+10 -2
block/blk-crypto-profile.c
··· 79 79 unsigned int slot_hashtable_size; 80 80 81 81 memset(profile, 0, sizeof(*profile)); 82 - init_rwsem(&profile->lock); 82 + 83 + /* 84 + * profile->lock of an underlying device can nest inside profile->lock 85 + * of a device-mapper device, so use a dynamic lock class to avoid 86 + * false-positive lockdep reports. 87 + */ 88 + lockdep_register_key(&profile->lockdep_key); 89 + __init_rwsem(&profile->lock, "&profile->lock", &profile->lockdep_key); 83 90 84 91 if (num_slots == 0) 85 92 return 0; ··· 96 89 profile->slots = kvcalloc(num_slots, sizeof(profile->slots[0]), 97 90 GFP_KERNEL); 98 91 if (!profile->slots) 99 - return -ENOMEM; 92 + goto err_destroy; 100 93 101 94 profile->num_slots = num_slots; 102 95 ··· 442 435 { 443 436 if (!profile) 444 437 return; 438 + lockdep_unregister_key(&profile->lockdep_key); 445 439 kvfree(profile->slot_hashtable); 446 440 kvfree_sensitive(profile->slots, 447 441 sizeof(profile->slots[0]) * profile->num_slots);
+1
include/linux/blk-crypto-profile.h
··· 111 111 * keyslots while ensuring that they can't be changed concurrently. 112 112 */ 113 113 struct rw_semaphore lock; 114 + struct lock_class_key lockdep_key; 114 115 115 116 /* List of idle slots, with least recently used slot at front */ 116 117 wait_queue_head_t idle_slots_wait_queue;