[PATCH] x86/uaccess: Avoid barrier_nospec() in copy_from_user()

[Linus Torvalds]

On Wed, 23 Oct 2024 at 02:45, Borislav Petkov <bp at alien8.de> wrote:
>
> So I was able to get some info: CLAC/STAC in everything Zen4 and older is
> serializing so there's no issue there.
>
> Zen5 is a different story and AC is being renamed there and thus,
> non-serializing. So we'd need to do something there, perhaps something
> like Josh's patch...
>
> But the good thing is we can alternative it out only for those machines, apart
> from the rest.

Hmm. The problem with alternatives is that it gets increasingly ugly
when there are more than two of them.

And even doing a single alternative with some "don't do this unless X"
then means either extra no-ops or some horrid static branch.

Now, if we ignore LAM, we only have two cases (LA48 vs LA57), but we
know that even if we disable LAM until we do the LASS support, that
third case *will* happen.

Finally - the main reason to use the sign bit is that you don't need
some big constant - and the code is small, so the "shift right by 63
and or" is just two instructions and a couple of bytes.

But once you have an alternative *with* a big constant, that advantage
just goes away because you'll just replace the size the constant takes
up with no-ops instead.

So then you might as well just _always_ use a constant.

Which makes me wonder if we can't just use the runtime-const
infrastructure instead. That has the advantage that you can have any
number of different constants and it won't complicate the usage sites,
and you can easily just update the constant at init time with
runtime_const_init() to an arbitrary value.

So then LAM support becomes just updating the value for
runtime_const_init() - none of the code changes, and no different
alternatives cases.

Something like the attached, in other words. Note that this actually does that

+               if (cpu_feature_enabled(X86_FEATURE_LAM))
+                       USER_PTR_MAX = (1ul << 63) - PAGE_SIZE;

partly to show it as docs, and partly because we should just clear
that X86_FEATURE_LAM bit if LASS isn't set, so I think it's right
anyway.

NOTE! This is obviously untested and I didn't check that it does the
cmp/sbb/or the right way around.

Also, it's worth noting that the cmp/sbb/or sequence (if I *did* get
it the right way around) will set all bits only if the original
address is *larger* than USER_PTR_MAX, but we already subtract
PAGE_SIZE from the actual hardware maximum due to other errata, so
that's fine.

IOW, it will turn USER_PTR_MAX and anything less into itself, but
USER_PTR_MAX+1 (and above) becomes all ones.

                   Linus
-------------- next part --------------
A non-text attachment was scrubbed...
Name: patch.diff
Type: text/x-patch
Size: 3010 bytes
Desc: not available
URL: <http://lists.ozlabs.org/pipermail/linuxppc-dev/attachments/20241023/7b926cf8/attachment.bin>