Re: [PATCH] blk-crypto: dynamically allocate fallback profile

[Eric Biggers <ebiggers@kernel.org>]

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@dorminy.me>
> ---
>  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@vger.kernel.org

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: