Re: [PATCH 1/2] arm64: uaccess: disable preempt during uaccess through ttbr0

[Catalin Marinas <catalin.marinas@arm.com>]

On Tue, Dec 21, 2021 at 09:42:41PM +0800, Pingfan Liu wrote:
> If using software PAN, the ttbr0 should keep unchanged, otherwise,
> considering the following scenario:
>   task1
>     __uaccess_ttbr0_enable()
>     switch_mm(this,next,tsk), which resets ttbr0 to __pa_symbol(reserved_pg_dir)
>     switch_mm(prev,this,tsk), which can not re-install the user page table automatically

Have you found a real problem with this in practice or just by code
inspection?

The assumption is that during uaccess_ttbr0_enable/disable regions, the
only way to get into switch_mm() is as a result of a page fault or
interrupt. The __swpan_{entry,exit}_el1 functions should take care of
restoring ttbr0 when returning to the interrupted context.

> Tackle this issue by disabling preemption.
> 
> Signed-off-by: Pingfan Liu <kernelfans@gmail.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Vincenzo Frascino <vincenzo.frascino@arm.com>
> Cc: Andrey Konovalov <andreyknvl@gmail.com>
> Cc: Arnd Bergmann <arnd@arndb.de>
> To: linux-arm-kernel@lists.infradead.org
> ---
>  arch/arm64/include/asm/uaccess.h | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/arch/arm64/include/asm/uaccess.h b/arch/arm64/include/asm/uaccess.h
> index 3a5ff5e20586..406888877bbd 100644
> --- a/arch/arm64/include/asm/uaccess.h
> +++ b/arch/arm64/include/asm/uaccess.h
> @@ -121,6 +121,7 @@ static inline bool uaccess_ttbr0_disable(void)
>  	if (!system_uses_ttbr0_pan())
>  		return false;
>  	__uaccess_ttbr0_disable();
> +	preempt_enable();
>  	return true;
>  }
>  
> @@ -128,6 +129,7 @@ static inline bool uaccess_ttbr0_enable(void)
>  {
>  	if (!system_uses_ttbr0_pan())
>  		return false;
> +	preempt_disable();
>  	__uaccess_ttbr0_enable();
>  	return true;
>  }

preempt_disable() won't help much here if, for example, the subsequent
uaccess gets a fault and need to sleep until the accessed page gets
available. I suspect you'd get some sleeping in atomic warning as well
with the right debug options enabled.

-- 
Catalin

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel