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

Eric Biggers
2025-11-07T04:20:40Z

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>

It might be worth leaving a note in the commit message that this also
drops the incomplete support for asynchronous algorithms. ("Incomplete"
in the sense that they could be used, but only synchronously.)

Also note that with asynchronous algorithms no being longer supported,
the code can actually be simplified further because the
DECLARE_CRYPTO_WAIT(wait) objects are no longer necessary. The sequence
of crypto_sync_skcipher calls that is used should be similar to what
fscrypt_crypt_data_unit() does.

That could be done in a separate patch if you want, though.

> +void blk_crypto_fallback_encrypt_bio(struct bio *src_bio)
> +{
> + struct bio_crypt_ctx *bc = src_bio->bi_crypt_context;
> + struct blk_crypto_keyslot *slot;
> + blk_status_t status;
> +
> + if (!blk_crypto_fallback_bio_valid(src_bio))
> + return;
> +
> + status = blk_crypto_get_keyslot(blk_crypto_fallback_profile,
> + bc->bc_key, &slot);
> + if (status == BLK_STS_OK) {
> + status = __blk_crypto_fallback_encrypt_bio(src_bio,
> + blk_crypto_fallback_tfm(slot));
> + blk_crypto_put_keyslot(slot);
> + }
> + if (status != BLK_STS_OK) {
> + src_bio->bi_status = status;
> + bio_endio(src_bio);
> + return;
> + }

Unnecessary return statement above.

> /*
> * The crypto API fallback's main decryption routine.
> * Decrypts input bio in place, and calls bio_endio on the bio.
> */
> -static void blk_crypto_fallback_decrypt_bio(struct work_struct *work)
> +static blk_status_t __blk_crypto_fallback_decrypt_bio(struct bio *bio,
> + struct bio_crypt_ctx *bc, struct bvec_iter iter,
> + struct crypto_sync_skcipher *tfm)

Comment above needs to be updated, since this function no longer calls
bio_endio().

- Eric