futex: Fix race on the initial mm->futex.phash.ref allocation
Subsystems (2)
PATCH 1/1 futex: Fix race on the initial mm->futex.phash.ref allocation
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.
--- kernel/futex/core.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-)
kernel/futex/core.c +10 -6
@@ -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),
Signature · 1 line
2.43.0
Peter Zijlstra
reviewerQuoted 23 lines · Hyunwoo Kim
futex_hash_allocate()allocatesmm->futex.phash.refwithout any locking. Commitd9b05321e21e("futex: Movefutex_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: DropCLONE_THREADrequirement for private default hash alloc") widenedneed_futex_hash_allocate_default()to cover anyCLONE_VMclone, 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 seesmm->futex.phash.refas 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 infutex_q_lock().
Quoted 7 lines
Store the counter once with
cmpxchg()and let the loserfree_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()")Signed-off-by Hyunwoo Kim <[email protected]>
Yep :-( I'll go stick this in locking/urgent. Thanks!
Quoted 36 lines
--- kernel/futex/core.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-)kernel/futex/core.c +10
-6@@ -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),Signature · 1 line
2.43.0
The following commit has been merged into the locking/urgent branch of tip:
The following commit has been merged into the locking/urgent branch of tip:
Commit-ID: 4f664e37fe72ab21426d2d74fcbcf7399e8fc52c Gitweb: https://git.kernel.org/tip/4f664e37fe72ab21426d2d74fcbcf7399e8fc52c Author: Hyunwoo Kim <[email protected]> AuthorDate: Tue, 11 Aug 2026 23:03:16 +09:00 Committer: Peter Zijlstra <[email protected]> CommitterDate: Wed, 12 Aug 2026 12:57:13 +02:00
futex: Fix race on the initial mm->futex.phash.ref allocation
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.
--- kernel/futex/core.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-)
kernel/futex/core.c +10 -6
@@ -1857,14 +1857,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),
The following commit has been merged into the locking/urgent branch of tip:
The following commit has been merged into the locking/urgent branch of tip:
Commit-ID: bde0238083647381d4747355c5a19115a3422b96 Gitweb: https://git.kernel.org/tip/bde0238083647381d4747355c5a19115a3422b96 Author: Hyunwoo Kim <[email protected]> AuthorDate: Tue, 11 Aug 2026 23:03:16 +09:00 Committer: Thomas Gleixner <[email protected]> CommitterDate: Sun, 16 Aug 2026 00:16:32 +02:00
futex: Fix race on the initial mm->futex.phash.ref allocation
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.
--- kernel/futex/core.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-)
kernel/futex/core.c +10 -6
@@ -1857,14 +1857,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),