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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] When the host CPU lacks CX16 support, the 16-byte fallback returns a mismatch without updating the old value pointer, violating the cmpxchg API contract and causing infinite retry loops.
--

commit 5f4dae7c1efaa8622accc402392f8d8a532481b4
Author: Sairaj Kodilkar <sarunkod@amd.com>

x86/uaccess: Extend CMPXCHG user helpers to 128-bit operands

This commit extends the user CMPXCHG helpers to support 16-byte operands
on x86-64 using cmpxchg16b. KVM uses this to atomically emulate guest
cmpxchg16b on guest RAM mapped via userspace addresses.

> diff --git a/arch/x86/include/asm/uaccess.h b/arch/x86/include/asm/uaccess.h
> --- a/arch/x86/include/asm/uaccess.h
> +++ b/arch/x86/include/asm/uaccess.h
[ ... ]
> @@ -580,6 +634,14 @@ extern void __try_cmpxchg_user_wrong_size(void);
>  	case 8:	__ret = __try_cmpxchg64_user_asm((__force u64 *)(_ptr), (_oldp),\
>  						 (_nval), _label);		\
>  		break;								\
> +	case 16:								\
> +		if (boot_cpu_has(X86_FEATURE_CX16))				\
> +			__ret = __try_cmpxchg128_user_asm(			\
> +					(__force u128 *)(_ptr),			\
> +					(_oldp), (_nval), _label);		\
> +		else								\
> +			__ret = 0;						\

[Severity: High]
Does setting __ret to 0 here on hardware without CX16 support violate the
cmpxchg API contract?

When boot_cpu_has(X86_FEATURE_CX16) is false, it returns 0 (mismatch) but
fails to fetch the actual memory value and write it into _oldp.

If a guest executes cmpxchg16b on an MMIO address (forcing KVM emulation),
KVM's emulator decodes it and invokes unsafe_try_cmpxchg_user. Because
it returns 0 without updating the pointer, x86_emulate_insn skips
writeback_registers() and returns EMULATION_OK. 

The guest registers remain unchanged while EFLAGS shows failure (ZF=0).

Can this cause a denial of service where the guest endlessly retries the
instruction with the same stale registers?

Should this instead use goto _label; to cleanly abort the unsupported
operation, which would cause KVM to properly abort the emulation?

> +		break;								\
>  	default: __try_cmpxchg_user_wrong_size();				\
>  	}									\
>  	__ret;						})

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260908074739.10748-1-sarunkod@amd.com?part=1