* [PATCH] futex: Fix might_sleep() warning in futex_pivot_pending()
@ 2026-08-13 6:50 syzbot
2026-08-14 13:38 ` Peter Zijlstra
0 siblings, 1 reply; 5+ messages in thread
From: syzbot @ 2026-08-13 6:50 UTC (permalink / raw)
To: syzkaller-bugs, Yao Kai, linux-kernel, Ingo Molnar,
Thomas Gleixner
Cc: andrealmeid, dave, dvhart, liuyongqiang13, peterz, syzbot
From: Yao Kai <yaokai34@huawei.com>
A recent change modified futex_pivot_pending() to acquire a mutex to fix a
race condition. However, futex_pivot_pending() is evaluated as a condition
inside wait_var_event() in futex_hash_allocate(). Since wait_var_event()
sets the task state to TASK_UNINTERRUPTIBLE before evaluating the
condition, calling a blocking operation like mutex_lock() is invalid and
triggers a might_sleep() warning:
do not call blocking ops when !TASK_RUNNING; state=2 set at
[<ffffffff819e8c8d>] prepare_to_wait_event+0x3dd/0x480
kernel/sched/wait.c:317
WARNING: kernel/sched/core.c:9124 at __might_sleep+0x92/0xf0
kernel/sched/core.c:9120
Call Trace:
<TASK>
__mutex_lock_common kernel/locking/mutex.c:623 [inline]
__mutex_lock+0x118/0x1550 kernel/locking/mutex.c:821
class_mutex_constructor include/linux/mutex.h:253 [inline]
futex_pivot_pending kernel/futex/core.c:1789 [inline]
futex_hash_allocate+0x7fb/0xf00 kernel/futex/core.c:1872
__do_sys_prctl kernel/sys.c:2885 [inline]
__se_sys_prctl+0x78c/0x1910 kernel/sys.c:2534
Fix this by reverting futex_pivot_pending() to a lockless implementation
using RCU and memory barriers, which is the idiomatic way to handle
conditions in wait_event loops. By reading the hash pointer first,
executing an smp_rmb() memory barrier, and then reading hash_new, we
leverage the Message Passing (MP) pattern to guarantee correctness without
blocking. This pairs with the rcu_assign_pointer() release barrier in
__futex_pivot_hash(). If the reader sees the new hash, it is guaranteed to
see the cleared hash_new and correctly return true. If the reader sees the
old hash, it will check futex_ref_is_dead(old), which will return true if
the writer has already completed the pivot. The old hash memory is
guaranteed to remain valid for the duration of the check in
futex_ref_is_dead() because futex_pivot_pending() executes within an RCU
read-side critical section and the old hash is freed using kvfree_rcu().
Fixes: 8e7ff730dd96 ("futex: Fix race in futex_pivot_pending() during private hash resize")
Assisted-by: Gemini:gemini-3.6-flash Gemini:gemini-3.1-pro-preview syzbot
Reported-by: syzbot+350a93852ac854927f45@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=350a93852ac854927f45
Link: https://syzkaller.appspot.com/ai_job?id=29771462-e030-4501-832d-adbf8cb167c2
Signed-off-by: Yao Kai <yaokai34@huawei.com>
---
diff --git a/kernel/futex/core.c b/kernel/futex/core.c
index 128c5752f..e84be5410 100644
--- a/kernel/futex/core.c
+++ b/kernel/futex/core.c
@@ -202,7 +202,7 @@ static bool __futex_pivot_hash(struct mm_struct *mm, struct futex_private_hash *
fph = rcu_dereference_protected(mmph->hash, lockdep_is_held(&mmph->lock));
if (fph) {
if (!futex_ref_is_dead(fph)) {
- mmph->hash_new = new;
+ WRITE_ONCE(mmph->hash_new, new);
return false;
}
@@ -224,7 +224,7 @@ static void futex_pivot_hash(struct mm_struct *mm)
fph = mm->futex.phash.hash_new;
if (fph) {
- mm->futex.phash.hash_new = NULL;
+ WRITE_ONCE(mm->futex.phash.hash_new, NULL);
__futex_pivot_hash(mm, fph);
}
}
@@ -1786,12 +1786,18 @@ static bool futex_pivot_pending(struct mm_struct *mm)
struct futex_mm_phash *mmph = &mm->futex.phash;
struct futex_private_hash *fph;
- guard(mutex)(&mmph->lock);
+ guard(rcu)();
- if (!mmph->hash_new)
+ fph = rcu_dereference(mmph->hash);
+ /*
+ * Ensure that if we see the new hash, we will also see the cleared
+ * hash_new pointer. Pairs with rcu_assign_pointer() in
+ * __futex_pivot_hash().
+ */
+ smp_rmb();
+ if (!READ_ONCE(mmph->hash_new))
return true;
- fph = rcu_dereference_raw(mmph->hash);
return futex_ref_is_dead(fph);
}
@@ -1879,7 +1885,7 @@ static int futex_hash_allocate(unsigned int hash_slots, unsigned int flags)
cur = rcu_dereference_protected(mm->futex.phash.hash,
lockdep_is_held(&mm->futex.phash.lock));
new = mm->futex.phash.hash_new;
- mm->futex.phash.hash_new = NULL;
+ WRITE_ONCE(mm->futex.phash.hash_new, NULL);
if (fph) {
if (cur && !cur->hash_mask) {
@@ -1889,7 +1895,7 @@ static int futex_hash_allocate(unsigned int hash_slots, unsigned int flags)
* the second one returns here.
*/
free = fph;
- mm->futex.phash.hash_new = new;
+ WRITE_ONCE(mm->futex.phash.hash_new, new);
return -EBUSY;
}
if (cur && !new) {
base-commit: db2ddb87143519e20a95aa36c60b36107b736a58
--
See https://goo.gle/syzbot-ai-patches for information about AI-generated patches.
The person who has signed off on the patch is responsible for
addressing comments.
syzbot engineers can be reached at syzkaller@googlegroups.com.
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] futex: Fix might_sleep() warning in futex_pivot_pending() 2026-08-13 6:50 [PATCH] futex: Fix might_sleep() warning in futex_pivot_pending() syzbot @ 2026-08-14 13:38 ` Peter Zijlstra 2026-08-17 7:29 ` Yao Kai 0 siblings, 1 reply; 5+ messages in thread From: Peter Zijlstra @ 2026-08-14 13:38 UTC (permalink / raw) To: syzbot Cc: syzkaller-bugs, Yao Kai, linux-kernel, Ingo Molnar, Thomas Gleixner, andrealmeid, dave, dvhart, liuyongqiang13, syzbot On Thu, Aug 13, 2026 at 06:50:49AM +0000, syzbot wrote: > From: Yao Kai <yaokai34@huawei.com> > > A recent change modified futex_pivot_pending() to acquire a mutex to fix a > race condition. However, futex_pivot_pending() is evaluated as a condition > inside wait_var_event() in futex_hash_allocate(). Since wait_var_event() > sets the task state to TASK_UNINTERRUPTIBLE before evaluating the > condition, calling a blocking operation like mutex_lock() is invalid and > triggers a might_sleep() warning: > > do not call blocking ops when !TASK_RUNNING; state=2 set at > [<ffffffff819e8c8d>] prepare_to_wait_event+0x3dd/0x480 > kernel/sched/wait.c:317 > WARNING: kernel/sched/core.c:9124 at __might_sleep+0x92/0xf0 > kernel/sched/core.c:9120 > Call Trace: > <TASK> > __mutex_lock_common kernel/locking/mutex.c:623 [inline] > __mutex_lock+0x118/0x1550 kernel/locking/mutex.c:821 > class_mutex_constructor include/linux/mutex.h:253 [inline] > futex_pivot_pending kernel/futex/core.c:1789 [inline] > futex_hash_allocate+0x7fb/0xf00 kernel/futex/core.c:1872 > __do_sys_prctl kernel/sys.c:2885 [inline] > __se_sys_prctl+0x78c/0x1910 kernel/sys.c:2534 > > Fix this by reverting futex_pivot_pending() to a lockless implementation > using RCU and memory barriers, which is the idiomatic way to handle > conditions in wait_event loops. By reading the hash pointer first, > executing an smp_rmb() memory barrier, and then reading hash_new, we > leverage the Message Passing (MP) pattern to guarantee correctness without > blocking. This pairs with the rcu_assign_pointer() release barrier in > __futex_pivot_hash(). If the reader sees the new hash, it is guaranteed to > see the cleared hash_new and correctly return true. If the reader sees the > old hash, it will check futex_ref_is_dead(old), which will return true if > the writer has already completed the pivot. The old hash memory is > guaranteed to remain valid for the duration of the check in > futex_ref_is_dead() because futex_pivot_pending() executes within an RCU > read-side critical section and the old hash is freed using kvfree_rcu(). > > Fixes: 8e7ff730dd96 ("futex: Fix race in futex_pivot_pending() during private hash resize") > Assisted-by: Gemini:gemini-3.6-flash Gemini:gemini-3.1-pro-preview syzbot > Reported-by: syzbot+350a93852ac854927f45@syzkaller.appspotmail.com > Closes: https://syzkaller.appspot.com/bug?extid=350a93852ac854927f45 > Link: https://syzkaller.appspot.com/ai_job?id=29771462-e030-4501-832d-adbf8cb167c2 > Signed-off-by: Yao Kai <yaokai34@huawei.com> Bah. So a younger me added that might_sleep() test, because yes, sleeping from a wait loop is dodgy. It mostly works in this case, but bah. I also build an alternative wait look scheme it seems, but we don't have nice helpers for that, and its never been applied to bit/var waits. I've hacked up the below. Its not exactly what I call nice, but it compiles, so it must be perfect... right? --- diff --git a/include/linux/wait.h b/include/linux/wait.h index dce055e6add3..7e215330199c 100644 --- a/include/linux/wait.h +++ b/include/linux/wait.h @@ -1228,6 +1228,7 @@ long prepare_to_wait_event(struct wait_queue_head *wq_head, struct wait_queue_en void finish_wait(struct wait_queue_head *wq_head, struct wait_queue_entry *wq_entry); long wait_woken(struct wait_queue_entry *wq_entry, unsigned mode, long timeout); int woken_wake_function(struct wait_queue_entry *wq_entry, unsigned mode, int sync, void *key); +int woken_wake_bit_function(struct wait_queue_entry *wq_entry, unsigned mode, int sync, void *key); int autoremove_wake_function(struct wait_queue_entry *wq_entry, unsigned mode, int sync, void *key); #define DEFINE_WAIT_FUNC(name, function) \ diff --git a/include/linux/wait_bit.h b/include/linux/wait_bit.h index ace7379d627d..553d7b23e3ad 100644 --- a/include/linux/wait_bit.h +++ b/include/linux/wait_bit.h @@ -32,6 +32,7 @@ int out_of_line_wait_on_bit_timeout(unsigned long *word, int, wait_bit_action_f int out_of_line_wait_on_bit_lock(unsigned long *word, int, wait_bit_action_f *action, unsigned int mode); struct wait_queue_head *bit_waitqueue(unsigned long *word, int bit); extern void __init wait_bit_init(void); +extern struct wait_bit_key *__var_wake_key(struct wait_queue_entry *wq_entry, void *arg); int wake_bit_function(struct wait_queue_entry *wq_entry, unsigned mode, int sync, void *key); diff --git a/kernel/futex/core.c b/kernel/futex/core.c index a7c2a6242718..d3311047d259 100644 --- a/kernel/futex/core.c +++ b/kernel/futex/core.c @@ -46,6 +46,7 @@ #include <linux/slab.h> #include <linux/vmalloc.h> #include <linux/kmemleak.h> +#include <linux/wait_bit.h> #include <vdso/futex.h> @@ -1886,11 +1887,24 @@ static int futex_hash_allocate(unsigned int hash_slots, unsigned int flags) futex_hash_bucket_init(&fph->queues[i]); if (custom) { + struct wait_bit_queue_entry __wbq_entry; + struct wait_queue_head *__wq_head; + /* * Only let prctl() wait / retry; don't unduly delay clone(). */ again: - wait_var_event(mm, futex_pivot_pending(mm)); + __wq_head = __var_waitqueue(mm); + init_wait_var_entry(&__wbq_entry, mm, 0); + __wbq_entry.wq_entry.func = woken_wake_bit_function; + add_wait_queue(__wq_head, &__wbq_entry.wq_entry); + while (!futex_pivot_pending(mm)) { + int rc = wait_woken(&__wbq_entry.wq_entry, TASK_UNINTERRUPTIBLE, + MAX_SCHEDULE_TIMEOUT); + if (!rc) + break; + } + remove_wait_queue(__wq_head, &__wbq_entry.wq_entry); } scoped_guard(mutex, &mm->futex.phash.lock) { diff --git a/kernel/sched/wait.c b/kernel/sched/wait.c index 20f27e2cf7ae..d033f600f48c 100644 --- a/kernel/sched/wait.c +++ b/kernel/sched/wait.c @@ -5,6 +5,7 @@ * (C) 2004 Nadia Yvette Chambers, Oracle */ #include "sched.h" +#include <linux/wait_bit.h> void __init_waitqueue_head(struct wait_queue_head *wq_head, const char *name, struct lock_class_key *key) { @@ -463,3 +464,17 @@ int woken_wake_function(struct wait_queue_entry *wq_entry, unsigned mode, int sy return default_wake_function(wq_entry, mode, sync, key); } EXPORT_SYMBOL(woken_wake_function); + +int woken_wake_bit_function(struct wait_queue_entry *wq_entry, unsigned mode, int sync, void *arg) +{ + struct wait_bit_key *key = __var_wake_key(wq_entry, arg); + if (!key) + return 0; + + /* Pairs with the smp_store_mb() in wait_woken(). */ + smp_mb(); /* C */ + wq_entry->flags |= WQ_FLAG_WOKEN; + + return default_wake_function(wq_entry, mode, sync, key); +} +EXPORT_SYMBOL(woken_wake_bit_function); diff --git a/kernel/sched/wait_bit.c b/kernel/sched/wait_bit.c index 1088d3b7012c..e8127e83a48f 100644 --- a/kernel/sched/wait_bit.c +++ b/kernel/sched/wait_bit.c @@ -167,9 +167,7 @@ wait_queue_head_t *__var_waitqueue(void *p) } EXPORT_SYMBOL(__var_waitqueue); -static int -var_wake_function(struct wait_queue_entry *wq_entry, unsigned int mode, - int sync, void *arg) +struct wait_bit_key *__var_wake_key(struct wait_queue_entry *wq_entry, void *arg) { struct wait_bit_key *key = arg; struct wait_bit_queue_entry *wbq_entry = @@ -177,6 +175,17 @@ var_wake_function(struct wait_queue_entry *wq_entry, unsigned int mode, if (wbq_entry->key.flags != key->flags || wbq_entry->key.bit_nr != key->bit_nr) + return NULL; + + return key; +} + +static int +var_wake_function(struct wait_queue_entry *wq_entry, unsigned int mode, + int sync, void *arg) +{ + struct wait_bit_key *key = __var_wake_key(wq_entry, arg); + if (!key) return 0; return autoremove_wake_function(wq_entry, mode, sync, key); ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] futex: Fix might_sleep() warning in futex_pivot_pending() 2026-08-14 13:38 ` Peter Zijlstra @ 2026-08-17 7:29 ` Yao Kai 2026-08-18 10:46 ` Peter Zijlstra 0 siblings, 1 reply; 5+ messages in thread From: Yao Kai @ 2026-08-17 7:29 UTC (permalink / raw) To: Peter Zijlstra, syzbot Cc: syzkaller-bugs, linux-kernel, Ingo Molnar, Thomas Gleixner, andrealmeid, dave, dvhart, liuyongqiang13, syzbot On 8/14/2026 9:38 PM, Peter Zijlstra wrote: > On Thu, Aug 13, 2026 at 06:50:49AM +0000, syzbot wrote: >> From: Yao Kai <yaokai34@huawei.com> >> >> A recent change modified futex_pivot_pending() to acquire a mutex to fix a >> race condition. However, futex_pivot_pending() is evaluated as a condition >> inside wait_var_event() in futex_hash_allocate(). Since wait_var_event() >> sets the task state to TASK_UNINTERRUPTIBLE before evaluating the >> condition, calling a blocking operation like mutex_lock() is invalid and >> triggers a might_sleep() warning: >> >> do not call blocking ops when !TASK_RUNNING; state=2 set at >> [<ffffffff819e8c8d>] prepare_to_wait_event+0x3dd/0x480 >> kernel/sched/wait.c:317 >> WARNING: kernel/sched/core.c:9124 at __might_sleep+0x92/0xf0 >> kernel/sched/core.c:9120 >> Call Trace: >> <TASK> >> __mutex_lock_common kernel/locking/mutex.c:623 [inline] >> __mutex_lock+0x118/0x1550 kernel/locking/mutex.c:821 >> class_mutex_constructor include/linux/mutex.h:253 [inline] >> futex_pivot_pending kernel/futex/core.c:1789 [inline] >> futex_hash_allocate+0x7fb/0xf00 kernel/futex/core.c:1872 >> __do_sys_prctl kernel/sys.c:2885 [inline] >> __se_sys_prctl+0x78c/0x1910 kernel/sys.c:2534 >> >> Fix this by reverting futex_pivot_pending() to a lockless implementation >> using RCU and memory barriers, which is the idiomatic way to handle >> conditions in wait_event loops. By reading the hash pointer first, >> executing an smp_rmb() memory barrier, and then reading hash_new, we >> leverage the Message Passing (MP) pattern to guarantee correctness without >> blocking. This pairs with the rcu_assign_pointer() release barrier in >> __futex_pivot_hash(). If the reader sees the new hash, it is guaranteed to >> see the cleared hash_new and correctly return true. If the reader sees the >> old hash, it will check futex_ref_is_dead(old), which will return true if >> the writer has already completed the pivot. The old hash memory is >> guaranteed to remain valid for the duration of the check in >> futex_ref_is_dead() because futex_pivot_pending() executes within an RCU >> read-side critical section and the old hash is freed using kvfree_rcu(). >> >> Fixes: 8e7ff730dd96 ("futex: Fix race in futex_pivot_pending() during private hash resize") >> Assisted-by: Gemini:gemini-3.6-flash Gemini:gemini-3.1-pro-preview syzbot >> Reported-by: syzbot+350a93852ac854927f45@syzkaller.appspotmail.com >> Closes: https://syzkaller.appspot.com/bug?extid=350a93852ac854927f45 >> Link: https://syzkaller.appspot.com/ai_job?id=29771462-e030-4501-832d-adbf8cb167c2 >> Signed-off-by: Yao Kai <yaokai34@huawei.com> > > Bah. So a younger me added that might_sleep() test, because yes, > sleeping from a wait loop is dodgy. It mostly works in this case, but > bah. > > I also build an alternative wait look scheme it seems, but we don't have > nice helpers for that, and its never been applied to bit/var waits. > > I've hacked up the below. Its not exactly what I call nice, but it > compiles, so it must be perfect... right? > > --- > diff --git a/include/linux/wait.h b/include/linux/wait.h > index dce055e6add3..7e215330199c 100644 > --- a/include/linux/wait.h > +++ b/include/linux/wait.h > @@ -1228,6 +1228,7 @@ long prepare_to_wait_event(struct wait_queue_head *wq_head, struct wait_queue_en > void finish_wait(struct wait_queue_head *wq_head, struct wait_queue_entry *wq_entry); > long wait_woken(struct wait_queue_entry *wq_entry, unsigned mode, long timeout); > int woken_wake_function(struct wait_queue_entry *wq_entry, unsigned mode, int sync, void *key); > +int woken_wake_bit_function(struct wait_queue_entry *wq_entry, unsigned mode, int sync, void *key); > int autoremove_wake_function(struct wait_queue_entry *wq_entry, unsigned mode, int sync, void *key); > > #define DEFINE_WAIT_FUNC(name, function) \ > diff --git a/include/linux/wait_bit.h b/include/linux/wait_bit.h > index ace7379d627d..553d7b23e3ad 100644 > --- a/include/linux/wait_bit.h > +++ b/include/linux/wait_bit.h > @@ -32,6 +32,7 @@ int out_of_line_wait_on_bit_timeout(unsigned long *word, int, wait_bit_action_f > int out_of_line_wait_on_bit_lock(unsigned long *word, int, wait_bit_action_f *action, unsigned int mode); > struct wait_queue_head *bit_waitqueue(unsigned long *word, int bit); > extern void __init wait_bit_init(void); > +extern struct wait_bit_key *__var_wake_key(struct wait_queue_entry *wq_entry, void *arg); > > int wake_bit_function(struct wait_queue_entry *wq_entry, unsigned mode, int sync, void *key); > > diff --git a/kernel/futex/core.c b/kernel/futex/core.c > index a7c2a6242718..d3311047d259 100644 > --- a/kernel/futex/core.c > +++ b/kernel/futex/core.c > @@ -46,6 +46,7 @@ > #include <linux/slab.h> > #include <linux/vmalloc.h> > #include <linux/kmemleak.h> > +#include <linux/wait_bit.h> > > #include <vdso/futex.h> > > @@ -1886,11 +1887,24 @@ static int futex_hash_allocate(unsigned int hash_slots, unsigned int flags) > futex_hash_bucket_init(&fph->queues[i]); > > if (custom) { > + struct wait_bit_queue_entry __wbq_entry; > + struct wait_queue_head *__wq_head; > + > /* > * Only let prctl() wait / retry; don't unduly delay clone(). > */ > again: > - wait_var_event(mm, futex_pivot_pending(mm)); > + __wq_head = __var_waitqueue(mm); > + init_wait_var_entry(&__wbq_entry, mm, 0); > + __wbq_entry.wq_entry.func = woken_wake_bit_function; > + add_wait_queue(__wq_head, &__wbq_entry.wq_entry); > + while (!futex_pivot_pending(mm)) { > + int rc = wait_woken(&__wbq_entry.wq_entry, TASK_UNINTERRUPTIBLE, > + MAX_SCHEDULE_TIMEOUT); > + if (!rc) > + break; > + } > + remove_wait_queue(__wq_head, &__wbq_entry.wq_entry); > } > > scoped_guard(mutex, &mm->futex.phash.lock) { > diff --git a/kernel/sched/wait.c b/kernel/sched/wait.c > index 20f27e2cf7ae..d033f600f48c 100644 > --- a/kernel/sched/wait.c > +++ b/kernel/sched/wait.c > @@ -5,6 +5,7 @@ > * (C) 2004 Nadia Yvette Chambers, Oracle > */ > #include "sched.h" > +#include <linux/wait_bit.h> > > void __init_waitqueue_head(struct wait_queue_head *wq_head, const char *name, struct lock_class_key *key) > { > @@ -463,3 +464,17 @@ int woken_wake_function(struct wait_queue_entry *wq_entry, unsigned mode, int sy > return default_wake_function(wq_entry, mode, sync, key); > } > EXPORT_SYMBOL(woken_wake_function); > + > +int woken_wake_bit_function(struct wait_queue_entry *wq_entry, unsigned mode, int sync, void *arg) > +{ > + struct wait_bit_key *key = __var_wake_key(wq_entry, arg); > + if (!key) > + return 0; > + > + /* Pairs with the smp_store_mb() in wait_woken(). */ > + smp_mb(); /* C */ > + wq_entry->flags |= WQ_FLAG_WOKEN; > + > + return default_wake_function(wq_entry, mode, sync, key); > +} > +EXPORT_SYMBOL(woken_wake_bit_function); > diff --git a/kernel/sched/wait_bit.c b/kernel/sched/wait_bit.c > index 1088d3b7012c..e8127e83a48f 100644 > --- a/kernel/sched/wait_bit.c > +++ b/kernel/sched/wait_bit.c > @@ -167,9 +167,7 @@ wait_queue_head_t *__var_waitqueue(void *p) > } > EXPORT_SYMBOL(__var_waitqueue); > > -static int > -var_wake_function(struct wait_queue_entry *wq_entry, unsigned int mode, > - int sync, void *arg) > +struct wait_bit_key *__var_wake_key(struct wait_queue_entry *wq_entry, void *arg) > { > struct wait_bit_key *key = arg; > struct wait_bit_queue_entry *wbq_entry = > @@ -177,6 +175,17 @@ var_wake_function(struct wait_queue_entry *wq_entry, unsigned int mode, > > if (wbq_entry->key.flags != key->flags || > wbq_entry->key.bit_nr != key->bit_nr) > + return NULL; > + > + return key; > +} > + > +static int > +var_wake_function(struct wait_queue_entry *wq_entry, unsigned int mode, > + int sync, void *arg) > +{ > + struct wait_bit_key *key = __var_wake_key(wq_entry, arg); > + if (!key) > return 0; > > return autoremove_wake_function(wq_entry, mode, sync, key); Thanks! I think there is still a lost-wakeup window: T1 T2 add_wait_queue() /* not visible to T2 */ futex_pivot_pending() futex_ref_is_dead() = false futex_ref_put() = true wake_up_var() waitqueue_active() = false /* observes empty */ return wait_woken() schedule() Since wake_up_var() uses a lockless waitqueue_active() check, I think we need to order the waitqueue insertion before the first condition check: add_wait_queue(__wq_head, &__wbq_entry.wq_entry); /* * Pairs with the fully ordered refcount operation before wake_up_var(). * Ensures either the waker sees this waiter or we see the dead refcount. */ smp_mb(); while (!futex_pivot_pending(mm)) wait_woken(&__wbq_entry.wq_entry, TASK_UNINTERRUPTIBLE, MAX_SCHEDULE_TIMEOUT); The rc check can be dropped because MAX_SCHEDULE_TIMEOUT does not expire. Thanks, Yao Kai ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] futex: Fix might_sleep() warning in futex_pivot_pending() 2026-08-17 7:29 ` Yao Kai @ 2026-08-18 10:46 ` Peter Zijlstra 2026-08-18 12:24 ` Yao Kai 0 siblings, 1 reply; 5+ messages in thread From: Peter Zijlstra @ 2026-08-18 10:46 UTC (permalink / raw) To: Yao Kai Cc: syzbot, syzkaller-bugs, linux-kernel, Ingo Molnar, Thomas Gleixner, andrealmeid, dave, dvhart, liuyongqiang13, syzbot On Mon, Aug 17, 2026 at 03:29:14PM +0800, Yao Kai wrote: > Thanks! I think there is still a lost-wakeup window: > > T1 T2 > > add_wait_queue() > /* not visible to T2 */ > futex_pivot_pending() > futex_ref_is_dead() = false > futex_ref_put() = true > wake_up_var() > waitqueue_active() = false > /* observes empty */ > return > wait_woken() > schedule() > > Since wake_up_var() uses a lockless waitqueue_active() check, I think > we need to order the waitqueue insertion before the first condition > check: > > add_wait_queue(__wq_head, &__wbq_entry.wq_entry); > > /* > * Pairs with the fully ordered refcount operation before wake_up_var(). > * Ensures either the waker sees this waiter or we see the dead refcount. > */ > smp_mb(); Well, add_wait_queue() has UNLOCK(&wq_head->lock) and futex_pivot_pending() has LOCK(&mmph->lock), giving an UNLOCK+LOCK consistency, which IIRC is RCtso if you're on PowerPC and RCsc everywhere else. So yeah, this needs more. But I would instead suggest we use: smp_mb__after_spinlock(). Anyway, for this to matter one way or the other, the other side of this also needs a barrier. But it looks like futex_ref_put() already implies enough. When in atomic mode it implies a full smp_mb(). > while (!futex_pivot_pending(mm)) > wait_woken(&__wbq_entry.wq_entry, TASK_UNINTERRUPTIBLE, > MAX_SCHEDULE_TIMEOUT); > > The rc check can be dropped because MAX_SCHEDULE_TIMEOUT does not expire. Indeed, I had realized this after sending :-) Something like so then? --- diff --git a/include/linux/wait.h b/include/linux/wait.h index dce055e6add3..7e215330199c 100644 --- a/include/linux/wait.h +++ b/include/linux/wait.h @@ -1228,6 +1228,7 @@ long prepare_to_wait_event(struct wait_queue_head *wq_head, struct wait_queue_en void finish_wait(struct wait_queue_head *wq_head, struct wait_queue_entry *wq_entry); long wait_woken(struct wait_queue_entry *wq_entry, unsigned mode, long timeout); int woken_wake_function(struct wait_queue_entry *wq_entry, unsigned mode, int sync, void *key); +int woken_wake_bit_function(struct wait_queue_entry *wq_entry, unsigned mode, int sync, void *key); int autoremove_wake_function(struct wait_queue_entry *wq_entry, unsigned mode, int sync, void *key); #define DEFINE_WAIT_FUNC(name, function) \ diff --git a/include/linux/wait_bit.h b/include/linux/wait_bit.h index ace7379d627d..553d7b23e3ad 100644 --- a/include/linux/wait_bit.h +++ b/include/linux/wait_bit.h @@ -32,6 +32,7 @@ int out_of_line_wait_on_bit_timeout(unsigned long *word, int, wait_bit_action_f int out_of_line_wait_on_bit_lock(unsigned long *word, int, wait_bit_action_f *action, unsigned int mode); struct wait_queue_head *bit_waitqueue(unsigned long *word, int bit); extern void __init wait_bit_init(void); +extern struct wait_bit_key *__var_wake_key(struct wait_queue_entry *wq_entry, void *arg); int wake_bit_function(struct wait_queue_entry *wq_entry, unsigned mode, int sync, void *key); diff --git a/kernel/futex/core.c b/kernel/futex/core.c index a7c2a6242718..bd9fb0b17ee6 100644 --- a/kernel/futex/core.c +++ b/kernel/futex/core.c @@ -46,6 +46,7 @@ #include <linux/slab.h> #include <linux/vmalloc.h> #include <linux/kmemleak.h> +#include <linux/wait_bit.h> #include <vdso/futex.h> @@ -1886,11 +1887,34 @@ static int futex_hash_allocate(unsigned int hash_slots, unsigned int flags) futex_hash_bucket_init(&fph->queues[i]); if (custom) { + struct wait_bit_queue_entry __wbq_entry; + struct wait_queue_head *__wq_head; + /* * Only let prctl() wait / retry; don't unduly delay clone(). */ again: - wait_var_event(mm, futex_pivot_pending(mm)); + __wq_head = __var_waitqueue(mm); + init_wait_var_entry(&__wbq_entry, mm, 0); + __wbq_entry.wq_entry.func = woken_wake_bit_function; + add_wait_queue(__wq_head, &__wbq_entry.wq_entry); + + /* + * add_wait_queue() futex_ref_put() + * MB (this) MB (implied) + * futex_pivot_pending() wake_up_var() + * waitqueue_active() + * + * Notably, it must not be possible to see + * !futex_pivot_pending() && !waitqueue_active(). + */ + smp_mb__after_spinlock(); + + while (!futex_pivot_pending(mm) && + wait_woken(&__wbq_entry.wq_entry, TASK_UNINTERRUPTIBLE, + MAX_SCHEDULE_TIMEOUT)) + /* empty */; + remove_wait_queue(__wq_head, &__wbq_entry.wq_entry); } scoped_guard(mutex, &mm->futex.phash.lock) { diff --git a/kernel/sched/wait.c b/kernel/sched/wait.c index 20f27e2cf7ae..d033f600f48c 100644 --- a/kernel/sched/wait.c +++ b/kernel/sched/wait.c @@ -5,6 +5,7 @@ * (C) 2004 Nadia Yvette Chambers, Oracle */ #include "sched.h" +#include <linux/wait_bit.h> void __init_waitqueue_head(struct wait_queue_head *wq_head, const char *name, struct lock_class_key *key) { @@ -463,3 +464,17 @@ int woken_wake_function(struct wait_queue_entry *wq_entry, unsigned mode, int sy return default_wake_function(wq_entry, mode, sync, key); } EXPORT_SYMBOL(woken_wake_function); + +int woken_wake_bit_function(struct wait_queue_entry *wq_entry, unsigned mode, int sync, void *arg) +{ + struct wait_bit_key *key = __var_wake_key(wq_entry, arg); + if (!key) + return 0; + + /* Pairs with the smp_store_mb() in wait_woken(). */ + smp_mb(); /* C */ + wq_entry->flags |= WQ_FLAG_WOKEN; + + return default_wake_function(wq_entry, mode, sync, key); +} +EXPORT_SYMBOL(woken_wake_bit_function); diff --git a/kernel/sched/wait_bit.c b/kernel/sched/wait_bit.c index 1088d3b7012c..e8127e83a48f 100644 --- a/kernel/sched/wait_bit.c +++ b/kernel/sched/wait_bit.c @@ -167,9 +167,7 @@ wait_queue_head_t *__var_waitqueue(void *p) } EXPORT_SYMBOL(__var_waitqueue); -static int -var_wake_function(struct wait_queue_entry *wq_entry, unsigned int mode, - int sync, void *arg) +struct wait_bit_key *__var_wake_key(struct wait_queue_entry *wq_entry, void *arg) { struct wait_bit_key *key = arg; struct wait_bit_queue_entry *wbq_entry = @@ -177,6 +175,17 @@ var_wake_function(struct wait_queue_entry *wq_entry, unsigned int mode, if (wbq_entry->key.flags != key->flags || wbq_entry->key.bit_nr != key->bit_nr) + return NULL; + + return key; +} + +static int +var_wake_function(struct wait_queue_entry *wq_entry, unsigned int mode, + int sync, void *arg) +{ + struct wait_bit_key *key = __var_wake_key(wq_entry, arg); + if (!key) return 0; return autoremove_wake_function(wq_entry, mode, sync, key); ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] futex: Fix might_sleep() warning in futex_pivot_pending() 2026-08-18 10:46 ` Peter Zijlstra @ 2026-08-18 12:24 ` Yao Kai 0 siblings, 0 replies; 5+ messages in thread From: Yao Kai @ 2026-08-18 12:24 UTC (permalink / raw) To: Peter Zijlstra Cc: syzbot, syzkaller-bugs, linux-kernel, Ingo Molnar, Thomas Gleixner, andrealmeid, dave, dvhart, liuyongqiang13, syzbot On 8/18/2026 6:46 PM, Peter Zijlstra wrote: > On Mon, Aug 17, 2026 at 03:29:14PM +0800, Yao Kai wrote: >> Thanks! I think there is still a lost-wakeup window: >> >> T1 T2 >> >> add_wait_queue() >> /* not visible to T2 */ >> futex_pivot_pending() >> futex_ref_is_dead() = false >> futex_ref_put() = true >> wake_up_var() >> waitqueue_active() = false >> /* observes empty */ >> return >> wait_woken() >> schedule() >> >> Since wake_up_var() uses a lockless waitqueue_active() check, I think >> we need to order the waitqueue insertion before the first condition >> check: >> >> add_wait_queue(__wq_head, &__wbq_entry.wq_entry); >> >> /* >> * Pairs with the fully ordered refcount operation before wake_up_var(). >> * Ensures either the waker sees this waiter or we see the dead refcount. >> */ >> smp_mb(); > > Well, add_wait_queue() has UNLOCK(&wq_head->lock) and > futex_pivot_pending() has LOCK(&mmph->lock), giving an UNLOCK+LOCK > consistency, which IIRC is RCtso if you're on PowerPC and RCsc > everywhere else. > > So yeah, this needs more. But I would instead suggest we use: > > smp_mb__after_spinlock(). > > Anyway, for this to matter one way or the other, the other side of this > also needs a barrier. But it looks like futex_ref_put() already implies > enough. When in atomic mode it implies a full smp_mb(). > >> while (!futex_pivot_pending(mm)) >> wait_woken(&__wbq_entry.wq_entry, TASK_UNINTERRUPTIBLE, >> MAX_SCHEDULE_TIMEOUT); >> >> The rc check can be dropped because MAX_SCHEDULE_TIMEOUT does not expire. > > Indeed, I had realized this after sending :-) > > Something like so then? > > --- > diff --git a/include/linux/wait.h b/include/linux/wait.h > index dce055e6add3..7e215330199c 100644 > --- a/include/linux/wait.h > +++ b/include/linux/wait.h > @@ -1228,6 +1228,7 @@ long prepare_to_wait_event(struct wait_queue_head *wq_head, struct wait_queue_en > void finish_wait(struct wait_queue_head *wq_head, struct wait_queue_entry *wq_entry); > long wait_woken(struct wait_queue_entry *wq_entry, unsigned mode, long timeout); > int woken_wake_function(struct wait_queue_entry *wq_entry, unsigned mode, int sync, void *key); > +int woken_wake_bit_function(struct wait_queue_entry *wq_entry, unsigned mode, int sync, void *key); > int autoremove_wake_function(struct wait_queue_entry *wq_entry, unsigned mode, int sync, void *key); > > #define DEFINE_WAIT_FUNC(name, function) \ > diff --git a/include/linux/wait_bit.h b/include/linux/wait_bit.h > index ace7379d627d..553d7b23e3ad 100644 > --- a/include/linux/wait_bit.h > +++ b/include/linux/wait_bit.h > @@ -32,6 +32,7 @@ int out_of_line_wait_on_bit_timeout(unsigned long *word, int, wait_bit_action_f > int out_of_line_wait_on_bit_lock(unsigned long *word, int, wait_bit_action_f *action, unsigned int mode); > struct wait_queue_head *bit_waitqueue(unsigned long *word, int bit); > extern void __init wait_bit_init(void); > +extern struct wait_bit_key *__var_wake_key(struct wait_queue_entry *wq_entry, void *arg); > > int wake_bit_function(struct wait_queue_entry *wq_entry, unsigned mode, int sync, void *key); > > diff --git a/kernel/futex/core.c b/kernel/futex/core.c > index a7c2a6242718..bd9fb0b17ee6 100644 > --- a/kernel/futex/core.c > +++ b/kernel/futex/core.c > @@ -46,6 +46,7 @@ > #include <linux/slab.h> > #include <linux/vmalloc.h> > #include <linux/kmemleak.h> > +#include <linux/wait_bit.h> > > #include <vdso/futex.h> > > @@ -1886,11 +1887,34 @@ static int futex_hash_allocate(unsigned int hash_slots, unsigned int flags) > futex_hash_bucket_init(&fph->queues[i]); > > if (custom) { > + struct wait_bit_queue_entry __wbq_entry; > + struct wait_queue_head *__wq_head; > + > /* > * Only let prctl() wait / retry; don't unduly delay clone(). > */ > again: > - wait_var_event(mm, futex_pivot_pending(mm)); > + __wq_head = __var_waitqueue(mm); > + init_wait_var_entry(&__wbq_entry, mm, 0); > + __wbq_entry.wq_entry.func = woken_wake_bit_function; > + add_wait_queue(__wq_head, &__wbq_entry.wq_entry); > + > + /* > + * add_wait_queue() futex_ref_put() > + * MB (this) MB (implied) > + * futex_pivot_pending() wake_up_var() > + * waitqueue_active() > + * > + * Notably, it must not be possible to see > + * !futex_pivot_pending() && !waitqueue_active(). > + */ > + smp_mb__after_spinlock(); I still think we should use smp_mb() here, smp_mb__after_spinlock() only orders accesses preceding the lock acquisition against later accesses. The waitqueue insertion happens after that acquisition, so I don't think smp_mb__after_spinlock() covers it here. > + > + while (!futex_pivot_pending(mm) && > + wait_woken(&__wbq_entry.wq_entry, TASK_UNINTERRUPTIBLE, > + MAX_SCHEDULE_TIMEOUT)) > + /* empty */; Since MAX_SCHEDULE_TIMEOUT never returns zero, so I think this can be: while (!futex_pivot_pending(mm)) wait_woken(&__wbq_entry.wq_entry, TASK_UNINTERRUPTIBLE, MAX_SCHEDULE_TIMEOUT)); > + remove_wait_queue(__wq_head, &__wbq_entry.wq_entry); > } > > scoped_guard(mutex, &mm->futex.phash.lock) { > diff --git a/kernel/sched/wait.c b/kernel/sched/wait.c > index 20f27e2cf7ae..d033f600f48c 100644 > --- a/kernel/sched/wait.c > +++ b/kernel/sched/wait.c > @@ -5,6 +5,7 @@ > * (C) 2004 Nadia Yvette Chambers, Oracle > */ > #include "sched.h" > +#include <linux/wait_bit.h> > > void __init_waitqueue_head(struct wait_queue_head *wq_head, const char *name, struct lock_class_key *key) > { > @@ -463,3 +464,17 @@ int woken_wake_function(struct wait_queue_entry *wq_entry, unsigned mode, int sy > return default_wake_function(wq_entry, mode, sync, key); > } > EXPORT_SYMBOL(woken_wake_function); > + > +int woken_wake_bit_function(struct wait_queue_entry *wq_entry, unsigned mode, int sync, void *arg) > +{ > + struct wait_bit_key *key = __var_wake_key(wq_entry, arg); > + if (!key) > + return 0; > + > + /* Pairs with the smp_store_mb() in wait_woken(). */ > + smp_mb(); /* C */ > + wq_entry->flags |= WQ_FLAG_WOKEN; > + > + return default_wake_function(wq_entry, mode, sync, key); > +} > +EXPORT_SYMBOL(woken_wake_bit_function); > diff --git a/kernel/sched/wait_bit.c b/kernel/sched/wait_bit.c > index 1088d3b7012c..e8127e83a48f 100644 > --- a/kernel/sched/wait_bit.c > +++ b/kernel/sched/wait_bit.c > @@ -167,9 +167,7 @@ wait_queue_head_t *__var_waitqueue(void *p) > } > EXPORT_SYMBOL(__var_waitqueue); > > -static int > -var_wake_function(struct wait_queue_entry *wq_entry, unsigned int mode, > - int sync, void *arg) > +struct wait_bit_key *__var_wake_key(struct wait_queue_entry *wq_entry, void *arg) > { > struct wait_bit_key *key = arg; > struct wait_bit_queue_entry *wbq_entry = > @@ -177,6 +175,17 @@ var_wake_function(struct wait_queue_entry *wq_entry, unsigned int mode, > > if (wbq_entry->key.flags != key->flags || > wbq_entry->key.bit_nr != key->bit_nr) > + return NULL; > + > + return key; > +} > + > +static int > +var_wake_function(struct wait_queue_entry *wq_entry, unsigned int mode, > + int sync, void *arg) > +{ > + struct wait_bit_key *key = __var_wake_key(wq_entry, arg); > + if (!key) > return 0; > > return autoremove_wake_function(wq_entry, mode, sync, key); Thanks, Yao Kai ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-08-18 12:24 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-08-13 6:50 [PATCH] futex: Fix might_sleep() warning in futex_pivot_pending() syzbot 2026-08-14 13:38 ` Peter Zijlstra 2026-08-17 7:29 ` Yao Kai 2026-08-18 10:46 ` Peter Zijlstra 2026-08-18 12:24 ` Yao Kai
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox