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 <[email protected]>
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
