[PATCH kernel] powerpc/uaccess: Skip might_fault() when user access is enabled

[PATCH kernel] powerpc/uaccess: Skip might_fault() when user access is enabled

[Alexey Kardashevskiy]

The amount of code executed with enabled user space access (unlocked KUAP)
should be minimal. However with CONFIG_PROVE_LOCKING or
CONFIG_DEBUG_ATOMIC_SLEEP enabled, might_fault() may end up replaying
interrupts which in turn may access the user space and forget to restore
the KUAP state.

The problem places are strncpy_from_user (and similar) which unlock KUAP
and call unsafe_get_user -> __get_user_allowed -> __get_user_nocheck()
with do_allow=false to skip KUAP as the caller took care of it.

This changes __get_user_nocheck() to look at @do_allow to decide whether
to skip might_fault(). Since strncpy_from_user/etc call might_fault()
anyway before unlocking KUAP, there should be no visible change.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---


This an attempt to fix that KUAP restore problem from
"powerpc/kuap: Restore AMR after replaying soft interrupts".



---
 arch/powerpc/include/asm/uaccess.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h
index 501c9a79038c..cd6c0427a9e2 100644
--- a/arch/powerpc/include/asm/uaccess.h
+++ b/arch/powerpc/include/asm/uaccess.h
@@ -313,7 +313,7 @@ do {								\
 	__typeof__(size) __gu_size = (size);			\
 								\
 	__chk_user_ptr(__gu_addr);				\
-	if (!is_kernel_addr((unsigned long)__gu_addr))		\
+	if (!do_allow && !is_kernel_addr((unsigned long)__gu_addr)) \
 		might_fault();					\
 	barrier_nospec();					\
 	if (do_allow)								\
-- 
2.17.1

Re: [PATCH kernel] powerpc/uaccess: Skip might_fault() when user access is enabled

[Alexey Kardashevskiy]

On 02/02/2021 20:14, Alexey Kardashevskiy wrote:
> The amount of code executed with enabled user space access (unlocked KUAP)
> should be minimal. However with CONFIG_PROVE_LOCKING or
> CONFIG_DEBUG_ATOMIC_SLEEP enabled, might_fault() may end up replaying
> interrupts which in turn may access the user space and forget to restore
> the KUAP state.
> 
> The problem places are strncpy_from_user (and similar) which unlock KUAP
> and call unsafe_get_user -> __get_user_allowed -> __get_user_nocheck()
> with do_allow=false to skip KUAP as the caller took care of it.
> 
> This changes __get_user_nocheck() to look at @do_allow to decide whether
> to skip might_fault(). Since strncpy_from_user/etc call might_fault()
> anyway before unlocking KUAP, there should be no visible change.
> 
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
> 
> 
> This an attempt to fix that KUAP restore problem from
> "powerpc/kuap: Restore AMR after replaying soft interrupts".
> 
> 
> 
> ---
>   arch/powerpc/include/asm/uaccess.h | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h
> index 501c9a79038c..cd6c0427a9e2 100644
> --- a/arch/powerpc/include/asm/uaccess.h
> +++ b/arch/powerpc/include/asm/uaccess.h
> @@ -313,7 +313,7 @@ do {								\
>   	__typeof__(size) __gu_size = (size);			\
>   								\
>   	__chk_user_ptr(__gu_addr);				\
> -	if (!is_kernel_addr((unsigned long)__gu_addr))		\
> +	if (!do_allow && !is_kernel_addr((unsigned long)__gu_addr)) \


ah my bad, must be "if (do_allow..."


>   		might_fault();					\
>   	barrier_nospec();					\
>   	if (do_allow)								\
> 

-- 
Alexey