Re: [PATCH 0/5] powerpc: Implement masked user access

David Laight
2025-06-22T16:20:49Z

On Sun, 22 Jun 2025 11:52:38 +0200
Christophe Leroy <christophe.leroy@csgroup.eu> wrote:

> Masked user access avoids the address/size verification by access_ok().
> Allthough its main purpose is to skip the speculation in the
> verification of user address and size hence avoid the need of spec
> mitigation, it also has the advantage to reduce the amount of
> instructions needed so it also benefits to platforms that don't
> need speculation mitigation, especially when the size of the copy is
> not know at build time.

It also removes a conditional branch that is quite likely to be
statically predicted 'the wrong way'.

> Unlike x86_64 which masks the address to 'all bits set' when the
> user address is invalid, here the address is set to an address in
> the gap. It avoids relying on the zero page to catch offseted
> accesses. On book3s/32 it makes sure the opening remains on user
> segment. The overcost is a single instruction in the masking.

That isn't true (any more).
Linus changed the check to (approx):
if (uaddr > TASK_SIZE)
uaddr = TASK_SIZE;
(Implemented with a conditional move)
Replacing the original version that used cmp, sbb, or to get 'all bits set'.
Quite likely the comments are wrong!

I thought there was a second architecture that implemented it - and might
still set ~0u?
As you noted returning 'TASK_SIZE' (or, at least, the base of a page that
is guaranteed to fault) means that the caller only has to do 'reasonably
sequential' accesses, and not guarantee to read offset zero first.

As a separate patch, provided there is a guard page between user and kernel,
and user accesses are 'reasonably sequential' even access_ok() need not
check the transfer length. Linus wasn't that brave :-)

I think some of the 'API' is still based on the original 386 code where
the page tables had to be checked by hand for CoW.

David

>
> First patch adds masked_user_read_access_begin() and
> masked_user_write_access_begin() to match with user_read_access_end()
> and user_write_access_end().
>
> Second patch adds speculation barrier to copy_from_user_iter() so that
> the barrier in powerpc raw_copy_from_user() which is redundant with
> the one in copy_from_user() can be removed.
>
> Third patch removes the redundant barrier_nospec() in
> raw_copy_from_user().
>
> Fourth patch removes the unused size parameter when enabling/disabling
> user access.
>
> Last patch implements masked user access.
>
> Christophe Leroy (5):
> uaccess: Add masked_user_{read/write}_access_begin
> uaccess: Add speculation barrier to copy_from_user_iter()
> powerpc: Remove unused size parametre to KUAP enabling/disabling
> functions
> powerpc: Move barrier_nospec() out of allow_read_{from/write}_user()
> powerpc: Implement masked user access
>
> arch/powerpc/Kconfig | 2 +-
> arch/powerpc/include/asm/book3s/32/kup.h | 2 +-
> arch/powerpc/include/asm/book3s/64/kup.h | 4 +-
> arch/powerpc/include/asm/kup.h | 24 ++--
> arch/powerpc/include/asm/nohash/32/kup-8xx.h | 2 +-
> arch/powerpc/include/asm/nohash/kup-booke.h | 2 +-
> arch/powerpc/include/asm/uaccess.h | 140 ++++++++++++++++---
> fs/select.c | 2 +-
> include/linux/uaccess.h | 8 ++
> kernel/futex/futex.h | 4 +-
> lib/iov_iter.c | 7 +
> lib/strncpy_from_user.c | 2 +-
> lib/strnlen_user.c | 2 +-
> 13 files changed, 158 insertions(+), 43 deletions(-)
>