On Tue, Aug 08, 2023 at 01:25:30PM -0400, Sweet Tea Dorminy wrote:
> blk_crypto_profile_init() calls lockdep_register_key(), which asserts
> that the provided memory is not a static object. Unfortunately,
> blk-crypto-fallback currently has a single static blk_crypto_profile,
> which means trying to use the fallback with lockdep explodes in
> blk_crypto_fallback_init().
>
> Fortunately it is simple enough to use a dynamically allocated profile
> for fallback, allowing the use of lockdep.
>
> Fixes: 488f6682c832e ("block: blk-crypto-fallback for Inline Encryption")
> Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@xxxxxxxxxx>
> ---
> block/blk-crypto-fallback.c | 27 +++++++++++++++------------
> 1 file changed, 15 insertions(+), 12 deletions(-)
Thanks for catching this! My bad for not running xfstests with the inlinecrypt
mount option recently. Can you use the correct Fixes tag:?
Fixes: 2fb48d88e77f ("blk-crypto: use dynamic lock class for blk_crypto_profile::lock")
Cc: stable@xxxxxxxxxxxxxxx
Also, when describing the problem please try to be more specific than
"explodes". I just get a WARN_ON. Beyond that, presumably it just makes
lockdep not track blk_crypto_fallback_profile.lock?
> @@ -534,29 +534,32 @@ static int blk_crypto_fallback_init(void)
> {
> int i;
> int err;
> - struct blk_crypto_profile *profile = &blk_crypto_fallback_profile;
>
> if (blk_crypto_fallback_inited)
> return 0;
>
> get_random_bytes(blank_key, BLK_CRYPTO_MAX_KEY_SIZE);
>
> + blk_crypto_fallback_profile =
> + kzalloc(sizeof(*blk_crypto_fallback_profile), GFP_KERNEL);
> +
Maybe add a comment:
/* Dynamic allocation is needed because of lockdep_register_key(). */
Also, kzalloc() should be checked for failure.
Can you consider folding the following into your patch?
diff --git a/block/blk-crypto-fallback.c b/block/blk-crypto-fallback.c
index de94e9bffec6d..0764668a78157 100644
--- a/block/blk-crypto-fallback.c
+++ b/block/blk-crypto-fallback.c
@@ -540,17 +540,21 @@ static int blk_crypto_fallback_init(void)
get_random_bytes(blank_key, BLK_CRYPTO_MAX_KEY_SIZE);
- blk_crypto_fallback_profile =
- kzalloc(sizeof(*blk_crypto_fallback_profile), GFP_KERNEL);
-
err = bioset_init(&crypto_bio_split, 64, 0, 0);
if (err)
goto out;
+ /* Dynamic allocation is needed because of lockdep_register_key(). */
+ blk_crypto_fallback_profile =
+ kzalloc(sizeof(*blk_crypto_fallback_profile), GFP_KERNEL);
+ if (!blk_crypto_fallback_profile) {
+ err = -ENOMEM;
+ goto fail_free_bioset;
+ }
+
err = blk_crypto_profile_init(blk_crypto_fallback_profile,
blk_crypto_num_keyslots);
if (err)
- goto fail_free_bioset;
+ goto fail_free_profile;
err = -ENOMEM;
blk_crypto_fallback_profile->ll_ops = blk_crypto_fallback_ll_ops;
@@ -601,6 +605,8 @@ static int blk_crypto_fallback_init(void)
destroy_workqueue(blk_crypto_wq);
fail_destroy_profile:
blk_crypto_profile_destroy(blk_crypto_fallback_profile);
+fail_free_profile:
+ kfree(blk_crypto_fallback_profile);
fail_free_bioset:
bioset_exit(&crypto_bio_split);
out:
