Re: [PATCH v1] mempool: fix min_nr exceeding the elements array after concurrent resizes

[Harry Yoo]

I refined the commit message a bit per Vlastimil's feedback...

On Tue, Oct 20, 2026 at 10:42:53AM +0200, Vlastimil Babka (SUSE) wrote:
> On 10/16/26 17:46, Harry Yoo wrote:
> >> ---
> >>
> >>  mm/mempool.c | 4 +++-
> >>  1 file changed, 3 insertions(+), 1 deletion(-)
> >> 
> >> diff --git a/mm/mempool.c b/mm/mempool.c
> >> index cb74e718b2c6..95533e2975ce 100644
> >> --- a/mm/mempool.c
> >> +++ b/mm/mempool.c
> >> @@ -378,7 +378,9 @@ int mempool_resize(struct mempool *pool, int new_min_nr)
> >>  			pool->free(element, pool->pool_data);
> >>  			spin_lock_irqsave(&pool->lock, flags);
> >>  		}
> >> -		pool->min_nr = new_min_nr;
> >> +		/* Raced, the array may now be smaller than new_min_nr */
> >> +		if (new_min_nr <= pool->min_nr)
> 
> There could be just '<' ?
> The outer if uses <= but the == case only leads to an early goto out_unlock.

However I didn't adjust this. the end result:
> mempool: fix min_nr exceeding the elements array after concurrent resizes
> 
> When mempool_resize() shrinks a pool, it releases and re-acquires
> pool->lock for every pool->free() call until the pool holds no more
> than new_min_nr elements, and then updates pool->min_nr to new_min_nr.
> 
> The shrink path has stored new_min_nr unconditionally since a historical
> commit 6892188b54d4 ("[PATCH] Re: [patch] change mempool to not alter
> managed elements"). However, unconditionally updating pool->min_nr is
> incorrect when a concurrent mempool_resize() has shrunk the pool size
> below new_min_nr.
> 
> Re-check new_min_nr against pool->min_nr under the lock and update
> pool->min_nr only if new_min_nr is not larger than pool->min_nr.
> 
> This was discovered by formal verification of mm/mempool.c against the
> kernel binary. While reproducing it in the kernel has not been
> successful, in a userspace implementation of mempool, ASAN was able to
> detect a heap buffer overflow.
> 
> In theory, an in-tree caller can hit this: fs/smb/client resizes the
> global cifs_req_poolp from cifs_demultiplex_thread() (grow, when a TCP
> session is created) and from clean_demultiplex_info() (shrink, when one
> goes away). The two are coordinated only through the atomic
> tcpSesAllocCount, so mounts and unmounts racing on one client run the
> resizes above concurrently; pool->free is kmem_cache_free() there and
> the window is a preemption inside it.  The other callers
> (block/blk-zoned.c, drivers/s390/scsi/zfcp_erp.c) serialise their
> resizes.

[+Cc folks in COMMON INTERNET FILE SYSTEM CLIENT (CIFS and SMB3)
 as the paragraph above mentions it. ]

> Assisted-by: LLM
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")

Here I'm bit hesitant to add Cc: stable for a 24-years-old theoretical
race that hasn't been reproduced in the kernel. But it would be easy
to backport if anybody complains.

> Signed-off-by: Nickolai Zeldovich <nickolai@xxxxxxxxxxxxx>
> Reviewed-by: Christoph Hellwig <hch@xxxxxx>
> Link: https://git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git/commit/?id=6892188b54d484098b79b0c9813c20e543124e2d
> Link: https://patch.msgid.link/20261016132510.789715-1-nickolai@xxxxxxxxxxxxx
> [ harry@xxxxxxxxxx: refine changelog per Vlastimil's comment ]
> Signed-off-by: Harry Yoo (Meta) <harry@xxxxxxxxxx>

This will be queued to slab/for-next-fixes soon.

Thanks!

-- 
Cheers,
Harry / Hyeonggon