From: Peter Zijlstra <peterz@infradead.org>
To: Thomas Gleixner <tglx@kernel.org>
Cc: LKML <linux-kernel@vger.kernel.org>,
"Mathieu Desnoyers" <mathieu.desnoyers@efficios.com>,
"André Almeida" <andrealmeid@igalia.com>,
"Sebastian Andrzej Siewior" <bigeasy@linutronix.de>,
"Carlos O'Donell" <carlos@redhat.com>,
"Florian Weimer" <fweimer@redhat.com>,
"Rich Felker" <dalias@aerifal.cx>,
"Torvald Riegel" <triegel@redhat.com>,
"Darren Hart" <dvhart@infradead.org>,
"Ingo Molnar" <mingo@kernel.org>,
"Davidlohr Bueso" <dave@stgolabs.net>,
"Arnd Bergmann" <arnd@arndb.de>,
"Liam R . Howlett" <Liam.Howlett@oracle.com>,
"Uros Bizjak" <ubizjak@gmail.com>,
"Thomas Weißschuh" <linux@weissschuh.net>,
"Mark Brown" <broonie@kernel.org>,
"Richard Weinberger" <richard@nod.at>
Subject: Re: [patch V5 11/16] futex: Provide infrastructure to plug the non contended robust futex unlock race
Date: Wed, 3 Jun 2026 11:14:15 +0200 [thread overview]
Message-ID: <20260603091415.GU3102624@noisy.programming.kicks-ass.net> (raw)
In-Reply-To: <20260602090535.773669210@kernel.org>
On Tue, Jun 02, 2026 at 11:10:04AM +0200, Thomas Gleixner wrote:
> On X86 this boils down to this simplified assembly sequence:
>
> mov %esi,%eax // Load TID into EAX
> xor %ecx,%ecx // Set ECX to 0
> #3 lock cmpxchg %ecx,(%rdi) // Try the TID -> 0 transition
> .Lstart:
> jnz .Lend
> #4 movq %rcx,(%rdx) // Clear list_op_pending
> .Lend:
>
> If the cmpxchg() succeeds and the task is interrupted before it can clear
> list_op_pending in the robust list head (#4) and the task crashes in a
> signal handler or gets killed then it ends up in do_exit() and subsequently
> in the robust list handling, which then might run into the unmap/map issue
> described above.
>
> This is only relevant when user space was interrupted and a signal is
> pending. The fix-up has to be done before signal delivery is attempted
> because:
>
> 1) The signal might be fatal so get_signal() ends up in do_exit()
>
> 2) The signal handler might crash or the task is killed before returning
> from the handler. At that point the instruction pointer in pt_regs is
> not longer the instruction pointer of the initially interrupted unlock
> sequence.
However, due to the pending field being strictly per thread (thread
local storage and all that), the whole construct of futex robust unlock
is not signal safe in the sense that signal handlers must not use it.
A signal handler trying to use this would result in nested use of the
pending field, and that leads to corrupted state.
> The right place to handle this is in __exit_to_user_mode_loop() before
> invoking arch_do_signal_or_restart() as this covers obviously both
> scenarios.
>
> As this is only relevant when the task was interrupted in user space, this
> is tied to RSEQ and the generic entry code as RSEQ keeps track of user
> space interrupts unconditionally even if the task does not have a RSEQ
> region installed. That makes the decision very lightweight:
>
> if (current->rseq.user_irq && within(regs, csr->unlock_ip_range))
> futex_fixup_robust_unlock(regs, csr);
>
> futex_fixup_robust_unlock() then invokes a architecture specific function
> to returen the pending op pointer or NULL. The function evaluates the
> register content to decide whether the pending ops pointer in the robust
> list head needs to be cleared.
>
> Assuming the above unlock sequence, then on x86 this decision is the
> trivial evaluation of the zero flag:
>
> return regs->eflags & X86_EFLAGS_ZF ? regs->dx : NULL;
>
> Other architectures might need to do more complex evaluations due to LLSC,
> but the approach is valid in general. The size of the pointer is determined
> from the matching range struct, which covers both 32-bit and 64-bit builds
> including COMPAT.
So my initial thoughts today were that we should probably also move the
IP to .Lend, to avoid userspace from writing to that location again.
However, due to the above mentioned restrictions vs signals, there
cannot be a situation where this matters, and so the point is moot.
A double store is harmless and it makes the kernel just this little bit
simpler.
The only reason I'm sending this email is to have this more explicitly
documented for posterity I suppose ;-)
> The unlock sequence is going to be placed in the VDSO so that the kernel
> can keep everything synchronized, especially the register usage. The
> resulting code sequence for user space is:
>
> if (__vdso_futex_robust_list$SZ_try_unlock(lock, tid, &pending_op) != tid)
> err = sys_futex($OP | FUTEX_ROBUST_UNLOCK,....);
>
> Both the VDSO unlock and the kernel side unlock ensure that the pending_op
> pointer is always cleared when the lock becomes unlocked.
next prev parent reply other threads:[~2026-06-03 9:14 UTC|newest]
Thread overview: 44+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-06-02 9:09 [patch V5 00/16] futex: Address the robust futex unlock race for real Thomas Gleixner
2026-06-02 9:09 ` [patch V5 01/16] percpu: Sanitize __percpu_qual include hell Thomas Gleixner
2026-06-03 14:25 ` [tip: locking/core] " tip-bot2 for Thomas Gleixner
2026-06-02 9:09 ` [patch V5 02/16] futex: Move futex task related data into a struct Thomas Gleixner
2026-06-03 14:25 ` [tip: locking/core] " tip-bot2 for Thomas Gleixner
2026-06-02 9:09 ` [patch V5 03/16] futex: Make futex_mm_init() void Thomas Gleixner
2026-06-03 14:25 ` [tip: locking/core] " tip-bot2 for Thomas Gleixner
2026-06-02 9:09 ` [patch V5 04/16] futex: Move futex related mm_struct data into a struct Thomas Gleixner
2026-06-03 14:25 ` [tip: locking/core] " tip-bot2 for Thomas Gleixner
2026-06-02 9:09 ` [patch V5 05/16] futex: Provide UABI defines for robust list entry modifiers Thomas Gleixner
2026-06-03 14:25 ` [tip: locking/core] " tip-bot2 for Thomas Gleixner
2026-06-02 9:09 ` [patch V5 06/16] uaccess: Provide unsafe_atomic_store_release_user() Thomas Gleixner
2026-06-03 14:24 ` [tip: locking/core] " tip-bot2 for Thomas Gleixner
2026-06-02 9:09 ` [patch V5 07/16] x86: Select ARCH_MEMORY_ORDER_TSO Thomas Gleixner
2026-06-03 14:24 ` [tip: locking/core] " tip-bot2 for Thomas Gleixner
2026-06-02 9:09 ` [patch V5 08/16] futex: Cleanup UAPI defines Thomas Gleixner
2026-06-03 14:24 ` [tip: locking/core] " tip-bot2 for Thomas Gleixner
2026-06-04 8:23 ` [patch V5 08/16] " David Laight
2026-06-02 9:09 ` [patch V5 09/16] futex: Add support for unlocking robust futexes Thomas Gleixner
2026-06-03 8:22 ` Peter Zijlstra
2026-06-03 9:30 ` Peter Zijlstra
2026-06-03 14:40 ` Thomas Gleixner
2026-06-03 8:35 ` Peter Zijlstra
2026-06-03 14:24 ` [tip: locking/core] " tip-bot2 for Thomas Gleixner
2026-06-02 9:09 ` [patch V5 10/16] futex: Add robust futex unlock IP range Thomas Gleixner
2026-06-03 14:24 ` [tip: locking/core] " tip-bot2 for Thomas Gleixner
2026-06-02 9:10 ` [patch V5 11/16] futex: Provide infrastructure to plug the non contended robust futex unlock race Thomas Gleixner
2026-06-03 8:42 ` Peter Zijlstra
2026-06-03 9:14 ` Peter Zijlstra [this message]
2026-06-03 14:47 ` Thomas Gleixner
2026-06-03 9:23 ` Peter Zijlstra
2026-06-03 14:42 ` Thomas Gleixner
2026-06-03 14:24 ` [tip: locking/core] " tip-bot2 for Thomas Gleixner
2026-06-02 9:10 ` [patch V5 12/16] x86/vdso: Prepare for robust futex unlock support Thomas Gleixner
2026-06-03 14:24 ` [tip: locking/core] " tip-bot2 for Thomas Gleixner
2026-06-02 9:10 ` [patch V5 13/16] x86/vdso: Implement __vdso_futex_robust_try_unlock() Thomas Gleixner
2026-06-03 14:24 ` [tip: locking/core] " tip-bot2 for Thomas Gleixner
2026-06-02 9:10 ` [patch V5 14/16] Documentation: futex: Add a note about robust list race condition Thomas Gleixner
2026-06-03 14:24 ` [tip: locking/core] " tip-bot2 for André Almeida
2026-06-02 9:10 ` [patch V5 15/16] selftests: futex: Add tests for robust release operations Thomas Gleixner
2026-06-03 14:24 ` [tip: locking/core] " tip-bot2 for André Almeida
2026-06-02 9:10 ` [patch V5 16/16] [RFC] vdso, x86: Expose vdso.so.dbg through sysfs Thomas Gleixner
2026-06-02 10:39 ` Thomas Weißschuh
2026-06-02 20:02 ` Thomas Gleixner
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=20260603091415.GU3102624@noisy.programming.kicks-ass.net \
--to=peterz@infradead.org \
--cc=Liam.Howlett@oracle.com \
--cc=andrealmeid@igalia.com \
--cc=arnd@arndb.de \
--cc=bigeasy@linutronix.de \
--cc=broonie@kernel.org \
--cc=carlos@redhat.com \
--cc=dalias@aerifal.cx \
--cc=dave@stgolabs.net \
--cc=dvhart@infradead.org \
--cc=fweimer@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux@weissschuh.net \
--cc=mathieu.desnoyers@efficios.com \
--cc=mingo@kernel.org \
--cc=richard@nod.at \
--cc=tglx@kernel.org \
--cc=triegel@redhat.com \
--cc=ubizjak@gmail.com \
/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