Re: [PATCH 8/9] blk-crypto: use on-stack skciphers for fallback en/decryption

[Eric Biggers <ebiggers@kernel.org>]

On Fri, Oct 31, 2025 at 10:34:38AM +0100, Christoph Hellwig wrote:
> Allocating a skcipher dynamically can deadlock or cause unexpected
> I/O failures when called from writeback context.  Sidestep the
> allocation by using on-stack skciphers, similar to what the non
> blk-crypto fscrypt already does.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

skcipher => skcipher request

> @@ -238,12 +223,12 @@ static void blk_crypto_dun_to_iv(const u64 dun[BLK_CRYPTO_DUN_ARRAY_SIZE],
>   * encryption, encrypts the input bio using crypto API and submits the bounce
>   * bio.
>   */
> -void blk_crypto_fallback_encrypt_bio(struct bio *src_bio)

The above comment needs to be updated too.  Maybe leave the static
function __blk_crypto_fallback_encrypt_bio() uncommented, and write an
updated comment for the global function
blk_crypto_fallback_encrypt_bio().  Maybe something like this:

/*
 * blk-crypto-fallback's encryption routine.  Encrypts the source bio's data
 * into a sequence of bounce bios (usually 1, but there can be multiple if there
 * are more than BIO_MAX_VECS pages).  Submits the bounce bios, then completes
 * the source bio once all the bounce bios are done.  This takes ownership of
 * the source bio, i.e. the caller mustn't continue with submission.
 */

But really that ought to go in the previous commit, not this one.

> +static void blk_crypto_fallback_decrypt_bio(struct work_struct *work)
> +{
> +	struct bio_fallback_crypt_ctx *f_ctx =
> +		container_of(work, struct bio_fallback_crypt_ctx, work);
> +	struct bio *bio = f_ctx->bio;
> +	struct bio_crypt_ctx *bc = &f_ctx->crypt_ctx;
> +	struct blk_crypto_keyslot *slot;
> +	blk_status_t status;
> +
> +	status = blk_crypto_get_keyslot(blk_crypto_fallback_profile,
> +					bc->bc_key, &slot);
> +	if (status == BLK_STS_OK) {
> +		status = __blk_crypto_fallback_decrypt_bio(f_ctx->bio,
> +				&f_ctx->crypt_ctx, f_ctx->crypt_iter,
> +				blk_crypto_fallback_tfm(slot));
> +		blk_crypto_put_keyslot(slot);
> +	}

This is referencing f_ctx->bio and f_ctx->crypt_ctx when they were
already loaded into local variables.  Either the local variables should
be used, or they should be removed and the fields always used.

- Eric