[PATCH 0/3] Fix a deadlock in blk-crypto-fallback

[PATCH 0/3] Fix a deadlock in blk-crypto-fallback

[Eric Biggers]

This series fixes a deadlock in blk-crypto-fallback that could occur
when a large WRITE bio was submitted to dm-inlinecrypt.

As a prerequisite, it makes mempool_alloc_bulk() support fallible
allocations in the same way that mempool_alloc() does.

Please consider this series for the block tree for 7.3.

Eric Biggers (3):
  mm: make mempool_alloc_from_pool() return bool
  mm: support fallible mempool_alloc_bulk()
  blk-crypto-fallback: Fix deadlock when encrypting large bio in dm
    layer

 block/blk-crypto-fallback.c | 72 +++++++++++++++++++++++++++++++++----
 include/linux/mempool.h     |  4 +--
 mm/mempool.c                | 50 +++++++++++++++++---------
 3 files changed, 100 insertions(+), 26 deletions(-)


base-commit: 0d839570765118029aa8bf4a95444c6a11aacf85
-- 
2.55.0

[PATCH 1/3] mm: make mempool_alloc_from_pool() return bool

[Eric Biggers]

mempool_alloc_from_pool() is intentionally all-or-nothing, so make it
return a bool rather than the number of elements allocated.

Then make mempool_alloc_bulk() return right away if
mempool_alloc_from_pool() succeeds, rather than jumping back to the
retry_alloc label to allocate nothing and then returning.

Signed-off-by: Eric Biggers <ebiggers@kernel.org>
---
 mm/mempool.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/mm/mempool.c b/mm/mempool.c
index 473a029fa31f..d454bc9f39e9 100644
--- a/mm/mempool.c
+++ b/mm/mempool.c
@@ -409,7 +409,7 @@ int mempool_resize(struct mempool *pool, int new_min_nr)
 }
 EXPORT_SYMBOL(mempool_resize);
 
