linux-kernel - Re: [RFC PATCH] futex: don't retry futex_wait() with stale uaddr/val after spurious wakeup

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 linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Thu, 7 Nov 2019 08:16:59 -0500 (EST)
From:   Jan Stancek <jstancek@...hat.com>
To:     Thomas Gleixner <tglx@...utronix.de>
Cc:     Ingo Molnar <mingo@...nel.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Darren Hart <dvhart@...radead.org>, longman@...hat.com,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [RFC PATCH] futex: don't retry futex_wait() with stale
 uaddr/val after spurious wakeup

My apologies for the 'word salad'.

> So you go great length to "fix" the spurious wakeup case, but what happens if
> there is an actual signal?
> 
> It will return to handle the signal and then run into the exactly same
> situation because it restarts the syscall with the original uaddr1/uval,
> right?

Right, I missed that.

> 
> That means that the shortcut which was added in commit d58e6576b0de
> ("futex: Handle spurious wake up") is equivalent to the actual signal case.
> 
> So the above churn is pretty pointless because it "fixes" not even half of
> the problem and you can't fix the -ERESTARTSYS case at all.
> 
> IIRC the uaddr1 value is supposed to change on a requeue operation so that
> a late incoming waiter goes back with -EWOULDBLOCK. And excatly the same
> would happen on the retry.
> 
> Aside of that you are completely failing to explain in which context you
> observe this problem. Is that failing on libc, some test case or some other
> maybe experimental code?

It's test case (LTP futex_cmp_requeue01), which keeps uaddr1 value same
across requeue, and then sporadically fails to wake up some child processes.

> 
> If there is an actual use case for keeping the uaddr1 value the same across
> a requeue..

It seems both I and test author missed some hint at man page
for FUTEX_CMP_REQUEUE and assumed this was valid use case.

> But let's talk about that once you came up with a proper explanation for
> what you are trying to solve and why you think it's correct.

Thank you for the detailed response.