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 PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Mon, 26 Oct 2009 12:01:22 -0700 From: Darren Hart <dvhltc@...ibm.com> To: dino@...ibm.com CC: tglx@...utronix.de, linux-kernel@...r.kernel.org, linux-rt-users@...r.kernel.org, Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...e.hu>, Eric Dumazet <eric.dumazet@...il.com>, John Stultz <johnstul@...ux.vnet.ibm.com> Subject: Re: [patch -rt] Fix infinite loop with 2.6.31.4-rt14 V2 Darren Hart wrote: > Darren Hart wrote: >> Dinakar Guniguntala wrote: >> > Application threads calling futex_wait_requeue_pi run in an >> infinite loop >> > in the kernel if the futex value changes during the call. The >> following >> > patch fixes the problem. >> >> The key bit here being that EAGAIN == EWOULDBLOCK - who thought that >> was a good idea? > > And now that I think about it, when I reviewed this original patch I > remember mentioning that this isn't even needed for > futex_wait_requeue_pi() because we don't have the same wake-up race as > futex_wait() suffers from - since we don't use the same lock_ptr == NULL > test (nor do we use the wake_list in the requeue code). I suspect the > only case where -EAGAIN is being used here is when the uval doesn't > match val - no spurious wakeups. > > Dino, can you try with the following patch which just reverts the > spurious wakeup handling for the requeue_pi path. Dino mentioned in IRC that this is basically what he tried originally and that it worked fine. Thomas, any objections to this patch? -- Darren > From c21e762bf384e0a559fdf964e0ba7576550d90ec Mon Sep 17 00:00:00 2001 > From: Darren Hart <dvhltc@...ibm.com> > Date: Fri, 23 Oct 2009 16:18:48 -0700 > Subject: [PATCH] futex: revert spurious wakeup fix for requeue_pi > > The requeue_pi path doesn't use unqueue_me() (and the racy lock_ptr == > NULL test) nor does it use the wake_list of futex_wake() which led to > the following fix. > > 41890f2... futex: Handle spurious wake up > > See debugging discussing on LKML Message-ID: <4AD4080C.20703@...ibm.com> > > The changes in this fix to the requeue_pi path were considered to be a > likely unecessary, but harmless safety net. Since they are in fact > causing a problem, just remove them and insert a warning in their place. > We can remove the warning later, or even in this commit if folks would > rather. > > Signed-off-by: Darren Hart <dvhltc@...ibm.com> > Cc: Thomas Gleixner <tglx@...utronix.de> > Cc: Peter Zijlstra <peterz@...radead.org> > Cc: Ingo Molnar <mingo@...e.hu> > CC: Eric Dumazet <eric.dumazet@...il.com> > CC: Dinakar Guniguntala <dino@...ibm.com> > CC: John Stultz <johnstul@...ibm.com> > > Witholding CC to stable for further discussion. > --- > kernel/futex.c | 15 +++++++++------ > 1 files changed, 9 insertions(+), 6 deletions(-) > > diff --git a/kernel/futex.c b/kernel/futex.c > index 7c4a6ac..7e4e8b2 100644 > --- a/kernel/futex.c > +++ b/kernel/futex.c > @@ -2085,12 +2085,19 @@ int handle_early_requeue_pi_wakeup(struct > futex_hash_bucket *hb, > */ > plist_del(&q->list, &q->list.plist); > > - /* Handle spurious wakeups gracefully */ > - ret = -EAGAIN; > if (timeout && !timeout->task) > ret = -ETIMEDOUT; > else if (signal_pending(current)) > ret = -ERESTARTNOINTR; > + else { > + /* > + * We don't use the racy unqueue_me() path with the > + * q.lock_ptr NULL test, nor does requeue use a > + * wake_list. All wakeups here should be accounted for. > + */ > + printk(KERN_ERR "Spurious wakeup in %s\n", > + __FUNCTION__); > + } > } > return ret; > } > @@ -2171,7 +2178,6 @@ static int futex_wait_requeue_pi(u32 __user > *uaddr, int fshared, > q.bitset = bitset; > q.rt_waiter = &rt_waiter; > > -retry: > key2 = FUTEX_KEY_INIT; > ret = get_futex_key(uaddr2, fshared, &key2, VERIFY_WRITE); > if (unlikely(ret != 0)) > @@ -2264,9 +2270,6 @@ out_put_keys: > out_key2: > put_futex_key(fshared, &key2); > > - /* Spurious wakeup ? */ > - if (ret == -EAGAIN) > - goto retry; > out: > if (to) { > hrtimer_cancel(&to->timer); -- Darren Hart IBM Linux Technology Center Real-Time Linux Team -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists