public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Thomas Gleixner <tglx@kernel.org>
To: "Mathieu Desnoyers" <mathieu.desnoyers@efficios.com>,
	"André Almeida" <andrealmeid@igalia.com>
Cc: linux-kernel@vger.kernel.org, Carlos O'Donell <carlos@redhat.com>,
	Sebastian Andrzej Siewior <bigeasy@linutronix.de>,
	Peter Zijlstra <peterz@infradead.org>,
	Florian Weimer <fweimer@redhat.com>,
	Rich Felker <dalias@aerifal.cx>,
	Torvald Riegel <triegel@redhat.com>,
	Darren Hart <dvhart@infradead.org>,
	Ingo Molnar <mingo@redhat.com>,
	Davidlohr Bueso <dave@stgolabs.net>,
	Arnd Bergmann <arnd@arndb.de>,
	"Liam R . Howlett" <Liam.Howlett@oracle.com>
Subject: Re: [RFC PATCH] futex: Introduce __vdso_robust_futex_unlock
Date: Mon, 16 Mar 2026 18:12:34 +0100	[thread overview]
Message-ID: <874imfpukd.ffs@tglx> (raw)
In-Reply-To: <b91a3ad3-7d87-4eb2-9664-652f809f047f@efficios.com>

On Thu, Mar 12 2026 at 18:52, Mathieu Desnoyers wrote:
> On 2026-03-12 18:23, Thomas Gleixner wrote:
>>          xchg(futex->uval, h->unlock_val);
>
> Here is the problem with your proposed approach:
>
>    "XCHG — Exchange Register/Memory With Register"
>                                          ^^^^^^^^
>
> So only one of the xchg arguments can be a memory location.
> Therefore, you will end up needing an extra store after xchg
> to store the content of the result register into h->unlock_val.

Indeed.

> If the process dies between those two instructions, your proposed
> robust list code will be fooled and fall into the same bug that's
> been lingering for 14 years.

s/lingering/ignored/

To fix this for correctness sake it needs more than a hack in the kernel
without even looking at the overall larger picture. I sat down and did a
full analysis and here are the most important questions:

Q: Have non-PI and PI to be treated differently?

A: No.

   That's just historical evolution. While PI can't use XCHG because that
   would create inconsistent state, there is absolutely no reason why
   non-PI can't use try_cmpxchg().


Q: Is it required to unlock in user space first and then go into the kernel
   to wake up waiters?

A: No.

   That's again a historical leftover from the 1st generation futexes which
   preceeded both robust and PI. There is no technical reason to keep it
   this way.

   So both can do:

       if (cmpxchg(lock, tid, 0) != tid)
       		sys_futex(UNLOCK,....);

   which then allows for both non-PI and PI to hand the pending op pointer
   into the syscall and let the kernel deal with the unlock, the op pointer
   and the wake up in one go.

   That reduces the problem space to take care of the non-contended unlock
   case, where the pending op is cleared after the cmpxchg() succeeded.

   And yes, that part can be done in the VDSO and a fixup mechanism in the
   kernel.


Q: Are robust list pointers guaranteed to be 64-bit when running as a
   64-bit task?

A: No.

   The gaming emulators use both the native 64-bit robust list and the
   32-bit robust list from the same 64-bit application to make the
   emulation work.

   So both the UNLOCK syscall and the fixup need to have means to figure
   out the to be cleared size for that pointer.

   Sure, this can be done with a boat load of different functions and flags
   and whatever, but that makes the actual fixup handling in the kernel
   more complicated than necessary.


Q: Have regular signal delivery and process exit in case of crash or being
   killed by a external signal to be treated differently?

A: No.

   A task always goes through the same signal code path for both cases so
   all of this can be handled in _one_ place without even touching the
   robust list cleanup code.

   sys_exit() is different because there a task voluntarily exits and if
   it does so between the unlock and the clearing of the op pointer,
   then so be it. That'd be wilfull ignorance or malice and not any
   different from the task doing the corruption itself in user space
   right away.


Q: Are exception tables a good idea?

A: No.

   This is not an exception handling case. It's a fixup similar to RSEQ
   critical section fixups and so it has to be handled with dedicated
   mechanisms which are performant and not glued onto something which has a
   completely different purpose.


>> This fixes a long standing data corruption race condition with robust
>>  futexes, as pointed out here:
>>
>>  "File corruption race condition in robust mutex unlocking"
>>  https://sourceware.org/bugzilla/show_bug.cgi?id=14485
  
No comment.

Thanks,

	tglx

  parent reply	other threads:[~2026-03-16 17:12 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-11 18:54 [RFC PATCH] futex: Introduce __vdso_robust_futex_unlock Mathieu Desnoyers
2026-03-11 20:11 ` Mathieu Desnoyers
2026-03-12  8:49 ` Florian Weimer
2026-03-12 13:13   ` Mathieu Desnoyers
2026-03-12 14:12     ` Florian Weimer
2026-03-12 14:14       ` André Almeida
2026-03-12 16:09         ` Mathieu Desnoyers
2026-03-12 13:46 ` André Almeida
2026-03-12 14:04   ` Mathieu Desnoyers
2026-03-12 18:40     ` Mathieu Desnoyers
2026-03-12 18:58       ` André Almeida
2026-03-12 19:10     ` Thomas Gleixner
2026-03-12 19:16       ` Mathieu Desnoyers
2026-03-13  8:20         ` Florian Weimer
2026-03-12 20:19   ` Thomas Gleixner
2026-03-12 21:28     ` Mathieu Desnoyers
2026-03-12 22:23 ` Thomas Gleixner
2026-03-12 22:52   ` Mathieu Desnoyers
2026-03-13 12:12     ` Sebastian Andrzej Siewior
2026-03-13 12:17       ` Mathieu Desnoyers
2026-03-13 13:29         ` Sebastian Andrzej Siewior
2026-03-13 13:35           ` Mathieu Desnoyers
2026-03-16 17:12     ` Thomas Gleixner [this message]
2026-03-16 19:36       ` Mathieu Desnoyers
2026-03-16 20:27         ` Thomas Gleixner
2026-03-16 21:01           ` Mathieu Desnoyers
2026-03-16 22:19             ` Thomas Gleixner
2026-03-16 22:30               ` Mathieu Desnoyers
2026-03-16 23:29                 ` Thomas Gleixner
2026-03-20 18:13                   ` Mathieu Desnoyers
2026-03-24 21:35                     ` Thomas Gleixner
2026-03-25 14:12                       ` Mathieu Desnoyers

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=874imfpukd.ffs@tglx \
    --to=tglx@kernel.org \
    --cc=Liam.Howlett@oracle.com \
    --cc=andrealmeid@igalia.com \
    --cc=arnd@arndb.de \
    --cc=bigeasy@linutronix.de \
    --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=mathieu.desnoyers@efficios.com \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=triegel@redhat.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