LinuxPPC-Dev Archive on lore.kernel.org
 help / Atom feed
* [PATCH V2] powerpc/ptrace: Mitigate potential Spectre v1
@ 2019-01-30 12:46 Breno Leitao
  2019-01-30 15:55 ` Gustavo A. R. Silva
  2019-02-08 13:02 ` [V2] " Michael Ellerman
  0 siblings, 2 replies; 3+ messages in thread
From: Breno Leitao @ 2019-01-30 12:46 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Breno Leitao, gustavo

'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>
---
 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;
 	}
-- 
2.19.0


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH V2] powerpc/ptrace: Mitigate potential Spectre v1
  2019-01-30 12:46 [PATCH V2] powerpc/ptrace: Mitigate potential Spectre v1 Breno Leitao
@ 2019-01-30 15:55 ` Gustavo A. R. Silva
  2019-02-08 13:02 ` [V2] " Michael Ellerman
  1 sibling, 0 replies; 3+ messages in thread
From: Gustavo A. R. Silva @ 2019-01-30 15:55 UTC (permalink / raw)
  To: Breno Leitao, linuxppc-dev



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;
>  	}
> 

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [V2] powerpc/ptrace: Mitigate potential Spectre v1
  2019-01-30 12:46 [PATCH V2] powerpc/ptrace: Mitigate potential Spectre v1 Breno Leitao
  2019-01-30 15:55 ` Gustavo A. R. Silva
@ 2019-02-08 13:02 ` " Michael Ellerman
  1 sibling, 0 replies; 3+ messages in thread
From: Michael Ellerman @ 2019-02-08 13:02 UTC (permalink / raw)
  To: Breno Leitao, linuxppc-dev; +Cc: Breno Leitao, gustavo

On Wed, 2019-01-30 at 12:46:00 UTC, 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>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/ebb0e13ead2ddc186a80b1b0235deeef

cheers

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, back to index

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-30 12:46 [PATCH V2] powerpc/ptrace: Mitigate potential Spectre v1 Breno Leitao
2019-01-30 15:55 ` Gustavo A. R. Silva
2019-02-08 13:02 ` [V2] " Michael Ellerman

LinuxPPC-Dev Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linuxppc-dev/0 linuxppc-dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linuxppc-dev linuxppc-dev/ https://lore.kernel.org/linuxppc-dev \
		linuxppc-dev@lists.ozlabs.org linuxppc-dev@ozlabs.org linuxppc-dev@archiver.kernel.org
	public-inbox-index linuxppc-dev


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.ozlabs.lists.linuxppc-dev


AGPL code for this site: git clone https://public-inbox.org/ public-inbox