Re: [PATCH v2 1/2] x86/uaccess: Extend CMPXCHG user helpers to 128-bit operands
Sean Christopherson <[email protected]>
| Newsgroups | org.kernel.vger.kvm,dev.linux.lists.sashiko-reviews |
|---|---|
| Message-ID | <[email protected]> |
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.