[PATCH v2 4/4] arm64/syscalls: Specific usage of verify_pre_usermode_state

[Mark Rutland]

On Wed, Mar 08, 2017 at 05:24:56PM -0800, Thomas Garnier wrote:
> Implement specific usage of verify_pre_usermode_state for user-mode
> returns for arm64.
> ---
> Based on next-20170308
> ---
>  arch/arm64/Kconfig        |  1 +
>  arch/arm64/kernel/entry.S | 15 +++++++++++++++
>  2 files changed, 16 insertions(+)
> 
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 896eba61e5ed..da54774838d8 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -24,6 +24,7 @@ config ARM64
>  	select ARCH_WANT_COMPAT_IPC_PARSE_VERSION
>  	select ARCH_WANT_FRAME_POINTERS
>  	select ARCH_HAS_UBSAN_SANITIZE_ALL
> +	select ARCH_NO_SYSCALL_VERIFY_PRE_USERMODE_STATE
>  	select ARM_AMBA
>  	select ARM_ARCH_TIMER
>  	select ARM_GIC
> diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
> index 43512d4d7df2..eca392ae63e9 100644
> --- a/arch/arm64/kernel/entry.S
> +++ b/arch/arm64/kernel/entry.S
> @@ -737,6 +737,19 @@ ENTRY(cpu_switch_to)
>  	ret
>  ENDPROC(cpu_switch_to)
>  
> +#ifdef CONFIG_BUG_ON_DATA_CORRUPTION
> +.macro VERIFY_PRE_USERMODE_STATE
> +	bl	verify_pre_usermode_state
> +.endm
> +#else

We generally stick to lower case for the arm64 assembly macros. If we
need this, we should stick to the existing convention.

> +/* Similar to set_fs(USER_DS) in verify_pre_usermode_state without a warning. */
> +.macro VERIFY_PRE_USERMODE_STATE
> +	mov	x1, #TASK_SIZE_64
> +	str	x1, [tsk, #TSK_TI_ADDR_LIMIT]
> +.endm

We need arm64's set_fs() to configure UAO, too, so this is much weaker
than set_fs(), and will leave __{get,put}_user and
__copy_{to,from}_user() able to access kernel memory.

We don't currently have an asm helper to clear UAO, and unconditionally
poking that on exception return is liable to be somewhat expensive.

Also, given we're only trying to catch this in syscalls, I'm afraid I
don't see what we gain by doing this in the entry assembly.

Thanks,
Mark.

> +#endif
> +
> +
>  /*
>   * This is the fast syscall return path.  We do as little as possible here,
>   * and this includes saving x0 back into the kernel stack.
> @@ -744,6 +757,7 @@ ENDPROC(cpu_switch_to)
>  ret_fast_syscall:
>  	disable_irq				// disable interrupts
>  	str	x0, [sp, #S_X0]			// returned x0
> +	VERIFY_PRE_USERMODE_STATE
>  	ldr	x1, [tsk, #TSK_TI_FLAGS]	// re-check for syscall tracing
>  	and	x2, x1, #_TIF_SYSCALL_WORK
>  	cbnz	x2, ret_fast_syscall_trace
> @@ -771,6 +785,7 @@ work_pending:
>   */
>  ret_to_user:
>  	disable_irq				// disable interrupts
> +	VERIFY_PRE_USERMODE_STATE
>  	ldr	x1, [tsk, #TSK_TI_FLAGS]
>  	and	x2, x1, #_TIF_WORK_MASK
>  	cbnz	x2, work_pending
> -- 
> 2.12.0.246.ga2ecc84866-goog
>