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

On 8/25/2026 8:58 PM, Sean Christopherson wrote:
> 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, [email protected] 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/[email protected]?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.

Sorry, I got confused between static assertion and compile time
assertion. BUILD_BUG_ON() should work. Will update and repost the patches.

Thanks
Sairaj

> 
> 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)
>