Re: [PATCH] futex: Fix race on the initial mm->futex.phash.ref allocation

[Peter Zijlstra]

On Tue, Aug 11, 2026 at 11:03:16PM +0900, Hyunwoo Kim wrote:
> futex_hash_allocate() allocates mm->futex.phash.ref without any locking.
> Commit d9b05321e21e ("futex: Move futex_hash_free() back to __mmput()")
> moved the allocation here and assumed that the process has just a single
> thread at this point.
> 
> Commit ee9dce44362b ("futex: Drop CLONE_THREAD requirement for private
> default hash alloc") widened need_futex_hash_allocate_default() to cover
> any CLONE_VM clone, but left out vfork because the parent is suspended and
> cannot race.
> 
> That no longer holds once vfork is nested. If a vfork child calls vfork
> again and is then killed with SIGKILL, the parent is released from its
> vfork wait and runs concurrently with the grandchild in the same mm.
> Neither of them went through futex_hash_allocate_default().
> 
> When both call prctl(PR_FUTEX_HASH, PR_FUTEX_HASH_SET_SLOTS) at the same
> time, each one sees mm->futex.phash.ref as NULL and stores its own percpu
> counter. Only the last store survives. The counter stored first is no
> longer reachable from the mm, so the references on it are not seen by
> __futex_ref_atomic_end(). A private hash that still has references is then
> considered dead and freed, and a task that still holds one of its buckets
> writes into freed memory in futex_q_lock().
> 
> Store the counter once with cmpxchg() and let the loser free_percpu() its
> own. The initial reference has to be taken before the store, otherwise
> another task can install a private hash while the counter is still 0.
> 
> Fixes: d9b05321e21e ("futex: Move futex_hash_free() back to __mmput()")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Hyunwoo Kim <imv4bel@xxxxxxxxx>

Yep :-( I'll go stick this in locking/urgent. Thanks!

> ---
>  kernel/futex/core.c | 16 ++++++++++------
>  1 file changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/kernel/futex/core.c b/kernel/futex/core.c
> index 128c5752f225c2..806576978fa84c 100644
> --- a/kernel/futex/core.c
> +++ b/kernel/futex/core.c
> @@ -1842,14 +1842,18 @@ static int futex_hash_allocate(unsigned int hash_slots, unsigned int flags)
>  	}
>  
>  	if (!mm->futex.phash.ref) {
> +		unsigned int __percpu *ref = alloc_percpu(unsigned int);
> +
> +		if (!ref)
> +			return -ENOMEM;
> +
>  		/*
> -		 * This will always be allocated by the first thread and
> -		 * therefore requires no locking.
> +		 * Tasks sharing the mm can run this concurrently, so take the
> +		 * initial reference before publishing the counter.
>  		 */
> -		mm->futex.phash.ref = alloc_percpu(unsigned int);
> -		if (!mm->futex.phash.ref)
> -			return -ENOMEM;
> -		this_cpu_inc(*mm->futex.phash.ref); /* 0 -> 1 */
> +		this_cpu_inc(*ref); /* 0 -> 1 */
> +		if (cmpxchg(&mm->futex.phash.ref, NULL, ref))
> +			free_percpu(ref);
>  	}
>  
>  	fph = kvzalloc(struct_size(fph, queues, hash_slots),
> -- 
> 2.43.0
>