From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f54.google.com (mail-wm1-f54.google.com [209.85.128.54]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 211167E0E4 for ; Tue, 17 Mar 2026 15:33:51 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.54 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1773761633; cv=none; b=ucNx8MS+BKb6+8YlyAhOFVGqU/TAxzC0EDipfP6CcCkUPtABWAU/g85sod0BxnsE8vs0hSmDyTet+/ncYml4gOY/zDJU357t0wkQX4IN2aMFuINP5+LiKXdIiY3zreMkCOowqOfixr62J1JtvEHI68yQn7Rb/QiVlzpJDHDihJE= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1773761633; c=relaxed/simple; bh=sLX3VkBNwzGbiixeQJBLOX7C8NMCCGXwlb8UGrIMZZI=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=UtrNOhCQelT5CnWvNxsD9UHg5m/PRiWcvbmz1VjqBcP235jIEtO2ftpHFf4qd3rkyjeJboF4Tug1SNz1/nYWfooA9mU49jX9lyuCSetZzWH4J7m2aebx5ZyokV6ip+YKm4hJVPhy3WYThVfuBL4rJ/IB8A1AsRlE3hEg9SFL0Y0= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=lV6N3cUj; arc=none smtp.client-ip=209.85.128.54 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="lV6N3cUj" Received: by mail-wm1-f54.google.com with SMTP id 5b1f17b1804b1-4853f2826f7so61902975e9.1 for ; Tue, 17 Mar 2026 08:33:51 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1773761630; x=1774366430; darn=vger.kernel.org; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=XM7+FXeyBshSdxPh/Aic28DMFnFie8wX5ttNJC/BjvI=; b=lV6N3cUjm4JbYJ1LKaWap8Ci27FFVyObo491Gl+Xa7PYmqSdcRwc313kVigP0pSY8T ufY44UccEXvIA+z/Ziq4QhL6Xn/6wJvCGDezU3/YgoIpgycYZ8Vy8mto0dQorzj7L5g4 e4QsXOSfvfFP+ldiSJItSaO4IbTQa/Y9aZrvDexSx7aZfcP/3sG9q2OdmJKwhe8rKKk5 h816FC0tfKSLZiLHooskmgpZWG22QSSVWikf8Q/Ub0UxuNDEHs6i5qx35KGFDVd5txra 7MHlZ+ufGjPOlbr1V+GgEwMvBZNg0BVWQwjXKzao68oqvwNX+6rGG614j8+u214w4tOj NV+g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1773761630; x=1774366430; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :x-gm-gg:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=XM7+FXeyBshSdxPh/Aic28DMFnFie8wX5ttNJC/BjvI=; b=iTu+gpDNErwD/W21OlO0E0ow3IPmJ2mGNBCkDBsmCAVQ04sSztMc8Xn1sPemaAeQIX 1ITGYBPPGOilpP6CYlzvCRkSCnN1yFxWjMn5qxM6F7+fi04jRUVLZhQSF+4JPLNHsqtK N20wOWEM5J4wWRErIikGCWStGVqczgiMaFHAE3WJouwvgltx83lFn6DIsEzhrTKNnjEU TfMEVoZcYatL5obLPXOkMQkI6BMmvH4wQvx1CPVpEfLWhCFhutzA59FuivWIzN/yZWwD Dcz9Nin1b8CxfczdkXRQ1bZUJb217icCEbKnN3TcVe0M21sGewmrtLwpz8dIq1kjak5P Z+Fg== X-Forwarded-Encrypted: i=1; AJvYcCXmNgQxXyxzOCdQS45Tk7tKYasInURrLp580ZRrP2G6Nq0Qqw2HoqQliDJXed9s2gEui4fIv9Cpi2bBIQE=@vger.kernel.org X-Gm-Message-State: AOJu0YwGuqvXvJNYRdfR+OFqF5FjRjcFlQmeZEMjd7/EoIOO9JoVWBKe 7sLhHoLSgQOndi4u9y6+7da5P4Mv7y2RGGuKNXPxtCkS5fYmXZ13zaH3 X-Gm-Gg: ATEYQzzSNrfZLP5pvfw1rFe1Zs6P6+2w+BEfaKpAEUYM2zXI9dZNFYG+XVNSfpCn/6U gSPo588iDCWW2aJiIop8Ch44VijxpZeshiZLRMicTRj4R//XmCzqtGVKQ/CL/SHa+HZPqThsaiD PuwTTzFKHsU+WNCGh/mYmIu4ygmDcgrXmFswSv2D8MpCYVjcSKjlsBp2Gm1R21qTnwRxSRHXK68 pJ62kXq5/alulzZiFMq7QYkecPOtwYOZYAYm9ISB1lw49nTzUosi5CKlg40nl5XI3B79cyflub1 kqy5J0Df9nxwUa+SQ+dtpkHah8LZcXDqJfPD8dwEmFweX5Kp5Alp+5Fj/zkdAArTGD8NcfDiEFR 1qXGM1dnHynZpng6PjP8ziFLZ6N95bVTJyhYDslWuP/rlyz0SY7eMbmIrK7p09klByXjciQ8I45 n++akEgoaPIK6Me/J2Z1SPPZvGW2VOokNom/vr5MeFXBJ+14HT5E89ldUTPCjyyYZqw54Ef/lPY QXfniW471ydFjdkR11tYeZ+s1q9eJmWQKalOWE89ZbzQOjB09Ix1L7K X-Received: by 2002:a05:600c:34d3:b0:486:d76c:fa51 with SMTP id 5b1f17b1804b1-486d76cff4amr30967075e9.27.1773761630061; Tue, 17 Mar 2026 08:33:50 -0700 (PDT) Received: from [192.168.1.100] ([46.248.82.114]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-4855dbf9af1sm100012665e9.23.2026.03.17.08.33.48 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 17 Mar 2026 08:33:49 -0700 (PDT) Message-ID: <64921c86-b4f1-43f8-b03e-addbd1be19a0@gmail.com> Date: Tue, 17 Mar 2026 16:33:47 +0100 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [patch 8/8] x86/vdso: Implement __vdso_futex_robust_try_unlock() To: Thomas Gleixner , LKML Cc: Mathieu Desnoyers , =?UTF-8?Q?Andr=C3=A9_Almeida?= , Sebastian Andrzej Siewior , Carlos O'Donell , Peter Zijlstra , Florian Weimer , Rich Felker , Torvald Riegel , Darren Hart , Ingo Molnar , Davidlohr Bueso , Arnd Bergmann , "Liam R . Howlett" References: <20260316162316.356674433@kernel.org> <20260316164951.484640267@kernel.org> Content-Language: en-US From: Uros Bizjak In-Reply-To: <20260316164951.484640267@kernel.org> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit On 3/16/26 18:13, Thomas Gleixner wrote: > When the FUTEX_ROBUST_UNLOCK mechanism is used for unlocking (PI-)futexes, > then the unlock sequence in userspace looks like this: > > 1) robust_list_set_op_pending(mutex); > 2) robust_list_remove(mutex); > > lval = gettid(); > 3) if (atomic_try_cmpxchg(&mutex->lock, lval, 0)) > 4) robust_list_clear_op_pending(); > else > 5) sys_futex(OP,...FUTEX_ROBUST_UNLOCK); > > That still leaves a minimal race window between #3 and #4 where the mutex > could be acquired by some other task which observes that it is the last > user and: > > 1) unmaps the mutex memory > 2) maps a different file, which ends up covering the same address > > When then the original task exits before reaching #6 then the kernel robust > list handling observes the pending op entry and tries to fix up user space. > > In case that the newly mapped data contains the TID of the exiting thread > at the address of the mutex/futex the kernel will set the owner died bit in > that memory and therefore corrupt unrelated data. > > Provide a VDSO function which exposes the critical section window in the > VDSO symbol table. The resulting addresses are updated in the task's mm > when the VDSO is (re)map()'ed. > > The core code detects when a task was interrupted within the critical > section and is about to deliver a signal. It then invokes an architecture > specific function which determines whether the pending op pointer has to be > cleared or not. The assembly sequence for the non COMPAT case is: > > mov %esi,%eax // Load TID into EAX > xor %ecx,%ecx // Set ECX to 0 > lock cmpxchg %ecx,(%rdi) // Try the TID -> 0 transition > .Lstart: > jnz .Lend > movq $0x0,(%rdx) // Clear list_op_pending > .Lend: > ret > > So the decision can be simply based on the ZF state in regs->flags. > > If COMPAT is enabled then the try_unlock() function needs to take the size > bit in the OP pointer into account, which makes it slightly more complex: > > mov %esi,%eax // Load TID into EAX > mov %rdx,%rsi // Get the op pointer > xor %ecx,%ecx // Set ECX to 0 > and $0xfffffffffffffffe,%rsi // Clear the size bit > lock cmpxchg %ecx,(%rdi) // Try the TID -> 0 transition > .Lstart: > jnz .Lend > .Lsuccess: > testl $0x1,(%rdx) // Test the size bit > jz .Lop64 // Not set: 64-bit > movl $0x0,(%rsi) // Clear 32-bit > jmp .Lend > .Lop64: > movq $0x0,(%rsi) // Clear 64-bit > .Lend: > ret > > The decision function has to check whether regs->ip is in the success > portion as the size bit test obviously modifies ZF too. If it is before > .Lsuccess then ZF contains the cmpxchg() result. If it's at of after > .Lsuccess then the pointer has to be cleared. > > The original pointer with the size bit is preserved in RDX so the fixup can > utilize the existing clearing mechanism, which is used by sys_futex(). > > Arguably this could be avoided by providing separate functions and making > the IP range for the quick check in the exit to user path cover the whole > text section which contains the two functions. But that's not a win at all > because: > > 1) User space needs to handle the two variants instead of just > relying on a bit which can be saved in the mutex at > initialization time. > > 2) The fixup decision function has then to evaluate which code path is > used. That just adds more symbols and range checking for no real > value. > > The unlock function is inspired by an idea from Mathieu Desnoyers. > > Signed-off-by: Thomas Gleixner > Link: https://lore.kernel.org/20260311185409.1988269-1-mathieu.desnoyers@efficios.com > --- > arch/x86/Kconfig | 1 > arch/x86/entry/vdso/common/vfutex.c | 72 +++++++++++++++++++++++++++++++ > arch/x86/entry/vdso/vdso32/Makefile | 5 +- > arch/x86/entry/vdso/vdso32/vdso32.lds.S | 6 ++ > arch/x86/entry/vdso/vdso32/vfutex.c | 1 > arch/x86/entry/vdso/vdso64/Makefile | 7 +-- > arch/x86/entry/vdso/vdso64/vdso64.lds.S | 6 ++ > arch/x86/entry/vdso/vdso64/vdsox32.lds.S | 6 ++ > arch/x86/entry/vdso/vdso64/vfutex.c | 1 > arch/x86/include/asm/futex_robust.h | 44 ++++++++++++++++++ > 10 files changed, 144 insertions(+), 5 deletions(-) > > --- a/arch/x86/Kconfig > +++ b/arch/x86/Kconfig > @@ -237,6 +237,7 @@ config X86 > select HAVE_EFFICIENT_UNALIGNED_ACCESS > select HAVE_EISA if X86_32 > select HAVE_EXIT_THREAD > + select HAVE_FUTEX_ROBUST_UNLOCK > select HAVE_GENERIC_TIF_BITS > select HAVE_GUP_FAST > select HAVE_FENTRY if X86_64 || DYNAMIC_FTRACE > --- /dev/null > +++ b/arch/x86/entry/vdso/common/vfutex.c > @@ -0,0 +1,72 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +#include > + > +/* > + * Compat enabled kernels have to take the size bit into account to support the > + * mixed size use case of gaming emulators. Contrary to the kernel robust unlock > + * mechanism all of this does not test for the 32-bit modifier in 32-bit VDSOs > + * and in compat disabled kernels. User space can keep the pieces. > + */ > +#if defined(CONFIG_X86_64) && !defined(BUILD_VDSO32_64) > + > +#ifdef CONFIG_COMPAT The following asm template can be substantially improved. > +# define ASM_CLEAR_PTR \ > + " testl $1, (%[pop]) \n" \ Please use byte-wide instruction, TESTB with address operand modifier, "%a[pop]" instead of "(%[pop])": testb $1, %a[pop] > + " jz .Lop64 \n" \ > + " movl $0, (%[pad]) \n" \ Here you can reuse zero-valued operand "val" and use address operand modifier. Please note %k modifier. movl %k[val], %a[pad] > + " jmp __vdso_futex_robust_try_unlock_cs_end \n" \ > + ".Lop64: \n" \ > + " movq $0, (%[pad]) \n" Again, zero-valued operand "val" and address op modifier can be used here: movq %[val], %a[pad] > + > +# define ASM_PAD_CONSTRAINT ,[pad] "S" (((unsigned long)pop) & ~0x1UL) > + > +#else /* CONFIG_COMPAT */ > + > +# define ASM_CLEAR_PTR \ > + " movq $0, (%[pop]) \n" movq %[val], %a[pop] > + > +# define ASM_PAD_CONSTRAINT > + > +#endif /* !CONFIG_COMPAT */ > + > +#else /* CONFIG_X86_64 && !BUILD_VDSO32_64 */ > + > +# define ASM_CLEAR_PTR \ > + " movl $0, (%[pad]) \n" movl %[val], %a[pad] > + > +# define ASM_PAD_CONSTRAINT ,[pad] "S" (((unsigned long)pop) & ~0x1UL) > + > +#endif /* !CONFIG_X86_64 || BUILD_VDSO32_64 */ > + > +uint32_t __vdso_futex_robust_try_unlock(uint32_t *lock, uint32_t tid, void *pop) > +{ > + asm volatile ( > + ".global __vdso_futex_robust_try_unlock_cs_start \n" > + ".global __vdso_futex_robust_try_unlock_cs_success \n" > + ".global __vdso_futex_robust_try_unlock_cs_end \n" > + " \n" > + " lock cmpxchgl %[val], (%[ptr]) \n" > + " \n" > + "__vdso_futex_robust_try_unlock_cs_start: \n" > + " \n" > + " jnz __vdso_futex_robust_try_unlock_cs_end \n" > + " \n" > + "__vdso_futex_robust_try_unlock_cs_success: \n" > + " \n" > + ASM_CLEAR_PTR > + " \n" > + "__vdso_futex_robust_try_unlock_cs_end: \n" > + : [tid] "+a" (tid) You need earlyclobber here "+&a", because not all input arguemnts are read before this argument is written. > + : [ptr] "D" (lock), > + [pop] "d" (pop), > + [val] "r" (0) [val] "r" (0UL), so the correct register width will be used. I'd name this operand [zero], because 0 lives here, and it will be reused in several places. Uros. > + ASM_PAD_CONSTRAINT > + : "memory" > + ); > + > + return tid; > +} > + > +uint32_t futex_robust_try_unlock(uint32_t *, uint32_t, void **) > + __attribute__((weak, alias("__vdso_futex_robust_try_unlock"))); > --- a/arch/x86/entry/vdso/vdso32/Makefile > +++ b/arch/x86/entry/vdso/vdso32/Makefile > @@ -7,8 +7,9 @@ > vdsos-y := 32 > > # Files to link into the vDSO: > -vobjs-y := note.o vclock_gettime.o vgetcpu.o > -vobjs-y += system_call.o sigreturn.o > +vobjs-y := note.o vclock_gettime.o vgetcpu.o > +vobjs-y += system_call.o sigreturn.o > +vobjs-$(CONFIG_FUTEX_ROBUST_UNLOCK) += vfutex.o > > # Compilation flags > flags-y := -DBUILD_VDSO32 -m32 -mregparm=0 > --- a/arch/x86/entry/vdso/vdso32/vdso32.lds.S > +++ b/arch/x86/entry/vdso/vdso32/vdso32.lds.S > @@ -30,6 +30,12 @@ VERSION > __vdso_clock_gettime64; > __vdso_clock_getres_time64; > __vdso_getcpu; > +#ifdef CONFIG_FUTEX_ROBUST_UNLOCK > + __vdso_futex_robust_try_unlock; > + __vdso_futex_robust_try_unlock_cs_start; > + __vdso_futex_robust_try_unlock_cs_success; > + __vdso_futex_robust_try_unlock_cs_end; > +#endif > }; > > LINUX_2.5 { > --- /dev/null > +++ b/arch/x86/entry/vdso/vdso32/vfutex.c > @@ -0,0 +1 @@ > +#include "common/vfutex.c" > --- a/arch/x86/entry/vdso/vdso64/Makefile > +++ b/arch/x86/entry/vdso/vdso64/Makefile > @@ -8,9 +8,10 @@ vdsos-y := 64 > vdsos-$(CONFIG_X86_X32_ABI) += x32 > > # Files to link into the vDSO: > -vobjs-y := note.o vclock_gettime.o vgetcpu.o > -vobjs-y += vgetrandom.o vgetrandom-chacha.o > -vobjs-$(CONFIG_X86_SGX) += vsgx.o > +vobjs-y := note.o vclock_gettime.o vgetcpu.o > +vobjs-y += vgetrandom.o vgetrandom-chacha.o > +vobjs-$(CONFIG_X86_SGX) += vsgx.o > +vobjs-$(CONFIG_FUTEX_ROBUST_UNLOCK) += vfutex.o > > # Compilation flags > flags-y := -DBUILD_VDSO64 -m64 -mcmodel=small > --- a/arch/x86/entry/vdso/vdso64/vdso64.lds.S > +++ b/arch/x86/entry/vdso/vdso64/vdso64.lds.S > @@ -32,6 +32,12 @@ VERSION { > #endif > getrandom; > __vdso_getrandom; > +#ifdef CONFIG_FUTEX_ROBUST_UNLOCK > + __vdso_futex_robust_try_unlock; > + __vdso_futex_robust_try_unlock_cs_start; > + __vdso_futex_robust_try_unlock_cs_success; > + __vdso_futex_robust_try_unlock_cs_end; > +#endif > local: *; > }; > } > --- a/arch/x86/entry/vdso/vdso64/vdsox32.lds.S > +++ b/arch/x86/entry/vdso/vdso64/vdsox32.lds.S > @@ -22,6 +22,12 @@ VERSION { > __vdso_getcpu; > __vdso_time; > __vdso_clock_getres; > +#ifdef CONFIG_FUTEX_ROBUST_UNLOCK > + __vdso_futex_robust_try_unlock; > + __vdso_futex_robust_try_unlock_cs_start; > + __vdso_futex_robust_try_unlock_cs_success; > + __vdso_futex_robust_try_unlock_cs_end; > +#endif > local: *; > }; > } > --- /dev/null > +++ b/arch/x86/entry/vdso/vdso64/vfutex.c > @@ -0,0 +1 @@ > +#include "common/vfutex.c" > --- /dev/null > +++ b/arch/x86/include/asm/futex_robust.h > @@ -0,0 +1,44 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +#ifndef _ASM_X86_FUTEX_ROBUST_H > +#define _ASM_X86_FUTEX_ROBUST_H > + > +#include > + > +static __always_inline bool x86_futex_needs_robust_unlock_fixup(struct pt_regs *regs) > +{ > + /* > + * This is tricky in the compat case as it has to take the size check > + * into account. See the ASM magic in the VDSO vfutex code. If compat is > + * disabled or this is a 32-bit kernel then ZF is authoritive no matter > + * what. > + */ > + if (!IS_ENABLED(CONFIG_X86_64) || !IS_ENABLED(CONFIG_IA32_EMULATION)) > + return !!(regs->flags & X86_EFLAGS_ZF); > + > + /* > + * For the compat case, the core code already established that regs->ip > + * is >= cs_start and < cs_end. Now check whether it is at the > + * conditional jump which checks the cmpxchg() or if it succeeded and > + * does the size check, which obviously modifies ZF too. > + */ > + if (regs->ip >= current->mm->futex.unlock_cs_success_ip) > + return true; > + /* > + * It's at the jnz right after the cmpxchg(). ZF tells whether this > + * succeeded or not. > + */ > + return !!(regs->flags & X86_EFLAGS_ZF); > +} > + > +#define arch_futex_needs_robust_unlock_fixup(regs) \ > + x86_futex_needs_robust_unlock_fixup(regs) > + > +static __always_inline void __user *x86_futex_robust_unlock_get_pop(struct pt_regs *regs) > +{ > + return (void __user *)regs->dx; > +} > + > +#define arch_futex_robust_unlock_get_pop(regs) \ > + x86_futex_robust_unlock_get_pop(regs) > + > +#endif /* _ASM_X86_FUTEX_ROBUST_H */ >