diff mbox series

[2/2] powerpc/64s: interrupt entry use isel to prevent untrusted speculative r1 values used by the kernel

Message ID 20190820051106.15744-2-npiggin@gmail.com
State Under Review
Headers show
Series [1/2] powerpc/64s: remove support for kernel-mode syscalls | expand


Context Check Description
snowpatch_ozlabs/apply_patch success Successfully applied on branch next (c9633332103e55bc73d80d07ead28b95a22a85a3)
snowpatch_ozlabs/build-ppc64le success Build succeeded
snowpatch_ozlabs/build-ppc64be success Build succeeded
snowpatch_ozlabs/build-ppc64e success Build succeeded
snowpatch_ozlabs/build-pmac32 success Build succeeded
snowpatch_ozlabs/checkpatch success total: 0 errors, 0 warnings, 0 checks, 31 lines checked

Commit Message

Nicholas Piggin Aug. 20, 2019, 5:11 a.m. UTC
Interrupts may come from user or kernel, so the stack pointer needs to
be set to either the base of the kernel stack, or a new frame on the
existing kernel stack pointer, respectively.

Using a branch for this can lead to r1-indexed memory operations being
speculatively executed using a value of r1 controlled by userspace.
This is the first step to possible speculative execution vulnerability.

This does not appear to be a problem on its own, because loads from the
stack with this rogue address should only come from memory the kernel
previously stored to during the same speculative path, so they should
always be satisfied by the store buffers rather than exposing the
underlying memory contents.

There are some obscure cases where an r1-indexed load may be used in
other ways (e.g., stack unwinding), however they are rare and difficult
to control, and they still need to contain a sequence that subsequently
changes microarchitectural state based on the result of such a rogue
load, in a way that can be observed.

However it's safer to just close the concern at the first step, by
preventing untrusted speculative r1 value leaking into the kernel. Do
this by using isel to select the r1 value rather than a branch. isel
output is not predicted on POWER CPUs which support the instruction,
although this is not architecture.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
 arch/powerpc/kernel/exceptions-64s.S | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)
diff mbox series


diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
index 768f133de4f1..8282c01db83e 100644
--- a/arch/powerpc/kernel/exceptions-64s.S
+++ b/arch/powerpc/kernel/exceptions-64s.S
@@ -393,15 +393,29 @@  END_FTR_SECTION_NESTED(CPU_FTR_CFAR, CPU_FTR_CFAR, 66);			   \
  * On entry r13 points to the paca, r9-r13 are saved in the paca,
  * r9 contains the saved CR, r11 and r12 contain the saved SRR0 and
  * SRR1, and relocation is on.
+ *
+ * Using isel to select the r1 kernel stack depending on MSR_PR prevents
+ * speculative execution of memory ops with untrusted addresses (r1 from
+ * userspace) as a hardening measure, although there is no known vulnerability
+ * using a branch here instead. isel will not do value speculation on any POWER
+ * processor that implements it, although this is not currently documented.
 #define EXCEPTION_COMMON(area, trap)					   \
 	andi.	r10,r12,MSR_PR;		/* See if coming from user	*/ \
 	mr	r10,r1;			/* Save r1			*/ \
+	ld	r1,PACAKSAVE(r13);	/* base stack if from user	*/ \
+	addi	r1,r1,INT_FRAME_SIZE;	/* adjust for subi		*/ \
+	iseleq	r1,r10,r1;		/* original r1 if from kernel 	*/ \
+	subi	r1,r1,INT_FRAME_SIZE;	/* alloc frame on kernel stack	*/ \
+FTR_SECTION_ELSE							   \
 	subi	r1,r1,INT_FRAME_SIZE;	/* alloc frame on kernel stack	*/ \
 	beq-	1f;							   \
 	ld	r1,PACAKSAVE(r13);	/* kernel stack to use		*/ \
-1:	tdgei	r1,-INT_FRAME_SIZE;	/* trap if r1 is in userspace	*/ \
-	EMIT_BUG_ENTRY 1b,__FILE__,__LINE__,0;				   \
+1:									   \
+2:	tdgei	r1,-INT_FRAME_SIZE;	/* trap if r1 is in userspace	*/ \
+	EMIT_BUG_ENTRY 2b,__FILE__,__LINE__,0;				   \
 	kuap_save_amr_and_lock r9, r10, cr1, cr0;			   \
 	beq	4f;			/* if from kernel mode		*/ \