From: Ingo Molnar <mingo@kernel.org>
To: Thomas Gleixner <tglx@linutronix.de>
Cc: LKML <linux-kernel@vger.kernel.org>,
Peter Zijlstra <peterz@infradead.org>,
Darren Hart <darren@dvhart.com>, Yi Wang <wang.yi59@zte.com.cn>,
Yang Tao <yang.tao172@zte.com.cn>,
Oleg Nesterov <oleg@redhat.com>,
Florian Weimer <fweimer@redhat.com>,
Carlos O'Donell <carlos@redhat.com>,
Alexander Viro <viro@zeniv.linux.org.uk>
Subject: Re: [patch 00/12] futex: Cure robust/PI futex exit races
Date: Thu, 7 Nov 2019 09:41:36 +0100 [thread overview]
Message-ID: <20191107084136.GH30739@gmail.com> (raw)
In-Reply-To: <20191106215534.241796846@linutronix.de>
* Thomas Gleixner <tglx@linutronix.de> wrote:
> This series addresses a couple of robust/PI futex exit races:
>
> 1) The unlock races debugged and fixed by Yi and Yang
>
> These races are really subtle and I'm still puzzled how to trigger them
> reliably enough to decode them.
>
> The basic issue is that:
>
> A) An unlocking task can be killed between clearing the user space
> futex value and calling futex(FUTEX_WAKE).
>
> B) A woken up waiter can be killed before it can acquire the futex
> after returning to user space.
>
> In both cases the futex value is 0 and due to that the robust list exit
> code refuses to wake up waiters as the futex is not owned by the
> exiting task. As a consequence all other waiters might be blocked
> forever.
>
> 2) Oleg provided a test case which causes an infinite loop in the
> futex_lock_pi() code.
>
> The problem there is that an exiting task might be preempted by a
> waiter in a state which makes the waiter busy wait for the exiting task
> to complete the robust/PI exit cleanup code.
>
> That's obviously impossible when the waiter has higher priority than
> the exiting task and both are pinned on the same CPU resulting in a
> live lock.
>
> #1 is a straight forward and simple fix
>
> The solution Yi and Yang provided looks solid and in the worst case
> causes a spurious wakeup of a waiter which is nothing to worry about
> as all waiter code has to be prepared for that anyway.
>
> #2 is more complex
>
> In the current implementation there is no way to block until the exiting
> task has finished the cleanup.
>
> To fix this there is quite some code reshuffling required which at the
> same time is a valuable cleanup.
>
> The final solution is to guard the futex exit handling with a per task
> mutex and make the waiter block on that mutex until the exiting task has
> the cleanup completed.
>
> Details why a simpler solution is not feasible can be found here:
>
> https://lore.kernel.org/r/20191105152728.GA5666@redhat.com
>
> Ignore my confusion of fork vs. vfork at the beginning of the thread.
> Futexes do that to human brains. :)
>
> The following series addresses both issues.
>
> Patch 1 is a slightly polished version of the original Yi and Yang
> submission. It is included for completeness sake and because it
> creates conflicts with the larger surgery which fixes issue #2.
>
> Aside of that a few eyeballs more on that subtlety are definitely not
> a bad thing especially as this has a user space component in it.
>
> The rest of the series addresses issue #2 which is more or less a kernel
> only problem, but extra eyeballs are appreciated.
>
> I'm certainly not proud about the solution for #2 but it's the best I could
> come up with without violating the user/kernel state consistency
> constraints.
I really like the whole series - this is how it should have been
implemented originally, but the exit scenarios 'looked' so simple so it
was just open-coded ... Mea culpa. :-)
As to ->futex_exit_mutex: that's really just a consequence of the ABI,
and a lot cleaner than all the previous pretense that these exit ops are
atomic - which they fundamentally aren't.
Haven't tested the series beyond build coverage, but the high level
principles behind the whole series look very sound to me:
Reviewed-by: Ingo Molnar <mingo@kernel.org>
Thanks,
Ingo
next prev parent reply other threads:[~2019-11-07 8:41 UTC|newest]
Thread overview: 53+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-11-06 21:55 [patch 00/12] futex: Cure robust/PI futex exit races Thomas Gleixner
2019-11-06 21:55 ` [patch 01/12] futex: Prevent robust futex exit race Thomas Gleixner
2019-11-06 21:55 ` [patch 02/12] futex: Move futex exit handling into futex code Thomas Gleixner
2019-11-07 9:24 ` Peter Zijlstra
2019-11-07 9:38 ` Thomas Gleixner
2019-11-15 18:19 ` [tip: locking/core] " tip-bot2 for Thomas Gleixner
2019-11-16 20:53 ` Borislav Petkov
2019-11-20 9:38 ` tip-bot2 for Thomas Gleixner
2019-11-06 21:55 ` [patch 03/12] futex: Replace PF_EXITPIDONE with a state Thomas Gleixner
2019-11-15 18:19 ` [tip: locking/core] " tip-bot2 for Thomas Gleixner
2019-11-20 9:38 ` tip-bot2 for Thomas Gleixner
2019-11-06 21:55 ` [patch 04/12] exit/exec: Seperate mm_release() Thomas Gleixner
2019-11-15 18:19 ` [tip: locking/core] " tip-bot2 for Thomas Gleixner
2019-11-20 9:38 ` tip-bot2 for Thomas Gleixner
2019-11-06 21:55 ` [patch 05/12] futex: Split futex_mm_release() for exit/exec Thomas Gleixner
2019-11-15 18:19 ` [tip: locking/core] " tip-bot2 for Thomas Gleixner
2019-11-20 9:38 ` tip-bot2 for Thomas Gleixner
2019-11-06 21:55 ` [patch 06/12] futex: Set task::futex state to DEAD right after handling futex exit Thomas Gleixner
2019-11-15 18:19 ` [tip: locking/core] futex: Set task::futex_state " tip-bot2 for Thomas Gleixner
2019-11-20 9:38 ` tip-bot2 for Thomas Gleixner
2019-11-06 21:55 ` [patch 07/12] futex: Mark the begin of futex exit explicitly Thomas Gleixner
2019-11-15 18:19 ` [tip: locking/core] " tip-bot2 for Thomas Gleixner
2019-11-20 9:38 ` tip-bot2 for Thomas Gleixner
2019-11-06 21:55 ` [patch 08/12] futex: Sanitize exit state handling Thomas Gleixner
2019-11-07 9:38 ` Peter Zijlstra
2019-11-15 18:19 ` [tip: locking/core] " tip-bot2 for Thomas Gleixner
2019-11-20 9:38 ` tip-bot2 for Thomas Gleixner
2019-11-06 21:55 ` [patch 09/12] futex: Provide state handling for exec() as well Thomas Gleixner
2019-11-07 9:49 ` Peter Zijlstra
2019-11-07 9:54 ` Thomas Gleixner
2019-11-15 18:19 ` [tip: locking/core] " tip-bot2 for Thomas Gleixner
2019-11-20 9:38 ` tip-bot2 for Thomas Gleixner
2019-11-06 21:55 ` [patch 10/12] futex: Add mutex around futex exit Thomas Gleixner
2019-11-15 18:19 ` [tip: locking/core] " tip-bot2 for Thomas Gleixner
2019-11-20 9:38 ` tip-bot2 for Thomas Gleixner
2019-11-06 21:55 ` [patch 11/12] futex: Provide distinct return value when owner is exiting Thomas Gleixner
2019-11-15 18:19 ` [tip: locking/core] " tip-bot2 for Thomas Gleixner
2019-11-20 9:38 ` tip-bot2 for Thomas Gleixner
2019-11-06 21:55 ` [patch 12/12] futex: Prevent exit livelock Thomas Gleixner
2019-11-15 18:19 ` [tip: locking/core] " tip-bot2 for Thomas Gleixner
2019-11-20 9:38 ` tip-bot2 for Thomas Gleixner
2019-11-07 8:29 ` [patch 00/12] futex: Cure robust/PI futex exit races Ingo Molnar
2019-11-07 8:41 ` Ingo Molnar [this message]
2019-11-07 9:40 ` Peter Zijlstra
2019-11-07 15:03 ` Oleg Nesterov
2019-11-07 22:29 ` Florian Weimer
2019-11-07 22:40 ` Florian Weimer
2019-11-08 7:38 ` Florian Weimer
2019-11-08 9:18 ` Thomas Gleixner
2019-11-08 10:17 ` Florian Weimer
2019-11-08 10:37 ` Thomas Gleixner
2019-11-08 11:51 ` Thomas Gleixner
2019-11-11 9:48 ` Florian Weimer
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20191107084136.GH30739@gmail.com \
--to=mingo@kernel.org \
--cc=carlos@redhat.com \
--cc=darren@dvhart.com \
--cc=fweimer@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=oleg@redhat.com \
--cc=peterz@infradead.org \
--cc=tglx@linutronix.de \
--cc=viro@zeniv.linux.org.uk \
--cc=wang.yi59@zte.com.cn \
--cc=yang.tao172@zte.com.cn \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).