On Wed, Feb 24, 2021 at 08:24:04AM +0100, Christoph Hellwig wrote:
> bio_split with a NULL bs argumen used to fall back to kmalloc the
> bio, which does not guarantee forward progress and could to deadlocks.
"argumen" => "argument"
"could to" => "could lead to"
> Now that the overloading of the NULL bs argument to bio_alloc_bioset
> has been removed it crashes instead. Fix all that by using a special
> crafted bioset.
>
> Fixes: 3175199ab0ac ("block: split bio_kmalloc from bio_alloc_bioset")
> Reported-by: John Stultz <john.stultz@xxxxxxxxxx>
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
> Tested-by: John Stultz <john.stultz@xxxxxxxxxx>
> ---
> block/blk-crypto-fallback.c | 12 ++++++++++--
> 1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/block/blk-crypto-fallback.c b/block/blk-crypto-fallback.c
> index e8327c50d7c9f4..c176b7af56a7a5 100644
> --- a/block/blk-crypto-fallback.c
> +++ b/block/blk-crypto-fallback.c
> @@ -80,6 +80,7 @@ static struct blk_crypto_keyslot {
> static struct blk_keyslot_manager blk_crypto_ksm;
> static struct workqueue_struct *blk_crypto_wq;
> static mempool_t *blk_crypto_bounce_page_pool;
> +static struct bio_set crypto_bio_split;
Something like "blk_crypto_bioset" might be a better name, since the other
variables above are prefixed with "blk_crypto".
> /*
> * This is the key we set when evicting a keyslot. This *should* be the all 0's
> @@ -224,7 +225,8 @@ static bool blk_crypto_split_bio_if_needed(struct bio **bio_ptr)
> if (num_sectors < bio_sectors(bio)) {
> struct bio *split_bio;
>
> - split_bio = bio_split(bio, num_sectors, GFP_NOIO, NULL);
> + split_bio = bio_split(bio, num_sectors, GFP_NOIO,
> + &crypto_bio_split);
> if (!split_bio) {
> bio->bi_status = BLK_STS_RESOURCE;
> return false;
> @@ -538,9 +540,13 @@ static int blk_crypto_fallback_init(void)
>
> prandom_bytes(blank_key, BLK_CRYPTO_MAX_KEY_SIZE);
>
> - err = blk_ksm_init(&blk_crypto_ksm, blk_crypto_num_keyslots);
> + err = bioset_init(&crypto_bio_split, 64, 0, 0);
> if (err)
> goto out;
> +
> + err = blk_ksm_init(&blk_crypto_ksm, blk_crypto_num_keyslots);
> + if (err)
> + goto fail_free_bioset;
> err = -ENOMEM;
>
> blk_crypto_ksm.ksm_ll_ops = blk_crypto_ksm_ll_ops;
> @@ -591,6 +597,8 @@ static int blk_crypto_fallback_init(void)
> destroy_workqueue(blk_crypto_wq);
> fail_free_ksm:
> blk_ksm_destroy(&blk_crypto_ksm);
> +fail_free_bioset:
> + bioset_exit(&crypto_bio_split);
> out:
> return err;
> }
Anyway, this works. Feel free to add:
Reviewed-by: Eric Biggers <ebiggers@xxxxxxxxxx>
- Eric
