mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [BUG] futex: scheduling-while-atomic because nested vfork can break guard(private_hash)
@ 2026-09-10 15:27 Jann Horn
  2026-09-11  8:36 ` Peter Zijlstra
  0 siblings, 1 reply; 7+ messages in thread
From: Jann Horn @ 2026-09-10 15:27 UTC (permalink / raw)
  To: Hyunwoo Kim, Peter Zijlstra, Thomas Gleixner, Ingo Molnar,
	Darren Hart, Davidlohr Bueso, André Almeida
  Cc: kernel list

[-- Attachment #1: Type: text/plain, Size: 12037 bytes --]

Introduction
============
The futex private hash implementation was written with the assumption
that any MM used by more than one running task either has a private
hash or has irrevocably opted out of having a private hash; but this
isn't actually the case.

Commit ee9dce44362b ("futex: Drop CLONE_THREAD requirement for private
default hash alloc") tried to fix cases where processes can share an
MM without triggering private hash allocation, but overlooked that
nested vfork can also result in concurrently running processes sharing
an MM. After that was discovered, commit bde023808364 ("futex: Fix
race on the initial mm->futex.phash.ref allocation") instead changed
futex_hash_allocate() to not rely on this assumption anymore; but
other places in the futex code still make this assumption.

Issue description
============
exit_pi_state_list() contains this code to handle waiters on PI
futexes held by the current task, which is exiting or going through
execve:
```
 /*
* Ensure the hash remains stable (no resize) during the while loop
* below. The hb pointer is acquired under the pi_lock so we can't block
* on the mutex.
*/
[...]
guard(private_hash)(current->mm);
[...]
raw_spin_lock_irq(&curr->pi_lock);
while (!list_empty(head)) {
  [...]
  if (1) {
    CLASS(hbr, hbr)(&key);
```

CLASS(hbr, hbr)(&key) calls futex_hash(), which (if the MM is in the
middle of switching between two futex_private_hash instances) can call
futex_pivot_hash(), which can block on scoped_guard(mutex,
&mm->futex.phash.lock). guard(private_hash) is supposed to have taken
a reference on the futex_private_hash, which would prevent switching
to another futex_private_hash instance; but guard(private_hash) will
have been a no-op if the MM didn't have a futex_private_hash yet at
that time.

So it is possible to race as follows:

First, use nested vfork to create three processes P1, P2, and P3 that
share one MM and can run concurrently. That can be done like this:

 - P1 vforks to create P2'
 - P2' vforks to create P2
 - P2 sends SIGKILL to P2' to unblock P1
 - P1 vforks to create P3'
 - P3' vforks to create P3
 - P3 sends SIGKILL to P3' to unblock P1

Then race like this (view this in monospace):

P1         P2                   P3          RCU
==         ==                   ==          ===
           futex(FUTEX_LOCK_PI|FUTEX_PRIVATE_FLAG)
             [success]
                                futex(FUTEX_LOCK_PI|FUTEX_PRIVATE_FLAG)
                                  [blocks]
           do_exit
             exit_mm
               mm_exit_exec_release
                 futex_exit_exec_release
                   futex_cleanup
                     exit_pi_state_list
                       guard(private_hash)(current->mm)
futex_hash_prctl(PR_FUTEX_HASH, PR_FUTEX_HASH_SET_SLOTS)
futex_hash_prctl(PR_FUTEX_HASH, PR_FUTEX_HASH_SET_SLOTS)
  futex_hash_allocate
    scoped_guard(mutex, &mm->futex.phash.lock)
    futex_ref_drop
      __futex_ref_atomic_begin
        call_rcu_hurry(..., futex_ref_rcu)
                                            futex_ref_rcu
                                              __futex_ref_atomic_end
                       raw_spin_lock_irq(&curr->pi_lock)
                       CLASS(hbr, hbr)(&key)
                         futex_hash
                           futex_private_hash_get [fails]
                           futex_pivot_hash
                             scoped_guard(mutex, &mm->futex.phash.lock)
                               *** scheduling-while-atomic ***
    __futex_pivot_hash

Impact, related issues
==========
futex_wait_multiple_setup() follows the same pattern of using
guard(private_hash)(current->mm) to ensure that a later CLASS(hbr,
hbr)(&q->key) won't sleep and is probably affected by the same issue.

__futex_unlock_pi() and requeue_pi_wake_futex() also use
futex_private_hash(), but there the race seems benign.

This issue also means that different futex users could disagree about
the hash bucket that a private futex belongs in (older futex_hash()
calls returning a pointer into the global hash, newer futex_hash()
calls returning a pointer into the private hash). For most users of
CLASS(hbr, hbr)(...), that probably just leads to futex wake/wait
operations on the same userspace address not interacting with each
other, without affecting the kernel. But an interesting aspect of
exit_pi_state_list() is that it does spin_lock(&hb->lock) further down
without any visible access to the corresponding hash bucket; I can't
tell if that actually protects against something, but if yes, this
locking might be broken if the corresponding locking in another task
is holding the hb->lock of a global hash entry instead?

Sidenote
========
Maybe we should block CLONE_VFORK when current->vfork_done!=NULL ? It
is clearly bad to allow nested vfork() such that a SIGKILL can cause
two threads to run concurrently on the same userspace stack.

But I'm not sure if that's a good fix for these futex issues - a fix
that doesn't rely on such distant assumptions might be nicer...

Reproducer
==========
Tested at mainline commit 50d05c7c76c96b90462f24debacca971d2e86713,
with a build with options:

CONFIG_PREEMPT=y
CONFIG_RCU_STRICT_GRACE_PERIOD=y
CONFIG_DEBUG_ATOMIC_SLEEP=y

To test, patch delays into the kernel like this (also attached as
kernel-delay-patch.diff):
```
diff --git a/kernel/futex/core.c b/kernel/futex/core.c
index a061f54b606d..94ebd2c25b2a 100644
--- a/kernel/futex/core.c
+++ b/kernel/futex/core.c
@@ -47,6 +47,7 @@
 #include <linux/vmalloc.h>
 #include <linux/kmemleak.h>
 #include <linux/wait_bit.h>
+#include <linux/delay.h>

 #include <vdso/futex.h>

@@ -1359,6 +1360,7 @@ static void exit_pi_state_list(struct task_struct *curr)
         */
        WARN_ON(curr != current);
        guard(private_hash)(current->mm);
+       if (strcmp(current->comm, "SLOW-1") == 0) mdelay(1000);
        /*
         * We are a ZOMBIE and nobody can enqueue itself on
         * pi_state_list anymore, but we have to be careful
@@ -1945,6 +1947,7 @@ static int futex_hash_allocate(unsigned int
hash_slots, unsigned int flags)
                                 * reference on the existing hash.
                                 */
                                futex_ref_drop(cur);
+                               if (strcmp(current->comm, "SLOW-2") ==
0) mdelay(10000);
                        }

                        if (new) {
```

And run the following testcase:
```
#define _GNU_SOURCE
#include <err.h>
#include <sched.h>
#include <signal.h>
#include <stdio.h>
#include <stdlib.h>
#include <sys/mman.h>
#include <sys/prctl.h>
#include <sys/syscall.h>
#include <linux/futex.h>

static void *alloc_stack(void) {
  size_t stack_size = 1024*1024;
  void *p = mmap(NULL, stack_size, PROT_READ|PROT_WRITE,
MAP_ANONYMOUS|MAP_PRIVATE, -1, 0);
  if (p == MAP_FAILED)
    err(1, "stack mmap");
  if (madvise(p, 0x1000, MADV_GUARD_INSTALL))
    err(1, "stack madvise");
  return p + stack_size;
}

static pid_t child1, child1b;

static unsigned int futex_var;

int child_fn2(void *arg) {
  printf("child_fn2 entry, will kill %d\n", child1);

  syscall(__NR_futex, &futex_var, FUTEX_LOCK_PI|FUTEX_PRIVATE_FLAG, 0, NULL);
  if (kill(child1, SIGKILL))
    err(1, "kill");

  usleep(100000);
  prctl(PR_SET_NAME, "SLOW-1", 0, 0, 0);
  exit(0);
}
int child_fn1(void *arg) {
  child1 = getpid();
  if (clone(child_fn2, alloc_stack(),
CLONE_VM|CLONE_VFORK|CLONE_PARENT|SIGCHLD, NULL) == -1)
    err(1, "second clone");
  errx(1, "child_fn1 running after clone");
}

int child_fn2b(void *arg) {
  printf("child_fn2b entry, will kill %d\n", child1b);
  if (kill(child1b, SIGKILL))
    err(1, "kill");

  // this will block until the bug has been hit
  syscall(__NR_futex, &futex_var, FUTEX_LOCK_PI|FUTEX_PRIVATE_FLAG, 0, NULL);
  exit(0);
}

int child_fn1b(void *arg) {
  child1b = getpid();
  if (clone(child_fn2b, alloc_stack(),
CLONE_VM|CLONE_VFORK|CLONE_PARENT|SIGCHLD, NULL) == -1)
    err(1, "second clone");
  errx(1, "child_fn1b running after clone");
}

int main(void) {
  setbuf(stdout, NULL);
  if (clone(child_fn1, alloc_stack(), CLONE_VM|CLONE_VFORK|SIGCHLD, NULL) == -1)
    err(1, "first clone");
  if (clone(child_fn1b, alloc_stack(), CLONE_VM|CLONE_VFORK|SIGCHLD,
NULL) == -1)
    err(1, "first clone");
  printf("parent running after clone\n");
  usleep(500000);
  prctl(PR_FUTEX_HASH, PR_FUTEX_HASH_SET_SLOTS, 4, 0, 0);
  prctl(PR_SET_NAME, "SLOW-2", 0, 0, 0);
  prctl(PR_FUTEX_HASH, PR_FUTEX_HASH_SET_SLOTS, 8, 0, 0);
  return 0;
}
```

This gives me a "sleeping function called from invalid context"
warning, a "Invalid wait context" warning, and a
scheduling-while-atomic BUG:
```
sh-5.3# /host/h/test/nested_vfork/nested_vfork
child_fn2 entry, will kill 82
[...]
child_fn2b entry, will kill 84
parent running after clone
 BUG: sleeping function called from invalid context at
kernel/locking/mutex.c:623
 in_atomic(): 1, irqs_disabled(): 1, non_block: 0, pid: 83, name: SLOW-1
 preempt_count: 1, expected: 0
 RCU nest depth: 0, expected: 0
 locks held by SLOW-1/83: 2, last CPU#0:
  #0: ffff888101e73100 (&tsk->futex.exit_mutex){+.+.}-{4:4}, at:
futex_exit_exec_release+0x27/0x160
  #1: ffff888101e726f0 (&p->pi_lock){-.-.}-{2:2}, at:
exit_pi_state_list+0xd7/0x5a0
 irq event stamp: 3306
 hardirqs last  enabled at (3305): [<ffffffff838780bb>] __schedule+0x13eb/0x18d0
 hardirqs last disabled at (3306): [<ffffffff83886c47>]
_raw_spin_lock_irq+0x17/0x50
 softirqs last  enabled at (3294): [<ffffffff814b251d>]
__irq_exit_rcu+0xad/0x180
 softirqs last disabled at (3283): [<ffffffff814b251d>]
__irq_exit_rcu+0xad/0x180
 CPU: 0 UID: 0 PID: 83 Comm: SLOW-1 Tainted: G                 N
7.3.0-rc2-00099-g50d05c7c76c9-dirty #77 PREEMPT
 Tainted: [N]=TEST
 Call Trace:
  <TASK>
[...]
  __might_resched+0x290/0x2b0
  __might_sleep+0x54/0x90
[...]
  __mutex_lock+0x69/0x1180
[...]
  mutex_lock_nested+0x16/0x20
  futex_hash+0x2a4/0x310
[...]
  exit_pi_state_list+0x110/0x5a0
  futex_exit_exec_release+0x159/0x160
  mm_exit_exec_release+0x1e/0x1b0
  exit_mm+0x3b/0x200
  do_exit+0x43c/0x1390
  do_group_exit+0x139/0x140
[...]
  __x64_sys_exit_group+0x26/0x30
[...]
  </TASK>

 =============================
 [ BUG: Invalid wait context ]
 7.3.0-rc2-00099-g50d05c7c76c9-dirty #77 Tainted: G        W        N
 -----------------------------
 SLOW-1/83 is trying to lock:
 ffff888100098488 (&fd->phash.lock){+.+.}-{4:4}, at: futex_hash+0x2a4/0x310
 other info that might help us debug this:
 context-{5:5}
 locks held by SLOW-1/83: 2, last CPU#0:
  #0: ffff888101e73100 (&tsk->futex.exit_mutex){+.+.}-{4:4}, at:
futex_exit_exec_release+0x27/0x160
  #1: ffff888101e726f0 (&p->pi_lock){-.-.}-{2:2}, at:
exit_pi_state_list+0xd7/0x5a0
 stack backtrace:
 CPU: 0 UID: 0 PID: 83 Comm: SLOW-1 Tainted: G        W        N
7.3.0-rc2-00099-g50d05c7c76c9-dirty #77 PREEMPT
 Tainted: [W]=WARN, [N]=TEST
 Call Trace:
  <TASK>
[...]
  __lock_acquire+0xade/0x2c30
  lock_acquire+0xf1/0x210
[...]
  __mutex_lock+0xb7/0x1180
[...]
  mutex_lock_nested+0x16/0x20
  futex_hash+0x2a4/0x310
[...]
  exit_pi_state_list+0x110/0x5a0
[...]
  </TASK>
 BUG: scheduling while atomic: SLOW-1/83/0x00000002
 INFO: lockdep is turned off.
 irq event stamp: 3306
 hardirqs last  enabled at (3305): [<ffffffff838780bb>] __schedule+0x13eb/0x18d0
 hardirqs last disabled at (3306): [<ffffffff83886c47>]
_raw_spin_lock_irq+0x17/0x50
 softirqs last  enabled at (3294): [<ffffffff814b251d>]
__irq_exit_rcu+0xad/0x180
 softirqs last disabled at (3283): [<ffffffff814b251d>]
__irq_exit_rcu+0xad/0x180
 CPU: 0 UID: 0 PID: 83 Comm: SLOW-1 Tainted: G        W        N
7.3.0-rc2-00099-g50d05c7c76c9-dirty #77 PREEMPT
 Tainted: [W]=WARN, [N]=TEST
 Call Trace:
  <TASK>
  __dump_stack+0x21/0x30
  dump_stack_lvl+0x76/0xa0
  dump_stack+0x15/0x20
  __schedule_bug+0x83/0xa0
  __schedule+0x12a7/0x18d0
[...]
  schedule+0xf9/0x160
[...]
  schedule_preempt_disabled+0x11/0x20
  __mutex_lock+0x2ba/0x1180
[...]
  mutex_lock_nested+0x16/0x20
  futex_hash+0x2a4/0x310
[...]
  exit_pi_state_list+0x110/0x5a0
[...]
  </TASK>
```

[-- Attachment #2: kernel-delay-patch.diff --]
[-- Type: application/x-patch, Size: 884 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2026-09-11 17:51 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-10 15:27 [BUG] futex: scheduling-while-atomic because nested vfork can break guard(private_hash) Jann Horn
2026-09-11  8:36 ` Peter Zijlstra
2026-09-11  9:04   ` Peter Zijlstra
2026-09-11 11:51     ` Thomas Gleixner
2026-09-11 15:28       ` Jann Horn
2026-09-11 17:51         ` Davidlohr Bueso
2026-09-11 15:31   ` Jann Horn

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®