Message-ID: <aQozS2ZHX4x1APvb@google.com>
Date: Tue, 4 Nov 2025 09:09:31 -0800
From: Sean Christopherson <seanjc@...gle.com>
To: Linus Torvalds <torvalds@...ux-foundation.org>
Cc: Mateusz Guzik <mjguzik@...il.com>, "the arch/x86 maintainers" <x86@...nel.org>, brauner@...nel.org, 
	viro@...iv.linux.org.uk, jack@...e.cz, linux-kernel@...r.kernel.org, 
	linux-fsdevel@...r.kernel.org, tglx@...utronix.de, pfalcato@...e.de
Subject: Re: [PATCH 1/3] x86: fix access_ok() and valid_user_address() using
 wrong USER_PTR_MAX in modules

On Tue, Nov 04, 2025, Linus Torvalds wrote:
> [ Adding x86 maintainers - I had added Thomas earlier, but I guess at
> least Borislav might actually care and have input too ]
> 
> So I think the patch I will commit would look like the attached: it's
> similar to your suggestion, but without the renaming of USER_PTR_MAX,
> and with just a
> 
>   #ifdef MODULE
>     #define runtime_const_ptr(sym) (sym)
>   #else
>     #include <asm/runtime-const.h>
>   #endif
> 
> in the x86 asm/uaccess_64.h header file and an added '#error' for the
> MODULE case in the actual x86 runtime-const.h file.
> 
> As it is, this bug really only affects modular code that uses

What exactly is the bug?  Is the problem that module usage of runtime_const_ptr()
doesn't get patched on module load, and so module code ends up using the
0x0123456789abcdef placeholder?

> access_ok() and __{get,put}_user(), which is a really broken pattern
> to begin with these days, and is happily fairly rare.

Just to make sure I understand the impact, doesn't this also affect all flavors
of "nocheck" uaccesses?  E.g. access_ok() + __copy_{from,to}_user()?

> That is an old optimization that is no longer an optimization at all
> (since a plain "get_user()" is actually *faster* than the access_ok()
> and __get_user() these days), and I wish we didn't have any such code
> any more, but there are a handful of things that have never been
> converted to the modern world order.

Looking at the assembly, I assume get_user() is faster than __get_user() due to
the LFENCE in ASM_BARRIER_NOSPEC?

> So it is what it is, and we have to deal with it.

Assuming __{get,put}_user() are slower on x86 in all scenarios, would it make
sense to kill them off entirely for x86?  E.g. could we reroute them to the
"checked" variants?

For KVM x86, I'm more than happy to switch all two __{get,put}_user() calls to
the checked variants if they're faster.

> Also, even that kind of rare and broken code actually *works*,
> although the whole "non-canonical reads can speculatively leak
> possibly kernel data" does end up being an issue (largely theoretical
> because it's now limited to just a couple of odd-ball code sequences)
> 
> And yes, it works just because I picked a runtime-const value that is
> non-canonical. I'd say it's "by luck", but I did pick that value
> partly *because* it's non-canonical, so it's not _entirely_ just luck.
> But mostly.
> 
> That was all a long explanation for why I am planning on committing
> this as a real fix, even if the actual impact of it is largely
> theoretical.
> 
> Borislav - comments? Generating this patch took longer than it should
> have, but I had travel and jetlag and a flight that I expected to have
> wifi but didn't...  And properly it should probably be committed by
> x86 maintainers rather than me, but I did mess this code up in the
> first place.
> 
> The patch *looks* very straightforward, but since I'm on the road I am
> doing this on my laptop and haven't actually tested it yet (well, I've
> built this, and booted it, but nothing past that).

FWIW, AFAICT it doesn't cause any regressions for KVM's usage of access_ok().

Powered by blists - more mailing lists