-static unsigned int mempool_alloc_from_pool(struct mempool *pool, void **elems,
+static bool mempool_alloc_from_pool(struct mempool *pool, void **elems,
 		unsigned int count, unsigned int allocated,
 		gfp_t gfp_mask)
 {
@@ -432,7 +432,7 @@ static unsigned int mempool_alloc_from_pool(struct mempool *pool, void **elems,
 	 */
 	for (i = 0; i < count; i++)
 		kmemleak_update_trace(elems[i]);
-	return allocated;
+	return true;
 
 fail:
 	if (gfp_mask & __GFP_DIRECT_RECLAIM) {
@@ -454,7 +454,7 @@ static unsigned int mempool_alloc_from_pool(struct mempool *pool, void **elems,
 		spin_unlock_irqrestore(&pool->lock, flags);
 	}
 
-	return allocated;
+	return false;
 }
 
 /*
@@ -519,8 +519,8 @@ int mempool_alloc_bulk_noprof(struct mempool *pool, void **elems,
 	return 0;
 
 use_pool:
-	allocated = mempool_alloc_from_pool(pool, elems, count, allocated,
-			gfp_temp);
+	if (mempool_alloc_from_pool(pool, elems, count, allocated, gfp_temp))
+		return 0;
 	gfp_temp = gfp_mask;
 	goto repeat_alloc;
 }
-- 
2.55.0

Re: [PATCH 1/3] mm: make mempool_alloc_from_pool() return bool

[Christoph Hellwig]

Looks good and matches what I recently did on the slab side:

Reviewed-by: Christoph Hellwig <hch@lst.de>

[PATCH 2/3] mm: support fallible mempool_alloc_bulk()

[Eric Biggers]

To fix a deadlock, blk-crypto-fallback needs to be able to make fallible
mempool_alloc_bulk() allocations.  But mempool_alloc_bulk() hardcodes
GFP_KERNEL and infinite retries, which differs from mempool_alloc()
which supports fallible allocations via its gfp_mask argument.

Therefore, add a gfp_mask argument to mempool_alloc_bulk().  As with
mempool_alloc(), the presence of __GFP_DIRECT_RECLAIM in the mask
selects between the fallible and infallible modes.

For now it just provides all-or-nothing semantics and returns a bool,
similar to kmem_cache_alloc_bulk().

Signed-off-by: Eric Biggers <ebiggers@kernel.org>
---
 block/blk-crypto-fallback.c |  6 ++----
 include/linux/mempool.h     |  4 ++--
 mm/mempool.c                | 42 +++++++++++++++++++++++++------------
 3 files changed, 33 insertions(+), 19 deletions(-)

diff --git a/block/blk-crypto-fallback.c b/block/blk-crypto-fallback.c
index 2a5c52ab74b4..bda913c39381 100644
--- a/block/blk-crypto-fallback.c
+++ b/block/blk-crypto-fallback.c
@@ -172,7 +172,6 @@ static void blk_crypto_fallback_encrypt_endio(struct bio *enc_bio)
 static struct bio *blk_crypto_alloc_enc_bio(struct bio *bio_src,
 		unsigned int nr_segs, struct page ***pages_ret)
 {
-	unsigned int memflags = memalloc_noio_save();
 	unsigned int nr_allocated;
 	struct page **pages;
 	struct bio *bio;
@@ -206,12 +205,11 @@ static struct bio *blk_crypto_alloc_enc_bio(struct bio *bio_src,
 	 * any non-zero slot already contains a valid allocation.
 	 */
 	memset(pages, 0, sizeof(struct page *) * nr_segs);
-	nr_allocated = alloc_pages_bulk(GFP_KERNEL, nr_segs, pages);
+	nr_allocated = alloc_pages_bulk(GFP_NOIO, nr_segs, pages);
 	if (nr_allocated < nr_segs)
 		mempool_alloc_bulk(blk_crypto_bounce_page_pool,
 				(void **)pages + nr_allocated,
-				nr_segs - nr_allocated);
-	memalloc_noio_restore(memflags);
+				nr_segs - nr_allocated, GFP_NOIO);
 	*pages_ret = pages;
 	return bio;
 }
diff --git a/include/linux/mempool.h b/include/linux/mempool.h
index a0fa6d43e0dc..f7898cd1512b 100644
--- a/include/linux/mempool.h
+++ b/include/linux/mempool.h
@@ -65,8 +65,8 @@ void mempool_destroy(struct mempool *pool);
 void *mempool_alloc_noprof(struct mempool *pool, gfp_t gfp_mask) __malloc;
 #define mempool_alloc(...)						\
 	alloc_hooks(mempool_alloc_noprof(__VA_ARGS__))
-int mempool_alloc_bulk_noprof(struct mempool *pool, void **elem,
-		unsigned int count);
+bool mempool_alloc_bulk_noprof(struct mempool *pool, void **elem,
+		unsigned int count, gfp_t gfp_mask);
 #define mempool_alloc_bulk(...)						\
 	alloc_hooks(mempool_alloc_bulk_noprof(__VA_ARGS__))
 
diff --git a/mm/mempool.c b/mm/mempool.c
index d454bc9f39e9..d741e5f62554 100644
--- a/mm/mempool.c
+++ b/mm/mempool.c
@@ -473,25 +473,28 @@ static inline gfp_t mempool_adjust_gfp(gfp_t *gfp_mask)
 /**
  * mempool_alloc_bulk - allocate multiple elements from a memory pool
  * @pool:	pointer to the memory pool
- * @elems:	partially or fully populated elements array
- * @count:	number of entries in @elem that need to be allocated
+ * @elems:	pointer to array into which the element pointers will be stored
+ * @count:	number of elements to allocate
+ * @gfp_mask:	GFP_* flags.  %__GFP_ZERO is not supported.  If this mask
+ *		includes %__GFP_DIRECT_RECLAIM, then the allocation is retried
+ *		indefinitely until it succeeds and the return value is always
+ *		%true.  If the mask doesn't include %__GFP_DIRECT_RECLAIM, then
+ *		failure is allowed and %false can be returned.
  *
  * Allocate @count elements into @elems.  This is done by first calling into the
  * alloc_fn supplied at pool initialization time, and dipping into the reserved
- * pool when alloc_fn fails to allocate an element.
- *
- * On return all @count elements in @elems will be populated.
+ * pool to atomically allocate the remaining elements if alloc_fn fails.
  *
- * Return: Always 0.  If it wasn't for %$#^$ alloc tags, it would return void.
+ * Return: %true if the allocation succeeded, or %false if it failed.
  */
-int mempool_alloc_bulk_noprof(struct mempool *pool, void **elems,
-		unsigned int count)
+bool mempool_alloc_bulk_noprof(struct mempool *pool, void **elems,
+		unsigned int count, gfp_t gfp_mask)
 {
-	gfp_t gfp_mask = GFP_KERNEL;
 	gfp_t gfp_temp = mempool_adjust_gfp(&gfp_mask);
 	unsigned int allocated = 0;
 
 	VM_WARN_ON_ONCE(count > pool->min_nr);
+	VM_WARN_ON_ONCE(gfp_mask & __GFP_ZERO);
 	might_alloc(gfp_mask);
 
 	/*
@@ -516,13 +519,26 @@ int mempool_alloc_bulk_noprof(struct mempool *pool, void **elems,
 		allocated++;
 	}
 
-	return 0;
+	return true;
 
 use_pool:
+	/* Try to atomically allocate the remaining elements from the pool. */
 	if (mempool_alloc_from_pool(pool, elems, count, allocated, gfp_temp))
-		return 0;
-	gfp_temp = gfp_mask;
-	goto repeat_alloc;
+		return true;
+	/* Retry if this was just the opportunistic first pass. */
+	if (gfp_temp != gfp_mask) {
+		gfp_temp = gfp_mask;
+		goto repeat_alloc;
+	}
+	/* Retry indefinitely if __GFP_DIRECT_RECLAIM is set. */
+	if (gfp_mask & __GFP_DIRECT_RECLAIM)
+		goto repeat_alloc;
+	/* On failure, roll back any successful allocations from ->alloc(). */
+	while (allocated--) {
+		pool->free(elems[allocated], pool->pool_data);
+		elems[allocated] = NULL;
+	}
+	return false;
 }
 EXPORT_SYMBOL_GPL(mempool_alloc_bulk_noprof);
 
-- 
2.55.0

Re: [PATCH 2/3] mm: support fallible mempool_alloc_bulk()

[Christoph Hellwig]

On Thu, Aug 06, 2026 at 03:10:30PM -0700, Eric Biggers wrote:
> To fix a deadlock, blk-crypto-fallback needs to be able to make fallible
> mempool_alloc_bulk() allocations.

It doesn't.  fallible mempool allocations are a concept that doesn't
make much sense.  Please just go straight to the backing page allocator
instead for callers that do not need the mempool guarantees.

Re: [PATCH 2/3] mm: support fallible mempool_alloc_bulk()

[Eric Biggers]

On Mon, Aug 10, 2026 at 09:00:11AM -0700, Christoph Hellwig wrote:
> On Thu, Aug 06, 2026 at 03:10:30PM -0700, Eric Biggers wrote:
> > To fix a deadlock, blk-crypto-fallback needs to be able to make fallible
> > mempool_alloc_bulk() allocations.
> 
> It doesn't.  fallible mempool allocations are a concept that doesn't
> make much sense.  Please just go straight to the backing page allocator
> instead for callers that do not need the mempool guarantees.

It does make sense.  When alloc_pages_bulk() doesn't completely succeed,
there still might be pages available in the mempool.
blk_crypto_alloc_enc_bio() should try to take them before falling back
to scheduling the rescuer kworker.  The rescuer encounters scheduling
overhead and is single-threaded, so it's slow and should be used only
when absolutely necessary.

- Eric

Re: [PATCH 2/3] mm: support fallible mempool_alloc_bulk()

[Christoph Hellwig]

On Mon, Aug 10, 2026 at 09:14:04AM -0700, Eric Biggers wrote:
> On Mon, Aug 10, 2026 at 09:00:11AM -0700, Christoph Hellwig wrote:
> > On Thu, Aug 06, 2026 at 03:10:30PM -0700, Eric Biggers wrote:
> > > To fix a deadlock, blk-crypto-fallback needs to be able to make fallible
> > > mempool_alloc_bulk() allocations.
> > 
> > It doesn't.  fallible mempool allocations are a concept that doesn't
> > make much sense.  Please just go straight to the backing page allocator
> > instead for callers that do not need the mempool guarantees.
> 
> It does make sense.  When alloc_pages_bulk() doesn't completely succeed,
> there still might be pages available in the mempool.

But they should not go to a caller that does not need the mempool.

> blk_crypto_alloc_enc_bio() should try to take them before falling back
> to scheduling the rescuer kworker.  The rescuer encounters scheduling
> overhead and is single-threaded, so it's slow and should be used only
> when absolutely necessary.

No, it should just try a regular non-bulk alloc_pages (and eventually
alloc_pages_bulk should do that fallback for the callers, but that's
a separate discussion).

Re: [PATCH 2/3] mm: support fallible mempool_alloc_bulk()

[Eric Biggers]

On Mon, Aug 10, 2026 at 10:21:15AM -0700, Christoph Hellwig wrote:
> On Mon, Aug 10, 2026 at 09:14:04AM -0700, Eric Biggers wrote:
> > On Mon, Aug 10, 2026 at 09:00:11AM -0700, Christoph Hellwig wrote:
> > > On Thu, Aug 06, 2026 at 03:10:30PM -0700, Eric Biggers wrote:
> > > > To fix a deadlock, blk-crypto-fallback needs to be able to make fallible
> > > > mempool_alloc_bulk() allocations.
> > > 
> > > It doesn't.  fallible mempool allocations are a concept that doesn't
> > > make much sense.  Please just go straight to the backing page allocator
> > > instead for callers that do not need the mempool guarantees.
> > 
> > It does make sense.  When alloc_pages_bulk() doesn't completely succeed,
> > there still might be pages available in the mempool.
> 
> But they should not go to a caller that does not need the mempool.

The caller does need the mempool.

> > blk_crypto_alloc_enc_bio() should try to take them before falling back
> > to scheduling the rescuer kworker.  The rescuer encounters scheduling
> > overhead and is single-threaded, so it's slow and should be used only
> > when absolutely necessary.
> 
> No, it should just try a regular non-bulk alloc_pages (and eventually
> alloc_pages_bulk should do that fallback for the callers, but that's
> a separate discussion).

It already does that.  blk_crypto_alloc_enc_bio() first calls
alloc_pages_bulk().  If it doesn't completely succeed, it calls
mempool_alloc_bulk() to get the rest.  That itself tries regular
allocations again before actually using the mempool.  It needs a
guaranteed allocation, so it needs the mempool.

Now, as I explained in this patchset, whether this code can wait forever
for the mempool actually depends on whether it's a recursive bio
submission or not.  If it is, then it cannot wait, but ultimately it
does still need to use the mempool to get a guaranteed allocation.

Falling back to the rescuer kthread (which can wait on the mempool)
solves that.  But before taking that slow fallback, it's much more
efficient to check the mempool directly first, since pages may be
available there (and in fact it's fairly likely that they will be, since
regular allocations are always used first).  The whole point of the
mempool is that it can be used when the regular allocation fails.

This is also the only caller of mempool_alloc_bulk().  So I'm kind of
confused why it would not be allowed to implement the behavior that is
desired here, especially when the non-bulk API offers it already.

- Eric

[PATCH 3/3] blk-crypto-fallback: Fix deadlock when encrypting large bio in dm layer

[Eric Biggers]

Encrypting a large bio with more than BIO_MAX_VECS pages can still
deadlock, even after it was attempted to be fixed by
commit b37fbce460ad ("blk-crypto: optimize bio splitting in
blk_crypto_fallback_encrypt_bio") and commit 3d939695e682 ("blk-crypto:
use mempool_alloc_bulk for encrypted bio page allocation").

This is because __blk_crypto_fallback_encrypt_bio() assumes that the
bounce bios that it submits will eventually complete, unblocking it from
allocating additional bounce bios and pages.  However, dm-inlinecrypt.c
calls __blk_crypto_submit_bio() from within submit_bio() itself.  In
this case, the recursive submit_bio() simply adds the bounce bio to
current->bio_list without actually submitting it yet.  That breaks the
guarantee that forward progress is being made.

To fix this, allocate the bio and bounce pages with GFP_NOWAIT if
current->bio_list is set.  If it fails, punt the encryption of the
remaining part of the bio to a kworker.

Fixes: 488f6682c832 ("block: blk-crypto-fallback for Inline Encryption")
Cc: stable@vger.kernel.org
Signed-off-by: Eric Biggers <ebiggers@kernel.org>
---
 block/blk-crypto-fallback.c | 70 ++++++++++++++++++++++++++++++++++---
 1 file changed, 65 insertions(+), 5 deletions(-)

diff --git a/block/blk-crypto-fallback.c b/block/blk-crypto-fallback.c
index bda913c39381..973399011d5a 100644
--- a/block/blk-crypto-fallback.c
+++ b/block/blk-crypto-fallback.c
@@ -83,6 +83,10 @@ static struct workqueue_struct *blk_crypto_wq;
 static mempool_t *blk_crypto_bounce_page_pool;
 static struct bio_set enc_bio_set;
 
+static DEFINE_SPINLOCK(enc_rescue_list_lock);
+static struct bio_list enc_rescue_list;
+static struct work_struct enc_rescue_work;
+
 /*
  * This is the key we set when evicting a keyslot. This *should* be the all 0's
  * key, but AES-XTS rejects that key, so we use some random bytes instead.
@@ -175,9 +179,23 @@ static struct bio *blk_crypto_alloc_enc_bio(struct bio *bio_src,
 	unsigned int nr_allocated;
 	struct page **pages;
 	struct bio *bio;
+	gfp_t gfp_mask;
+
+	/*
+	 * During recursive bio submission (current->bio_list != NULL) any
+	 * submitted bounce bios just get added to current->bio_list; they
+	 * cannot complete and release resources yet.  Therefore, to avoid
+	 * deadlocks, don't wait indefinitely for additional resources.
+	 */
+	if (current->bio_list)
+		gfp_mask = GFP_NOWAIT;
+	else
+		gfp_mask = GFP_NOIO;
 
 	bio = bio_alloc_bioset(bio_src->bi_bdev, nr_segs, bio_src->bi_opf,
-			GFP_NOIO, &enc_bio_set);
+			       gfp_mask, &enc_bio_set);
+	if (unlikely(!bio))
+		return NULL;
 	if (bio_flagged(bio_src, BIO_REMAPPED))
 		bio_set_flag(bio, BIO_REMAPPED);
 	bio->bi_private		= bio_src;
@@ -205,11 +223,15 @@ static struct bio *blk_crypto_alloc_enc_bio(struct bio *bio_src,
 	 * any non-zero slot already contains a valid allocation.
 	 */
 	memset(pages, 0, sizeof(struct page *) * nr_segs);
-	nr_allocated = alloc_pages_bulk(GFP_NOIO, nr_segs, pages);
-	if (nr_allocated < nr_segs)
-		mempool_alloc_bulk(blk_crypto_bounce_page_pool,
+	nr_allocated = alloc_pages_bulk(gfp_mask, nr_segs, pages);
+	if (unlikely(nr_allocated < nr_segs) &&
+	    !mempool_alloc_bulk(blk_crypto_bounce_page_pool,
 				(void **)pages + nr_allocated,
-				nr_segs - nr_allocated, GFP_NOIO);
+				nr_segs - nr_allocated, gfp_mask)) {
+		free_pages_bulk(pages, nr_allocated);
+		bio_put(bio);
+		return NULL;
+	}
 	*pages_ret = pages;
 	return bio;
 }
@@ -237,6 +259,25 @@ static void blk_crypto_dun_to_iv(const u64 dun[BLK_CRYPTO_DUN_ARRAY_SIZE],
 		iv->dun[i] = cpu_to_le64(dun[i]);
 }
 
+static void blk_crypto_fallback_encrypt_bio(struct bio *src_bio);
+
+/* Encrypt a list of bios whose encryption was punted to a kworker. */
+static void blk_crypto_fallback_encrypt_work_fn(struct work_struct *work)
+{
+	struct bio_list list;
+	struct bio *src_bio;
+
+	WARN_ON_ONCE(current->bio_list);
+
+	spin_lock(&enc_rescue_list_lock);
+	list = enc_rescue_list;
+	bio_list_init(&enc_rescue_list);
+	spin_unlock(&enc_rescue_list_lock);
+
+	while ((src_bio = bio_list_pop(&list)))
+		blk_crypto_fallback_encrypt_bio(src_bio);
+}
+
 static void __blk_crypto_fallback_encrypt_bio(struct bio *src_bio,
 		struct crypto_sync_skcipher *tfm)
 {
@@ -271,6 +312,23 @@ static void __blk_crypto_fallback_encrypt_bio(struct bio *src_bio,
 new_bio:
 	nr_enc_pages = min(bio_segments(src_bio), BIO_MAX_VECS);
 	enc_bio = blk_crypto_alloc_enc_bio(src_bio, nr_enc_pages, &enc_pages);
+	if (unlikely(!enc_bio)) {
+		/*
+		 * Failed to allocate a bounce bio during recursive bio
+		 * submission.  We might be blocked on bios in current->bio_list
+		 * holding mempool elements.  To enable forward progress, punt
+		 * the remaining encryption work for src_bio to a kworker.
+		 *
+		 * The DUN may have been advanced, so make sure to update it.
+		 */
+		WARN_ON_ONCE(!current->bio_list);
+		memcpy(bc->bc_dun, curr_dun, sizeof(curr_dun));
+		spin_lock(&enc_rescue_list_lock);
+		bio_list_add(&enc_rescue_list, src_bio);
+		spin_unlock(&enc_rescue_list_lock);
+		queue_work(blk_crypto_wq, &enc_rescue_work);
+		return;
+	}
 	enc_idx = 0;
 	for (;;) {
 		struct bio_vec src_bv =
@@ -589,6 +647,8 @@ static int blk_crypto_fallback_init(void)
 	if (!bio_fallback_crypt_ctx_pool)
 		goto fail_free_crypt_ctx_cache;
 
+	INIT_WORK(&enc_rescue_work, blk_crypto_fallback_encrypt_work_fn);
+
 	blk_crypto_fallback_inited = true;
 
 	return 0;
-- 
2.55.0