Message-ID: <CAHk-=wjkaHdi2z62fn+rf++h-f0KM66MXKxVX-xd3X6vqs8SoQ@mail.gmail.com>
Date: Wed, 5 Nov 2025 04:07:44 +0900
From: Linus Torvalds <torvalds@...ux-foundation.org>
To: Sean Christopherson <seanjc@...gle.com>
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 Wed, 5 Nov 2025 at 02:09, Sean Christopherson <seanjc@...gle.com> wrote:
>
> 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?

Yes. The runtime-const fixup is intentionally simplistic, because the
ordering concerns with the more generic instruction rewriting was
painful (and architecture-specific).

And as part of being simple and stupid, it doesn't deal with modules,
and only runs early at boot.

> 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()?

The access_ok() issue happens with those too, but I don't think there
was any way to then trigger the speculative leak with non-canonical
addresses that way.  Iirc, you needed a load-load gadget and only had
a few cycles in which to do it.

But in theory, yes.

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

The access_ok() itself is also slower than the address masking, with
the whole "add size and check for overflow" dance that a plain
get_user() simply doesn't need.

Of course, at some point it can be advantageous to only check the
address once, and then do multiple __get_user() calls, and that was
obviously the *original* advantage (along with inlining the
single-instruction __get_user).

But with SMAP, the inlining advantage hasn't existed for years, and
the "avoid 3*N cheap ALU instructions by using a much more expensive
access_ok()" is dubious even for somewhat larger values of N.

> 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?

Sadly, no. We've wanted to do that many times for various other
reasons, and we really should, but because of historical semantics,
some horrendous users still use "__get_user()" for addresses that
might be user space or might be kernel space depending on use-case.

Maybe we should bite the bullet and just break any remaining cases of
that horrendous historical pattern. There might not be any actual
relevant ones left, and they should all be easyish to fix if we just
*find* them. But we had that pattern in at least some tracing code,
and I'd expect some random drivers too, just because it *used* to
historically work to do "the user access path does access_ok(), the
kernel access path doesn't, and then both can use __get_user()".

In fact, Josh Poimboeuf tried to do that __get_user() fix fairly
recently, but he hit at least the "coco" code mis-using this thing.

See vc_read_mem() in arch/x86/coco/sev/vc-handle.c.

Are there others? Probably not very many. But *finding* all those
cases is the painful part.

Anybody want a new pet project?

              Linus

Powered by blists - more mailing lists