Re: [PATCH] x86: fix user address masking non-canonical speculation issue

[Josh Poimboeuf <jpoimboe@kernel.org>]

On Wed, Oct 23, 2024 at 06:31:59PM -0700, Linus Torvalds wrote:
> @@ -2389,6 +2390,15 @@ void __init arch_cpu_finalize_init(void)
> +		/*
> +		 * Enable this when LAM is gated on LASS support
> +		if (cpu_feature_enabled(X86_FEATURE_LAM))
> +			USER_PTR_MAX = (1ul << 63) - PAGE_SIZE;
> +		 */

I'm probably missing something but once LAM is enabled, how wouldn't
this allow non-canonical address speculation?  i.e. when bit 47/56 is
set and 63 is cleared, would it not go untouched by mask_user_address()
and thus be speculatively interpreted by AMD as a kernel address?

Also, the comment above __access_ok() now seems obsolete:

/*
 * User pointers can have tag bits on x86-64.  This scheme tolerates
 * arbitrary values in those bits rather then masking them off.
 *
 * Enforce two rules:
 * 1. 'ptr' must be in the user half of the address space
 * 2. 'ptr+size' must not overflow into kernel addresses
 *
 * Note that addresses around the sign change are not valid addresses,
 * and will GP-fault even with LAM enabled if the sign bit is set (see
 * "CR3.LAM_SUP" that can narrow the canonicality check if we ever
 * enable it, but not remove it entirely).
 *
 * So the "overflow into kernel addresses" does not imply some sudden
 * exact boundary at the sign bit, and we can allow a lot of slop on the
 * size check.
 *
 * In fact, we could probably remove the size check entirely, since
 * any kernel accesses will be in increasing address order starting
 * at 'ptr', and even if the end might be in kernel space, we'll
 * hit the GP faults for non-canonical accesses before we ever get
 * there.
 *
 * That's a separate optimization, for now just handle the small
 * constant case.
 */

Other than the LAM question, it looks good to me and the code
generation+patching works as expected on a live system.

-- 
Josh