[PATCH] mempool: fix the race condition in mempool_resize()

[PATCH] mempool: fix the race condition in mempool_resize()

[Vitaly Wool]

From: Igor Belousov <igor.b@beldev.am>

mempool_resize() at some point has no valid elements array for a pool:
...
    kfree(pool->elements);
    /* here pool->elements is not valid */
    pool->elements = new_elements;
...

If e. g. mempool_alloc() tries to access pool->elements after kfree()
but before the assignment that follows, we end up with an undefined
behavior. Fix that by changing pool->elements to new_elements first
and then freeing up the old array.

Signed-off-by: Igor Belousov <igor.b@beldev.am>
Signed-off-by: Vitaly Wool <vitaly.wool@konsulko.se>
---
 mm/mempool.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/mm/mempool.c b/mm/mempool.c
index db23e0eef652..302d83cbeac1 100644
--- a/mm/mempool.c
+++ b/mm/mempool.c
@@ -384,8 +384,8 @@ int mempool_resize(struct mempool *pool, int new_min_nr)
 	}
 	memcpy(new_elements, pool->elements,
 			pool->curr_nr * sizeof(*new_elements));
-	kfree(pool->elements);
-	pool->elements = new_elements;
+	xchg(pool->elements, new_elements);
+	kfree(new_elements);
 	pool->min_nr = new_min_nr;
 
 	while (pool->curr_nr < pool->min_nr) {
-- 
2.39.2

Re: [PATCH] mempool: fix the race condition in mempool_resize()

[Harry Yoo]

On Wed, Mar 04, 2026 at 02:12:14PM +0100, Vitaly Wool wrote:
> From: Igor Belousov <igor.b@beldev.am>
> 
> mempool_resize() at some point has no valid elements array for a pool:
> ...
>     kfree(pool->elements);
>     /* here pool->elements is not valid */
>     pool->elements = new_elements;
> ...
> 
> If e. g. mempool_alloc() tries to access pool->elements after kfree()
> but before the assignment that follows, we end up with an undefined
> behavior. Fix that by changing pool->elements to new_elements first
> and then freeing up the old array.

Hi, is this from code inspection, or a real bug you observed?

I think pool->lock should prevent the bug you described from happening
and I don't think using xchg() is necessary when updating fields
protected by a spinlock.

-- 
Cheers,
Harry / Hyeonggon

> Signed-off-by: Igor Belousov <igor.b@beldev.am>
> Signed-off-by: Vitaly Wool <vitaly.wool@konsulko.se>
> ---
>  mm/mempool.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/mm/mempool.c b/mm/mempool.c
> index db23e0eef652..302d83cbeac1 100644
> --- a/mm/mempool.c
> +++ b/mm/mempool.c
> @@ -384,8 +384,8 @@ int mempool_resize(struct mempool *pool, int new_min_nr)
>  	}
>  	memcpy(new_elements, pool->elements,
>  			pool->curr_nr * sizeof(*new_elements));
> -	kfree(pool->elements);
> -	pool->elements = new_elements;
> +	xchg(pool->elements, new_elements);
> +	kfree(new_elements);
>  	pool->min_nr = new_min_nr;
>  
>  	while (pool->curr_nr < pool->min_nr) {
> -- 
> 2.39.2
> 
> 

Re: [PATCH] mempool: fix the race condition in mempool_resize()

[igor.b]

On 2026-03-04 17:31, Harry Yoo wrote:
> On Wed, Mar 04, 2026 at 02:12:14PM +0100, Vitaly Wool wrote:
>> From: Igor Belousov <igor.b@beldev.am>
>> 
>> mempool_resize() at some point has no valid elements array for a pool:
>> ...
>>     kfree(pool->elements);
>>     /* here pool->elements is not valid */
>>     pool->elements = new_elements;
>> ...
>> 
>> If e. g. mempool_alloc() tries to access pool->elements after kfree()
>> but before the assignment that follows, we end up with an undefined
>> behavior. Fix that by changing pool->elements to new_elements first
>> and then freeing up the old array.
> 
> Hi, is this from code inspection, or a real bug you observed?

This is a real problem, not that easy to reproduce though.

> I think pool->lock should prevent the bug you described from happening
> and I don't think using xchg() is necessary when updating fields
> protected by a spinlock.

Can you please explain how pool->lock protects pool->elements in e.g. 
remove_element() function?

Thanks,
Igor

> --
> Cheers,
> Harry / Hyeonggon
> 
>> Signed-off-by: Igor Belousov <igor.b@beldev.am>
>> Signed-off-by: Vitaly Wool <vitaly.wool@konsulko.se>
>> ---
>>  mm/mempool.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>> 
>> diff --git a/mm/mempool.c b/mm/mempool.c
>> index db23e0eef652..302d83cbeac1 100644
>> --- a/mm/mempool.c
>> +++ b/mm/mempool.c
>> @@ -384,8 +384,8 @@ int mempool_resize(struct mempool *pool, int 
>> new_min_nr)
>>  	}
>>  	memcpy(new_elements, pool->elements,
>>  			pool->curr_nr * sizeof(*new_elements));
>> -	kfree(pool->elements);
>> -	pool->elements = new_elements;
>> +	xchg(pool->elements, new_elements);
>> +	kfree(new_elements);
>>  	pool->min_nr = new_min_nr;
>> 
>>  	while (pool->curr_nr < pool->min_nr) {
>> --
>> 2.39.2
>> 
>> 

Re: [PATCH] mempool: fix the race condition in mempool_resize()

[Andrew Morton]

On Wed, 04 Mar 2026 19:20:30 +0400 igor.b@beldev.am wrote:

> > I think pool->lock should prevent the bug you described from happening
> > and I don't think using xchg() is necessary when updating fields
> > protected by a spinlock.
> 
> Can you please explain how pool->lock protects pool->elements in e.g. 
> remove_element() function?

afaict all callers of add_element() and remove_element() hold
pool->lock.  mempool_exit() doesn't but presumably there are no
concurrent accessers at this time.

So yes, perhaps a more complete problem description is needed here.