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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20241024061300.l5y4ng5gmkfrfdht@treble.attlocal.net>
Date: Wed, 23 Oct 2024 23:13:00 -0700
From: Josh Poimboeuf <jpoimboe@...nel.org>
To: Linus Torvalds <torvalds@...ux-foundation.org>
Cc: Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	x86@...nel.org, Andrew Cooper <andrew.cooper3@...rix.com>,
	Borislav Petkov <bp@...en8.de>
Subject: Re: [PATCH] x86: fix user address masking non-canonical speculation
 issue

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