Re: [PATCH V2] powerpc/ptrace: Mitigate potential Spectre v1

["Gustavo A. R. Silva" <gustavo@embeddedor.com>]

On 1/30/19 6:46 AM, Breno Leitao wrote:
> 'regno' is directly controlled by user space, hence leading to a potential
> exploitation of the Spectre variant 1 vulnerability.
> 
> On PTRACE_SETREGS and PTRACE_GETREGS requests, user space passes the
> register number that would be read or written. This register number is
> called 'regno' which is part of the 'addr' syscall parameter.
> 
> This 'regno' value is checked against the maximum pt_regs structure size,
> and then used to dereference it, which matches the initial part of a
> Spectre v1 (and Spectre v1.1) attack. The dereferenced value, then,
> is returned to userspace in the GETREGS case.
> 
> This patch sanitizes 'regno' before using it to dereference pt_reg.
> 
> Notice that given that speculation windows are large, the policy is
> to kill the speculation on the first load and not worry if it can be
> completed with a dependent load/store [1].
> 
> [1] https://marc.info/?l=linux-kernel&m=152449131114778&w=2
> 
> Signed-off-by: Breno Leitao <leitao@debian.org>

Acked-by: Gustavo A. R. Silva <gustavo@embeddedor.com>

> ---
>  arch/powerpc/kernel/ptrace.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c
> index cdd5d1d3ae41..7535f89e08cd 100644
> --- a/arch/powerpc/kernel/ptrace.c
> +++ b/arch/powerpc/kernel/ptrace.c
> @@ -33,6 +33,7 @@
>  #include <linux/hw_breakpoint.h>
>  #include <linux/perf_event.h>
>  #include <linux/context_tracking.h>
> +#include <linux/nospec.h>
>  
>  #include <linux/uaccess.h>
>  #include <linux/pkeys.h>
> @@ -274,6 +275,8 @@ static int set_user_trap(struct task_struct *task, unsigned long trap)
>   */
>  int ptrace_get_reg(struct task_struct *task, int regno, unsigned long *data)
>  {
> +	unsigned int regs_max;
> +
>  	if ((task->thread.regs == NULL) || !data)
>  		return -EIO;
>  
> @@ -297,7 +300,9 @@ int ptrace_get_reg(struct task_struct *task, int regno, unsigned long *data)
>  	}
>  #endif
>  
> -	if (regno < (sizeof(struct user_pt_regs) / sizeof(unsigned long))) {
> +	regs_max = sizeof(struct user_pt_regs) / sizeof(unsigned long);
> +	if (regno < regs_max) {
> +		regno = array_index_nospec(regno, regs_max);
>  		*data = ((unsigned long *)task->thread.regs)[regno];
>  		return 0;
>  	}
> @@ -321,6 +326,7 @@ int ptrace_put_reg(struct task_struct *task, int regno, unsigned long data)
>  		return set_user_dscr(task, data);
>  
>  	if (regno <= PT_MAX_PUT_REG) {
> +		regno = array_index_nospec(regno, PT_MAX_PUT_REG + 1);
>  		((unsigned long *)task->thread.regs)[regno] = data;
>  		return 0;
>  	}
>