linux-kernel - Re: [PATCH] mm: mempool: fix wake-up edge case bug for zero-minimum pools

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20250716141931.273ca3effdbc0f442523eac8@linux-foundation.org>
Date: Wed, 16 Jul 2025 14:19:31 -0700
From: Andrew Morton <akpm@...ux-foundation.org>
To: Yadan Fan <ydfan@...e.com>
Cc: linux-mm@...ck.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] mm: mempool: fix wake-up edge case bug for zero-minimum
 pools

On Wed, 16 Jul 2025 23:37:30 +0800 Yadan Fan <ydfan@...e.com> wrote:

> The mempool wake-up mechanism has a edge case bug that affects pools
> created with min_nr=0. When a thread blocks waiting for memory from an
> empty pool (curr_nr == 0), subsequent mempool_free() calls fail to wake
> the waiting thread because the condition "curr_nr < min_nr" evaluates
> to "0 < 0" which is false, this causes threads to sleep indefinitely.
> 
> There is at least 2 places where the mempool created with min_nr=0:
> 
> 1. lib/btree.c:191: mempool_create(0, btree_alloc, btree_free, NULL)
> 2. drivers/md/dm-verity-fec.c:791:
>     mempool_init_slab_pool(&f->extra_pool, 0, f->cache)

This is very old code.  Can you suggest why this has taken so long to
surface?

Which is a roundabout way of asking "should this be backported into
-stable kernels".  For that we'd need to know how this issue is
affecting our users.

> Add an explicit check in mempool_free() to handle the min_nr=0 case:
> when the pool has zero minimum reserves, is currently empty, and has
> active waiters, wake them up. The wq_has_sleeper() avoids unnecessary
> wake-ups when no threads are waiting.

Do we need the separate test?  What's wrong with the obvious approach
of replacing the "<" with "<=" in the preceding test?

And would the previous (ie, existing) test benefit from the
wq_has_sleeper() check?

> --- a/mm/mempool.c
> +++ b/mm/mempool.c
> @@ -545,6 +545,22 @@ void mempool_free(void *element, mempool_t *pool)
>                  }
>                  spin_unlock_irqrestore(&pool->lock, flags);
>          }
> +       /*
> +        * Handle the min_nr = 0 edge case:
> +        * For zero-minimum pools, curr_nr < min_nr (0 < 0) never succeeds,
> +        * so waiters sleeping on pool->wait would never be woken by the
> +        * normal wake-up path. This explicit check ensures that when
> +        * pool->min_nr == 0 and pool->curr_nr == 0, any active waiters
> +        * are properly awakened.
> +        * The wq_has_sleeper() avoids unnecessary wake-ups when no
> +        * threads are waiting.
> +        */
> +       if (unlikely(pool->min_nr == 0 &&
> +                    READ_ONCE(pool->curr_nr) == 0 &&
> +                    wq_has_sleeper(&pool->wait))) {
> +               wake_up(&pool->wait);
> +       }
> +

Something strange is happening with the whitespace here.  I pretty much
retyped the patch.  Please have a chat with your email client ;)