Re: [PATCH v2 1/2] x86/uaccess: Extend CMPXCHG user helpers to 128-bit operands

[Sean Christopherson <seanjc@google.com>]

On Tue, Aug 25, 2026, Sairaj Kodilkar wrote:
> On 8/11/2026 5:57 AM, Sean Christopherson wrote:
> > On Mon, Jul 06, 2026, Sairaj Kodilkar wrote:
> >> On 7/6/2026 12:22 PM, sashiko-bot@kernel.org wrote:
> >>>> diff --git a/arch/x86/include/asm/uaccess.h b/arch/x86/include/asm/uaccess.h
> >>>> index 367297b188c39..123755d471099 100644
> >>>> --- a/arch/x86/include/asm/uaccess.h
> >>>> +++ b/arch/x86/include/asm/uaccess.h
> >>> [ ... ]
> >>>
> >>>> @@ -551,11 +594,18 @@ do {                                                                            \
> >>>>
> >>>>   extern void __try_cmpxchg_user_wrong_size(void);
> >>>>
> >>>> -#ifndef CONFIG_X86_32
> >>>> +#ifdef CONFIG_X86_32
> >>>> +/* Always fail on 32 bit arch as it do not support 128 cmpxchg (i.e. cmpxchg16b
> >>>> + * instruction).
> >>>> + */
> >>>> +#define __try_cmpxchg128_user_asm(_ptr, _pold, _new, label) (1)
> >>> [Severity: Medium]
> >>> Does this 32-bit stub correctly signal failure?
> >>>
> >>> Since this macro evaluates to 1, won't the surrounding unsafe_try_cmpxchg_user
> >>> macro treat this as success (true) rather than failure (false)?
> >>>
> >>> This could cause the wrapper to negate the value and return 0 (success) to the
> >>> caller, falsely signaling a successful memory exchange without actually
> >>> updating memory or the old value. Even though there are no current 32-bit
> >>> callers using a 16-byte operand, should this evaluate to 0 instead of 1 to
> >>> ensure theoretical future callers don't encounter a silent atomicity break?
> >>>
> >>> --
> >>> Sashiko AI review · https://sashiko.dev/#/patchset/20260706063035.1139-1-sarunkod@amd.com?part=1
> >> Hi,
> >>
> >> Right, It should return 0 instead of 1, will rectify this in the next patch.
> > 
> > Is it possible to fail the build instead?  Attempting to generate CMPXCHG16B on
> > 32-bit should straigt up fail.
> 
> Hi Sean,
> 
> A compile-time failure in __try_cmpxchg128_user_asm would unfortunately
> break i386 builds even for callers that never use a 16-byte operand.
> 
> unsafe_try_cmpxchg_user dispatches on sizeof(*(_ptr)) via a switch, and
> the 128-bit path is one of those cases. As noted in the comment above
> the macro, Clang/LLVM tends to compile all switch arms when expanding
> this macro, and only discards unused paths afterward.

If that's actually happening, then this code is already broken, because u128
exists only on (some) 64-bit kernels.

A straight static_assert() will fail, but BUILD_BUG_ON() (or more precisely,
__compiletime_assert())  and all of the associated magic exists for these exact
scenarios.  Unlike static_assert(), __compiletime_assert() relies on dead-code
elimination, i.e. does exactly what we want.

diff --git a/arch/x86/include/asm/uaccess.h b/arch/x86/include/asm/uaccess.h
index a58d50d7a219..710a0b42eeef 100644
--- a/arch/x86/include/asm/uaccess.h
+++ b/arch/x86/include/asm/uaccess.h
@@ -595,10 +595,7 @@ do {										\
 extern void __try_cmpxchg_user_wrong_size(void);
 
 #ifdef CONFIG_X86_32
-/* Always fail on 32 bit arch as it do not support 128 cmpxchg (i.e. cmpxchg16b
- * instruction).
- */
-#define __try_cmpxchg128_user_asm(_ptr, _pold, _new, label) (1)
+#define __try_cmpxchg128_user_asm(_ptr, _pold, _new, label) ({ BUILD_BUG_ON(1); 1; })
 #else
 #define __try_cmpxchg64_user_asm(_ptr, _oldp, _nval, _label)		\
 	__try_cmpxchg_user_asm("q", "r", (_ptr), (_oldp), (_nval), _label)