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

[David Laight]

On Sun, 22 Jun 2025 11:52:38 +0200
Christophe Leroy <christophe.leroy@xxxxxxxxxx> 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(-)
>