Re: [PATCH v6 09/10] powerpc/64s: Implement KUAP for Radix MMU

[Christophe Leroy <christophe.leroy@c-s.fr>]

Le 18/04/2019 à 08:51, Michael Ellerman a écrit :
> Kernel Userspace Access Prevention utilises a feature of the Radix MMU
> which disallows read and write access to userspace addresses. By
> utilising this, the kernel is prevented from accessing user data from
> outside of trusted paths that perform proper safety checks, such as
> copy_{to/from}_user() and friends.
> 
> Userspace access is disabled from early boot and is only enabled when
> performing an operation like copy_{to/from}_user(). The register that
> controls this (AMR) does not prevent userspace from accessing itself,
> so there is no need to save and restore when entering and exiting
> userspace.
> 
> When entering the kernel from the kernel we save AMR and if it is not
> blocking user access (because eg. we faulted doing a user access) we
> reblock user access for the duration of the exception (ie. the page
> fault) and then restore the AMR when returning back to the kernel.
> 
> This feature can be tested by using the lkdtm driver (CONFIG_LKDTM=y)
> and performing the following:
> 
>    # (echo ACCESS_USERSPACE) > [debugfs]/provoke-crash/DIRECT
> 
> If enabled, this should send SIGSEGV to the thread.
> 
> We also add paranoid checking of AMR in switch and syscall return
> under CONFIG_PPC_KUAP_DEBUG.
> 
> Co-authored-by: Michael Ellerman <mpe@ellerman.id.au>
> Signed-off-by: Russell Currey <ruscur@russell.cc>
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>

[...]

> diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
> index 15c67d2c0534..7cc25389c6bd 100644
> --- a/arch/powerpc/kernel/entry_64.S
> +++ b/arch/powerpc/kernel/entry_64.S

[...]

> @@ -594,6 +606,8 @@ _GLOBAL(_switch)
>   	std	r23,_CCR(r1)
>   	std	r1,KSP(r3)	/* Set old stack pointer */
>   
> +	kuap_check_amr r9, r10
> +
>   	FLUSH_COUNT_CACHE
>   
>   	/*

I'm having a problem with this check. As you know I implemented the 
exact same check in _switch() in entry_32.S. After adding some printk 
inside an user_access_begin()/user_access_end() section, I started to 
get valid user accesses blocked by KUAP. Then I activated 
CONFIG_PPC_KUAP_DEBUG which led me to WARNINGs on this check.

This is due to schedule() being called by printk() inside the section 
where user access is unlocked. How is this supposed to work ? When 
scheduling via the decrementer interrupt, the value of the KUAP register 
is saved on stack at interrupt entry and the one from the new task is 
restored at interrupt exit, but I can't see where it is done when 
schedule() is called directly.

Did I miss something when implementing KUAP for PPC32 ?

Christophe