[PATCH v3 2/6] x86/uaccess: Avoid barrier_nospec() in 64-bit __get_user()

Josh Poimboeuf jpoimboe at kernel.org
Fri Nov 22 14:11:15 AEDT 2024


On Thu, Nov 21, 2024 at 05:02:06PM -0800, Linus Torvalds wrote:
> [ Time passes ]
> 
> Ugh. I tried it. It looks like this:
> 
> #define inlined_get_user(res, ptr) ({                           \
>         __label__ fail2, fail1;                                 \
>         __auto_type __up = (ptr);                               \
>         int __ret = 0;                                          \
>         if (can_do_masked_user_access())                        \
>                 __up = masked_user_access_begin(__up);          \
>         else if (!user_read_access_begin(__up, sizeof(*__up)))  \
>                 goto fail1;                                     \
>         unsafe_get_user(res, ptr, fail2);                       \
>         user_access_end();                                      \
>         if (0) {                                                \
> fail2:  user_access_end();                                      \
> fail1:  __ret = -EFAULT;                                        \
>         }                                                       \
>         __ret; })

That actually doesn't seem so bad, it's easy enough to follow the logic.
And it contains the ugly/fidgety all in one place so the callers' hands
don't have to get dirty.

We could easily use that macro in size-specific inline functions
selected by a macro with a sizeof(type) switch statement -- not so bad
IMO if they improve code usage and generation.

So all the user has to do is get_user_new_and_improved() -- resolving to
get_user_new_and_improved_x() -- and the compiler decides on the
inlining.  Which on average is hopefully better than Joe Developer's
inlining decisions?  Otherwise we've got bigger problems?

Then all the arches have to do is implement unsafe_*_user_{1,2,4,8} and
the "one good implementation" idea comes together?

BTW, looking at some other arches, I notice that get_user() is already
unconditionally inline for arm64, riscv, powerpc, and s390.

I also see that arm64 already defines get_user() to __get_user(), with
__get_user() having an access_ok().

It would be really nice to have the same behavior and shared code across
all the arches.

-- 
Josh


More information about the Linuxppc-dev mailing list