* [PATCH v4 2/7] powerpc/64: Add interrupt register sanitisation macros
2022-11-29 4:43 [PATCH v4 1/7] powerpc/64: Add INTERRUPT_SANITIZE_REGISTERS Kconfig Rohan McLure
@ 2022-11-29 4:43 ` Rohan McLure
2022-11-29 9:40 ` Nicholas Piggin
2022-11-29 4:43 ` [PATCH v4 3/7] powerpc/64: Sanitise common exit code for interrupts Rohan McLure
` (4 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: Rohan McLure @ 2022-11-29 4:43 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Rohan McLure, npiggin
Include in asm/ppc_asm.h macros to be used in multiple successive
patches to implement zeroising architected registers in interrupt
handlers. Registers will be sanitised in this fashion in future patches
to reduce the speculation influence of user-controlled register values.
These mitigations will be configurable through the
CONFIG_INTERRUPT_SANITIZE_REGISTERS Kconfig option.
Included are macros for conditionally zeroising registers and restoring
as required with the mitigation enabled. With the mitigation disabled,
non-volatiles must be restored on demand at separate locations to
those required by the mitigation.
Signed-off-by: Rohan McLure <rmclure@linux.ibm.com>
---
v4: New patch
---
arch/powerpc/include/asm/ppc_asm.h | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/arch/powerpc/include/asm/ppc_asm.h b/arch/powerpc/include/asm/ppc_asm.h
index 753a2757bcd4..272b2795c36a 100644
--- a/arch/powerpc/include/asm/ppc_asm.h
+++ b/arch/powerpc/include/asm/ppc_asm.h
@@ -74,6 +74,23 @@
#define SAVE_GPR(n, base) SAVE_GPRS(n, n, base)
#define REST_GPR(n, base) REST_GPRS(n, n, base)
+/* macros for handling user register sanitisation */
+#ifdef CONFIG_INTERRUPT_SANITIZE_REGISTERS
+#define SANITIZE_ZEROIZE_SYSCALL_GPRS() ZEROIZE_GPR(0); \
+ ZEROIZE_GPRS(5, 12); \
+ ZEROIZE_NVGPRS()
+#define SANITIZE_ZEROIZE_INTERRUPT_NVGPRS() ZEROIZE_NVGPRS()
+#define SANITIZE_ZEROIZE_NVGPRS() ZEROIZE_NVGPRS()
+#define SANITIZE_RESTORE_NVGPRS() REST_NVGPRS(r1)
+#define HANDLER_RESTORE_NVGPRS()
+#else
+#define SANITIZE_ZEROIZE_INTERRUPT_NVGPRS()
+#define SANITIZE_ZEROIZE_SYSCALL_GPRS()
+#define SANITIZE_ZEROIZE_NVGPRS()
+#define SANITIZE_RESTORE_NVGPRS()
+#define HANDLER_RESTORE_NVGPRS() REST_NVGPRS(r1)
+#endif /* CONFIG_INTERRUPT_SANITIZE_REGISTERS */
+
#define SAVE_FPR(n, base) stfd n,8*TS_FPRWIDTH*(n)(base)
#define SAVE_2FPRS(n, base) SAVE_FPR(n, base); SAVE_FPR(n+1, base)
#define SAVE_4FPRS(n, base) SAVE_2FPRS(n, base); SAVE_2FPRS(n+2, base)
--
2.37.2
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v4 2/7] powerpc/64: Add interrupt register sanitisation macros
2022-11-29 4:43 ` [PATCH v4 2/7] powerpc/64: Add interrupt register sanitisation macros Rohan McLure
@ 2022-11-29 9:40 ` Nicholas Piggin
0 siblings, 0 replies; 12+ messages in thread
From: Nicholas Piggin @ 2022-11-29 9:40 UTC (permalink / raw)
To: Rohan McLure, linuxppc-dev
On Tue Nov 29, 2022 at 2:43 PM AEST, Rohan McLure wrote:
> Include in asm/ppc_asm.h macros to be used in multiple successive
> patches to implement zeroising architected registers in interrupt
> handlers. Registers will be sanitised in this fashion in future patches
> to reduce the speculation influence of user-controlled register values.
> These mitigations will be configurable through the
> CONFIG_INTERRUPT_SANITIZE_REGISTERS Kconfig option.
>
> Included are macros for conditionally zeroising registers and restoring
> as required with the mitigation enabled. With the mitigation disabled,
> non-volatiles must be restored on demand at separate locations to
> those required by the mitigation.
>
> Signed-off-by: Rohan McLure <rmclure@linux.ibm.com>
Thanks. You might just call them SANITIZE_NVGPRS() etc if it's not
functionally important that they're zero. But I don't mind long names
too much.
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> v4: New patch
> ---
> arch/powerpc/include/asm/ppc_asm.h | 17 +++++++++++++++++
> 1 file changed, 17 insertions(+)
>
> diff --git a/arch/powerpc/include/asm/ppc_asm.h b/arch/powerpc/include/asm/ppc_asm.h
> index 753a2757bcd4..272b2795c36a 100644
> --- a/arch/powerpc/include/asm/ppc_asm.h
> +++ b/arch/powerpc/include/asm/ppc_asm.h
> @@ -74,6 +74,23 @@
> #define SAVE_GPR(n, base) SAVE_GPRS(n, n, base)
> #define REST_GPR(n, base) REST_GPRS(n, n, base)
>
> +/* macros for handling user register sanitisation */
> +#ifdef CONFIG_INTERRUPT_SANITIZE_REGISTERS
> +#define SANITIZE_ZEROIZE_SYSCALL_GPRS() ZEROIZE_GPR(0); \
> + ZEROIZE_GPRS(5, 12); \
> + ZEROIZE_NVGPRS()
> +#define SANITIZE_ZEROIZE_INTERRUPT_NVGPRS() ZEROIZE_NVGPRS()
> +#define SANITIZE_ZEROIZE_NVGPRS() ZEROIZE_NVGPRS()
> +#define SANITIZE_RESTORE_NVGPRS() REST_NVGPRS(r1)
> +#define HANDLER_RESTORE_NVGPRS()
> +#else
> +#define SANITIZE_ZEROIZE_INTERRUPT_NVGPRS()
> +#define SANITIZE_ZEROIZE_SYSCALL_GPRS()
> +#define SANITIZE_ZEROIZE_NVGPRS()
> +#define SANITIZE_RESTORE_NVGPRS()
> +#define HANDLER_RESTORE_NVGPRS() REST_NVGPRS(r1)
> +#endif /* CONFIG_INTERRUPT_SANITIZE_REGISTERS */
> +
> #define SAVE_FPR(n, base) stfd n,8*TS_FPRWIDTH*(n)(base)
> #define SAVE_2FPRS(n, base) SAVE_FPR(n, base); SAVE_FPR(n+1, base)
> #define SAVE_4FPRS(n, base) SAVE_2FPRS(n, base); SAVE_2FPRS(n+2, base)
> --
> 2.37.2
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v4 3/7] powerpc/64: Sanitise common exit code for interrupts
2022-11-29 4:43 [PATCH v4 1/7] powerpc/64: Add INTERRUPT_SANITIZE_REGISTERS Kconfig Rohan McLure
2022-11-29 4:43 ` [PATCH v4 2/7] powerpc/64: Add interrupt register sanitisation macros Rohan McLure
@ 2022-11-29 4:43 ` Rohan McLure
2022-11-29 9:46 ` Nicholas Piggin
2022-11-29 4:43 ` [PATCH v4 4/7] powerpc/64s: IOption for MSR stored in r12 Rohan McLure
` (3 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: Rohan McLure @ 2022-11-29 4:43 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Rohan McLure, npiggin
Interrupt code is shared between Book3E/S 64-bit systems for interrupt
handlers. Ensure that exit code correctly restores non-volatile gprs on
each system when CONFIG_INTERRUPT_SANITIZE_REGISTERS is enabled.
Also introduce macros for clearing/restoring registers on interrupt
entry for when this configuration option is either disabled or enabled.
Signed-off-by: Rohan McLure <rmclure@linux.ibm.com>
---
v4: New patch
---
arch/powerpc/kernel/interrupt_64.S | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/arch/powerpc/kernel/interrupt_64.S b/arch/powerpc/kernel/interrupt_64.S
index 978a173eb339..1ef4fdef74fb 100644
--- a/arch/powerpc/kernel/interrupt_64.S
+++ b/arch/powerpc/kernel/interrupt_64.S
@@ -408,9 +408,11 @@ interrupt_return_\srr\()_user: /* make backtraces match the _kernel variant */
_ASM_NOKPROBE_SYMBOL(interrupt_return_\srr\()_user)
addi r3,r1,STACK_FRAME_OVERHEAD
bl interrupt_exit_user_prepare
+#ifndef CONFIG_INTERRUPT_SANITIZE_REGISTERS
cmpdi r3,0
bne- .Lrestore_nvgprs_\srr
.Lrestore_nvgprs_\srr\()_cont:
+#endif
std r1,PACA_EXIT_SAVE_R1(r13) /* save r1 for restart */
#ifdef CONFIG_PPC_BOOK3S
.Linterrupt_return_\srr\()_user_rst_start:
@@ -424,6 +426,7 @@ _ASM_NOKPROBE_SYMBOL(interrupt_return_\srr\()_user)
stb r11,PACAIRQHAPPENED(r13) # clear out possible HARD_DIS
.Lfast_user_interrupt_return_\srr\():
+ SANITIZE_RESTORE_NVGPRS()
#ifdef CONFIG_PPC_BOOK3S
.ifc \srr,srr
lbz r4,PACASRR_VALID(r13)
@@ -493,9 +496,11 @@ ALT_FTR_SECTION_END_IFCLR(CPU_FTR_STCX_CHECKS_ADDRESS)
b . /* prevent speculative execution */
.Linterrupt_return_\srr\()_user_rst_end:
+#ifndef CONFIG_INTERRUPT_SANITIZE_REGISTERS
.Lrestore_nvgprs_\srr\():
REST_NVGPRS(r1)
b .Lrestore_nvgprs_\srr\()_cont
+#endif
#ifdef CONFIG_PPC_BOOK3S
interrupt_return_\srr\()_user_restart:
@@ -576,6 +581,7 @@ _ASM_NOKPROBE_SYMBOL(interrupt_return_\srr\()_kernel)
stb r11,PACAIRQHAPPENED(r13) // clear the possible HARD_DIS
.Lfast_kernel_interrupt_return_\srr\():
+ SANITIZE_RESTORE_NVGPRS()
cmpdi cr1,r3,0
#ifdef CONFIG_PPC_BOOK3S
.ifc \srr,srr
--
2.37.2
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v4 3/7] powerpc/64: Sanitise common exit code for interrupts
2022-11-29 4:43 ` [PATCH v4 3/7] powerpc/64: Sanitise common exit code for interrupts Rohan McLure
@ 2022-11-29 9:46 ` Nicholas Piggin
0 siblings, 0 replies; 12+ messages in thread
From: Nicholas Piggin @ 2022-11-29 9:46 UTC (permalink / raw)
To: Rohan McLure, linuxppc-dev
On Tue Nov 29, 2022 at 2:43 PM AEST, Rohan McLure wrote:
> Interrupt code is shared between Book3E/S 64-bit systems for interrupt
> handlers. Ensure that exit code correctly restores non-volatile gprs on
> each system when CONFIG_INTERRUPT_SANITIZE_REGISTERS is enabled.
>
> Also introduce macros for clearing/restoring registers on interrupt
> entry for when this configuration option is either disabled or enabled.
>
> Signed-off-by: Rohan McLure <rmclure@linux.ibm.com>
> ---
> v4: New patch
> ---
> arch/powerpc/kernel/interrupt_64.S | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/arch/powerpc/kernel/interrupt_64.S b/arch/powerpc/kernel/interrupt_64.S
> index 978a173eb339..1ef4fdef74fb 100644
> --- a/arch/powerpc/kernel/interrupt_64.S
> +++ b/arch/powerpc/kernel/interrupt_64.S
> @@ -408,9 +408,11 @@ interrupt_return_\srr\()_user: /* make backtraces match the _kernel variant */
> _ASM_NOKPROBE_SYMBOL(interrupt_return_\srr\()_user)
> addi r3,r1,STACK_FRAME_OVERHEAD
> bl interrupt_exit_user_prepare
> +#ifndef CONFIG_INTERRUPT_SANITIZE_REGISTERS
> cmpdi r3,0
> bne- .Lrestore_nvgprs_\srr
> .Lrestore_nvgprs_\srr\()_cont:
> +#endif
Looks pretty good. You might add a comment here to say nvgprs are always
restored, in the sanitize case. Not that it's hard to grep for.
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
Thanks,
Nick
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v4 4/7] powerpc/64s: IOption for MSR stored in r12
2022-11-29 4:43 [PATCH v4 1/7] powerpc/64: Add INTERRUPT_SANITIZE_REGISTERS Kconfig Rohan McLure
2022-11-29 4:43 ` [PATCH v4 2/7] powerpc/64: Add interrupt register sanitisation macros Rohan McLure
2022-11-29 4:43 ` [PATCH v4 3/7] powerpc/64: Sanitise common exit code for interrupts Rohan McLure
@ 2022-11-29 4:43 ` Rohan McLure
2022-11-29 9:48 ` Nicholas Piggin
2022-11-29 4:43 ` [PATCH v4 5/7] powerpc/64s: Zeroise gprs on interrupt routine entry on Book3S Rohan McLure
` (2 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: Rohan McLure @ 2022-11-29 4:43 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Rohan McLure, npiggin
Interrupt handlers in asm/exceptions-64s.S contain a great deal of common
code produced by the GEN_COMMON macros. Currently, at the exit point of
the macro, r12 will contain the contents of the MSR. A future patch will
cause these macros to zeroise architected registers to avoid potential
speculation influence of user data.
Provide an IOption that signals that r12 must be retained, as the
interrupt handler assumes it to hold the contents of the MSR.
Signed-off-by: Rohan McLure <rmclure@linux.ibm.com>
---
v4: Split 64s register sanitisation commit to establish this IOption
---
arch/powerpc/kernel/exceptions-64s.S | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
index 5381a43e50fe..58d72db1d484 100644
--- a/arch/powerpc/kernel/exceptions-64s.S
+++ b/arch/powerpc/kernel/exceptions-64s.S
@@ -111,6 +111,7 @@ name:
#define ISTACK .L_ISTACK_\name\() /* Set regular kernel stack */
#define __ISTACK(name) .L_ISTACK_ ## name
#define IKUAP .L_IKUAP_\name\() /* Do KUAP lock */
+#define IMSR_R12 .L_IMSR_R12_\name\() /* Assumes MSR saved to r12 */
#define INT_DEFINE_BEGIN(n) \
.macro int_define_ ## n name
@@ -176,6 +177,9 @@ do_define_int n
.ifndef IKUAP
IKUAP=1
.endif
+ .ifndef IMSR_R12
+ IMSR_R12=0
+ .endif
.endm
/*
@@ -1751,6 +1755,7 @@ INT_DEFINE_BEGIN(fp_unavailable)
#ifdef CONFIG_KVM_BOOK3S_PR_POSSIBLE
IKVM_REAL=1
#endif
+ IMSR_R12=1
INT_DEFINE_END(fp_unavailable)
EXC_REAL_BEGIN(fp_unavailable, 0x800, 0x100)
@@ -2372,6 +2377,7 @@ INT_DEFINE_BEGIN(altivec_unavailable)
#ifdef CONFIG_KVM_BOOK3S_PR_POSSIBLE
IKVM_REAL=1
#endif
+ IMSR_R12=1
INT_DEFINE_END(altivec_unavailable)
EXC_REAL_BEGIN(altivec_unavailable, 0xf20, 0x20)
@@ -2421,6 +2427,7 @@ INT_DEFINE_BEGIN(vsx_unavailable)
#ifdef CONFIG_KVM_BOOK3S_PR_POSSIBLE
IKVM_REAL=1
#endif
+ IMSR_R12=1
INT_DEFINE_END(vsx_unavailable)
EXC_REAL_BEGIN(vsx_unavailable, 0xf40, 0x20)
--
2.37.2
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v4 4/7] powerpc/64s: IOption for MSR stored in r12
2022-11-29 4:43 ` [PATCH v4 4/7] powerpc/64s: IOption for MSR stored in r12 Rohan McLure
@ 2022-11-29 9:48 ` Nicholas Piggin
0 siblings, 0 replies; 12+ messages in thread
From: Nicholas Piggin @ 2022-11-29 9:48 UTC (permalink / raw)
To: Rohan McLure, linuxppc-dev
On Tue Nov 29, 2022 at 2:43 PM AEST, Rohan McLure wrote:
> Interrupt handlers in asm/exceptions-64s.S contain a great deal of common
> code produced by the GEN_COMMON macros. Currently, at the exit point of
> the macro, r12 will contain the contents of the MSR. A future patch will
> cause these macros to zeroise architected registers to avoid potential
> speculation influence of user data.
>
> Provide an IOption that signals that r12 must be retained, as the
> interrupt handler assumes it to hold the contents of the MSR.
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
>
> Signed-off-by: Rohan McLure <rmclure@linux.ibm.com>
> ---
> v4: Split 64s register sanitisation commit to establish this IOption
> ---
> arch/powerpc/kernel/exceptions-64s.S | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
> index 5381a43e50fe..58d72db1d484 100644
> --- a/arch/powerpc/kernel/exceptions-64s.S
> +++ b/arch/powerpc/kernel/exceptions-64s.S
> @@ -111,6 +111,7 @@ name:
> #define ISTACK .L_ISTACK_\name\() /* Set regular kernel stack */
> #define __ISTACK(name) .L_ISTACK_ ## name
> #define IKUAP .L_IKUAP_\name\() /* Do KUAP lock */
> +#define IMSR_R12 .L_IMSR_R12_\name\() /* Assumes MSR saved to r12 */
>
> #define INT_DEFINE_BEGIN(n) \
> .macro int_define_ ## n name
> @@ -176,6 +177,9 @@ do_define_int n
> .ifndef IKUAP
> IKUAP=1
> .endif
> + .ifndef IMSR_R12
> + IMSR_R12=0
> + .endif
> .endm
>
> /*
> @@ -1751,6 +1755,7 @@ INT_DEFINE_BEGIN(fp_unavailable)
> #ifdef CONFIG_KVM_BOOK3S_PR_POSSIBLE
> IKVM_REAL=1
> #endif
> + IMSR_R12=1
> INT_DEFINE_END(fp_unavailable)
>
> EXC_REAL_BEGIN(fp_unavailable, 0x800, 0x100)
> @@ -2372,6 +2377,7 @@ INT_DEFINE_BEGIN(altivec_unavailable)
> #ifdef CONFIG_KVM_BOOK3S_PR_POSSIBLE
> IKVM_REAL=1
> #endif
> + IMSR_R12=1
> INT_DEFINE_END(altivec_unavailable)
>
> EXC_REAL_BEGIN(altivec_unavailable, 0xf20, 0x20)
> @@ -2421,6 +2427,7 @@ INT_DEFINE_BEGIN(vsx_unavailable)
> #ifdef CONFIG_KVM_BOOK3S_PR_POSSIBLE
> IKVM_REAL=1
> #endif
> + IMSR_R12=1
> INT_DEFINE_END(vsx_unavailable)
>
> EXC_REAL_BEGIN(vsx_unavailable, 0xf40, 0x20)
> --
> 2.37.2
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v4 5/7] powerpc/64s: Zeroise gprs on interrupt routine entry on Book3S
2022-11-29 4:43 [PATCH v4 1/7] powerpc/64: Add INTERRUPT_SANITIZE_REGISTERS Kconfig Rohan McLure
` (2 preceding siblings ...)
2022-11-29 4:43 ` [PATCH v4 4/7] powerpc/64s: IOption for MSR stored in r12 Rohan McLure
@ 2022-11-29 4:43 ` Rohan McLure
2022-11-29 10:20 ` Nicholas Piggin
2022-11-29 4:43 ` [PATCH v4 6/7] powerpc/64e: Clear gprs on interrupt routine entry on Book3E Rohan McLure
2022-11-29 4:43 ` [PATCH v4 7/7] powerpc/64: Sanitise user registers on interrupt in pseries, POWERNV Rohan McLure
5 siblings, 1 reply; 12+ messages in thread
From: Rohan McLure @ 2022-11-29 4:43 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Rohan McLure, npiggin
Zeroise user state in gprs (assign to zero) to reduce the influence of user
registers on speculation within kernel syscall handlers. Clears occur
at the very beginning of the sc and scv 0 interrupt handlers, with
restores occurring following the execution of the syscall handler.
Zeroise GPRS r0, r2-r11, r14-r31, on entry into the kernel for all
other interrupt sources. The remaining gprs are overwritten by
entry macros to interrupt handlers, irrespective of whether or not a
given handler consumes these register values. If an interrupt does not
select the IMSR_R12 IOption, zeroise r12.
Prior to this commit, r14-r31 are restored on a per-interrupt basis at
exit, but now they are always restored on 64bit Book3S. Remove explicit
REST_NVGPRS invocations on 64-bit Book3S. 32-bit systems do not clear
user registers on interrupt, and continue to depend on the return value
of interrupt_exit_user_prepare to determine whether or not to restore
non-volatiles.
The mmap_bench benchmark in selftests should rapidly invoke pagefaults.
See ~0.8% performance regression with this mitigation, but this
indicates the worst-case performance due to heavier-weight interrupt
handlers. This mitigation is able to be enabled/disabled through
CONFIG_INTERRUPT_SANITIZE_REGISTERS.
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
Signed-off-by: Rohan McLure <rmclure@linux.ibm.com>
---
v2: REST_NVGPRS should be conditional on mitigation in scv handler. Fix
improper multi-line preprocessor macro in interrupt_64.S
v4: Split off IMSR_R12 definition into its own patch. Move macro
definitions for register sanitisation into asm/ppc_asm.h
---
arch/powerpc/kernel/exceptions-64s.S | 27 ++++++++++++++++++---------
arch/powerpc/kernel/interrupt_64.S | 16 ++++++++++++++--
2 files changed, 32 insertions(+), 11 deletions(-)
diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
index 58d72db1d484..8ee3db3b6595 100644
--- a/arch/powerpc/kernel/exceptions-64s.S
+++ b/arch/powerpc/kernel/exceptions-64s.S
@@ -506,6 +506,7 @@ DEFINE_FIXED_SYMBOL(\name\()_common_real, text)
std r10,0(r1) /* make stack chain pointer */
std r0,GPR0(r1) /* save r0 in stackframe */
std r10,GPR1(r1) /* save r1 in stackframe */
+ ZEROIZE_GPR(0)
/* Mark our [H]SRRs valid for return */
li r10,1
@@ -548,8 +549,14 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
std r9,GPR11(r1)
std r10,GPR12(r1)
std r11,GPR13(r1)
+ .if !IMSR_R12
+ ZEROIZE_GPRS(9, 12)
+ .else
+ ZEROIZE_GPRS(9, 11)
+ .endif
SAVE_NVGPRS(r1)
+ SANITIZE_ZEROIZE_NVGPRS()
.if IDAR
.if IISIDE
@@ -581,8 +588,8 @@ BEGIN_FTR_SECTION
END_FTR_SECTION_IFSET(CPU_FTR_CFAR)
ld r10,IAREA+EX_CTR(r13)
std r10,_CTR(r1)
- std r2,GPR2(r1) /* save r2 in stackframe */
- SAVE_GPRS(3, 8, r1) /* save r3 - r8 in stackframe */
+ SAVE_GPRS(2, 8, r1) /* save r2 - r8 in stackframe */
+ ZEROIZE_GPRS(2, 8)
mflr r9 /* Get LR, later save to stack */
LOAD_PACA_TOC() /* get kernel TOC into r2 */
std r9,_LINK(r1)
@@ -700,6 +707,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_CFAR)
mtlr r9
ld r9,_CCR(r1)
mtcr r9
+ SANITIZE_RESTORE_NVGPRS()
REST_GPRS(2, 13, r1)
REST_GPR(0, r1)
/* restore original r1. */
@@ -1445,7 +1453,7 @@ ALT_MMU_FTR_SECTION_END_IFCLR(MMU_FTR_TYPE_RADIX)
* do_break() may have changed the NV GPRS while handling a breakpoint.
* If so, we need to restore them with their updated values.
*/
- REST_NVGPRS(r1)
+ HANDLER_RESTORE_NVGPRS()
b interrupt_return_srr
@@ -1671,7 +1679,7 @@ EXC_COMMON_BEGIN(alignment_common)
GEN_COMMON alignment
addi r3,r1,STACK_FRAME_OVERHEAD
bl alignment_exception
- REST_NVGPRS(r1) /* instruction emulation may change GPRs */
+ HANDLER_RESTORE_NVGPRS() /* instruction emulation may change GPRs */
b interrupt_return_srr
@@ -1737,7 +1745,7 @@ EXC_COMMON_BEGIN(program_check_common)
.Ldo_program_check:
addi r3,r1,STACK_FRAME_OVERHEAD
bl program_check_exception
- REST_NVGPRS(r1) /* instruction emulation may change GPRs */
+ HANDLER_RESTORE_NVGPRS() /* instruction emulation may change GPRs */
b interrupt_return_srr
@@ -2169,7 +2177,7 @@ EXC_COMMON_BEGIN(emulation_assist_common)
GEN_COMMON emulation_assist
addi r3,r1,STACK_FRAME_OVERHEAD
bl emulation_assist_interrupt
- REST_NVGPRS(r1) /* instruction emulation may change GPRs */
+ HANDLER_RESTORE_NVGPRS() /* instruction emulation may change GPRs */
b interrupt_return_hsrr
@@ -2489,7 +2497,7 @@ EXC_COMMON_BEGIN(facility_unavailable_common)
GEN_COMMON facility_unavailable
addi r3,r1,STACK_FRAME_OVERHEAD
bl facility_unavailable_exception
- REST_NVGPRS(r1) /* instruction emulation may change GPRs */
+ HANDLER_RESTORE_NVGPRS() /* instruction emulation may change GPRs */
b interrupt_return_srr
@@ -2517,7 +2525,8 @@ EXC_COMMON_BEGIN(h_facility_unavailable_common)
GEN_COMMON h_facility_unavailable
addi r3,r1,STACK_FRAME_OVERHEAD
bl facility_unavailable_exception
- REST_NVGPRS(r1) /* XXX Shouldn't be necessary in practice */
+ /* XXX Shouldn't be necessary in practice */
+ HANDLER_RESTORE_NVGPRS()
b interrupt_return_hsrr
@@ -2743,7 +2752,7 @@ EXC_COMMON_BEGIN(altivec_assist_common)
addi r3,r1,STACK_FRAME_OVERHEAD
#ifdef CONFIG_ALTIVEC
bl altivec_assist_exception
- REST_NVGPRS(r1) /* instruction emulation may change GPRs */
+ HANDLER_RESTORE_NVGPRS() /* instruction emulation may change GPRs */
#else
bl unknown_exception
#endif
diff --git a/arch/powerpc/kernel/interrupt_64.S b/arch/powerpc/kernel/interrupt_64.S
index 1ef4fdef74fb..66a56659e8c8 100644
--- a/arch/powerpc/kernel/interrupt_64.S
+++ b/arch/powerpc/kernel/interrupt_64.S
@@ -96,6 +96,11 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
* but this is the best we can do.
*/
+ /*
+ * Zero user registers to prevent influencing speculative execution
+ * state of kernel code.
+ */
+ SANITIZE_ZEROIZE_SYSCALL_GPRS()
bl system_call_exception
.Lsyscall_vectored_\name\()_exit:
@@ -124,6 +129,7 @@ BEGIN_FTR_SECTION
HMT_MEDIUM_LOW
END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
+ SANITIZE_RESTORE_NVGPRS()
cmpdi r3,0
bne .Lsyscall_vectored_\name\()_restore_regs
@@ -159,7 +165,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
ld r4,_LINK(r1)
ld r5,_XER(r1)
- REST_NVGPRS(r1)
+ HANDLER_RESTORE_NVGPRS()
REST_GPR(0, r1)
mtcr r2
mtctr r3
@@ -275,6 +281,11 @@ END_BTB_FLUSH_SECTION
wrteei 1
#endif
+ /*
+ * Zero user registers to prevent influencing speculative execution
+ * state of kernel code.
+ */
+ SANITIZE_ZEROIZE_SYSCALL_GPRS()
bl system_call_exception
.Lsyscall_exit:
@@ -315,6 +326,7 @@ BEGIN_FTR_SECTION
stdcx. r0,0,r1 /* to clear the reservation */
END_FTR_SECTION_IFCLR(CPU_FTR_STCX_CHECKS_ADDRESS)
+ SANITIZE_RESTORE_NVGPRS()
cmpdi r3,0
bne .Lsyscall_restore_regs
/* Zero volatile regs that may contain sensitive kernel data */
@@ -342,7 +354,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
.Lsyscall_restore_regs:
ld r3,_CTR(r1)
ld r4,_XER(r1)
- REST_NVGPRS(r1)
+ HANDLER_RESTORE_NVGPRS()
mtctr r3
mtspr SPRN_XER,r4
REST_GPR(0, r1)
--
2.37.2
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v4 5/7] powerpc/64s: Zeroise gprs on interrupt routine entry on Book3S
2022-11-29 4:43 ` [PATCH v4 5/7] powerpc/64s: Zeroise gprs on interrupt routine entry on Book3S Rohan McLure
@ 2022-11-29 10:20 ` Nicholas Piggin
0 siblings, 0 replies; 12+ messages in thread
From: Nicholas Piggin @ 2022-11-29 10:20 UTC (permalink / raw)
To: Rohan McLure, linuxppc-dev
On Tue Nov 29, 2022 at 2:43 PM AEST, Rohan McLure wrote:
> Zeroise user state in gprs (assign to zero) to reduce the influence of user
> registers on speculation within kernel syscall handlers. Clears occur
> at the very beginning of the sc and scv 0 interrupt handlers, with
> restores occurring following the execution of the syscall handler.
>
> Zeroise GPRS r0, r2-r11, r14-r31, on entry into the kernel for all
> other interrupt sources. The remaining gprs are overwritten by
> entry macros to interrupt handlers, irrespective of whether or not a
> given handler consumes these register values. If an interrupt does not
> select the IMSR_R12 IOption, zeroise r12.
>
> Prior to this commit, r14-r31 are restored on a per-interrupt basis at
> exit, but now they are always restored on 64bit Book3S. Remove explicit
> REST_NVGPRS invocations on 64-bit Book3S. 32-bit systems do not clear
> user registers on interrupt, and continue to depend on the return value
> of interrupt_exit_user_prepare to determine whether or not to restore
> non-volatiles.
>
> The mmap_bench benchmark in selftests should rapidly invoke pagefaults.
> See ~0.8% performance regression with this mitigation, but this
> indicates the worst-case performance due to heavier-weight interrupt
> handlers. This mitigation is able to be enabled/disabled through
> CONFIG_INTERRUPT_SANITIZE_REGISTERS.
>
> Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
> Signed-off-by: Rohan McLure <rmclure@linux.ibm.com>
> ---
> v2: REST_NVGPRS should be conditional on mitigation in scv handler. Fix
> improper multi-line preprocessor macro in interrupt_64.S
> v4: Split off IMSR_R12 definition into its own patch. Move macro
> definitions for register sanitisation into asm/ppc_asm.h
> ---
> arch/powerpc/kernel/exceptions-64s.S | 27 ++++++++++++++++++---------
> arch/powerpc/kernel/interrupt_64.S | 16 ++++++++++++++--
> 2 files changed, 32 insertions(+), 11 deletions(-)
>
> diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
> index 58d72db1d484..8ee3db3b6595 100644
> --- a/arch/powerpc/kernel/exceptions-64s.S
> +++ b/arch/powerpc/kernel/exceptions-64s.S
> @@ -506,6 +506,7 @@ DEFINE_FIXED_SYMBOL(\name\()_common_real, text)
> std r10,0(r1) /* make stack chain pointer */
> std r0,GPR0(r1) /* save r0 in stackframe */
> std r10,GPR1(r1) /* save r1 in stackframe */
> + ZEROIZE_GPR(0)
>
> /* Mark our [H]SRRs valid for return */
> li r10,1
> @@ -548,8 +549,14 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
> std r9,GPR11(r1)
> std r10,GPR12(r1)
> std r11,GPR13(r1)
> + .if !IMSR_R12
> + ZEROIZE_GPRS(9, 12)
> + .else
> + ZEROIZE_GPRS(9, 11)
> + .endif
These unconditionally zero. Should be SANITIZE_ZEROIZE_GPRS()? Same
for a few more cases.
Hmm. r12 actually contains come-from-MSR here, which isn't really user
controlled. r9-r11 just got loaded with some user GPRs, but they're
the usual scratch registers and get used for other things later.
The whole interrupt entry file could probably use a spring clean, some
re-scheduling, data layout improvements, and more consistent use of
scratch registers... so I'm overly concerned about removing every
possible redundant instruction here if it makes things a bit harder
to follow. But we might be able to get rid of the above zeroize, with
a comment.
>
> SAVE_NVGPRS(r1)
> + SANITIZE_ZEROIZE_NVGPRS()
>
> .if IDAR
> .if IISIDE
> @@ -581,8 +588,8 @@ BEGIN_FTR_SECTION
> END_FTR_SECTION_IFSET(CPU_FTR_CFAR)
> ld r10,IAREA+EX_CTR(r13)
> std r10,_CTR(r1)
> - std r2,GPR2(r1) /* save r2 in stackframe */
> - SAVE_GPRS(3, 8, r1) /* save r3 - r8 in stackframe */
> + SAVE_GPRS(2, 8, r1) /* save r2 - r8 in stackframe */
> + ZEROIZE_GPRS(2, 8)
> mflr r9 /* Get LR, later save to stack */
> LOAD_PACA_TOC() /* get kernel TOC into r2 */
> std r9,_LINK(r1)
r2 gets zeroed then used again in LOAD_PACA_TOC too.
Otherwise looks pretty good... Although CTR doesn't get zeroed and I
suppose it could be speculatively used as a source (e.g., bctr). CRs
other than 0 and sometimes 1 too, they're probably a bit less important
than CTR though. We don't use TAR in the kernel so that one should be
okay to leave (maybe with a comment).
That can be done another time, I'd like to see the GPR sanitising
patches go in... It *might* just be a matter of one mtctr in the
case of !RELOCATABLE kernel though to get the ctr...
Still-Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
Thanks,
Nick
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v4 6/7] powerpc/64e: Clear gprs on interrupt routine entry on Book3E
2022-11-29 4:43 [PATCH v4 1/7] powerpc/64: Add INTERRUPT_SANITIZE_REGISTERS Kconfig Rohan McLure
` (3 preceding siblings ...)
2022-11-29 4:43 ` [PATCH v4 5/7] powerpc/64s: Zeroise gprs on interrupt routine entry on Book3S Rohan McLure
@ 2022-11-29 4:43 ` Rohan McLure
2022-11-29 4:43 ` [PATCH v4 7/7] powerpc/64: Sanitise user registers on interrupt in pseries, POWERNV Rohan McLure
5 siblings, 0 replies; 12+ messages in thread
From: Rohan McLure @ 2022-11-29 4:43 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Rohan McLure, npiggin
Zero GPRS r14-r31 on entry into the kernel for interrupt sources to
limit influence of user-space values in potential speculation gadgets.
Prior to this commit, all other GPRS are reassigned during the common
prologue to interrupt handlers and so need not be zeroised explicitly.
This may be done safely, without loss of register state prior to the
interrupt, as the common prologue saves the initial values of
non-volatiles, which are unconditionally restored in interrupt_64.S.
Mitigation defaults to enabled by INTERRUPT_SANITIZE_REGISTERS.
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
Signed-off-by: Rohan McLure <rmclure@linux.ibm.com>
---
Resubmitting patches as their own series after v6 partially merged:
Link: https://lore.kernel.org/all/166488988686.779920.13794870102696416283.b4-ty@ellerman.id.au/t/
---
arch/powerpc/kernel/exceptions-64e.S | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/kernel/exceptions-64e.S b/arch/powerpc/kernel/exceptions-64e.S
index 930e36099015..6b842d5ea4e1 100644
--- a/arch/powerpc/kernel/exceptions-64e.S
+++ b/arch/powerpc/kernel/exceptions-64e.S
@@ -358,7 +358,6 @@ ret_from_mc_except:
std r14,PACA_EXMC+EX_R14(r13); \
std r15,PACA_EXMC+EX_R15(r13)
-
/* Core exception code for all exceptions except TLB misses. */
#define EXCEPTION_COMMON_LVL(n, scratch, excf) \
exc_##n##_common: \
@@ -394,7 +393,8 @@ exc_##n##_common: \
std r12,STACK_FRAME_OVERHEAD-16(r1); /* mark the frame */ \
std r3,_TRAP(r1); /* set trap number */ \
std r0,RESULT(r1); /* clear regs->result */ \
- SAVE_NVGPRS(r1);
+ SAVE_NVGPRS(r1); \
+ SANITIZE_ZEROIZE_NVGPRS(); /* minimise speculation influence */
#define EXCEPTION_COMMON(n) \
EXCEPTION_COMMON_LVL(n, SPRN_SPRG_GEN_SCRATCH, PACA_EXGEN)
--
2.37.2
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v4 7/7] powerpc/64: Sanitise user registers on interrupt in pseries, POWERNV
2022-11-29 4:43 [PATCH v4 1/7] powerpc/64: Add INTERRUPT_SANITIZE_REGISTERS Kconfig Rohan McLure
` (4 preceding siblings ...)
2022-11-29 4:43 ` [PATCH v4 6/7] powerpc/64e: Clear gprs on interrupt routine entry on Book3E Rohan McLure
@ 2022-11-29 4:43 ` Rohan McLure
2022-11-29 10:21 ` Nicholas Piggin
5 siblings, 1 reply; 12+ messages in thread
From: Rohan McLure @ 2022-11-29 4:43 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Rohan McLure, npiggin
Cause pseries and POWERNV platforms to default to zeroising all potentially
user-defined registers when entering the kernel by means of any interrupt
source, reducing user-influence of the kernel and the likelihood or
producing speculation gadgets.
Signed-off-by: Rohan McLure <rmclure@linux.ibm.com>
---
Resubmitting patches as their own series after v6 partially merged:
Link: https://lore.kernel.org/all/166488988686.779920.13794870102696416283.b4-ty@ellerman.id.au/t/
v4: Default on POWERNV as well.
---
arch/powerpc/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 280c797e0f30..2ab114a02f62 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -534,7 +534,7 @@ config HOTPLUG_CPU
config INTERRUPT_SANITIZE_REGISTERS
bool "Clear gprs on interrupt arrival"
depends on PPC64 && ARCH_HAS_SYSCALL_WRAPPER
- default PPC_BOOK3E_64
+ default PPC_BOOK3E_64 || PPC_PSERIES || PPC_POWERNV
help
Reduce the influence of user register state on interrupt handlers and
syscalls through clearing user state from registers before handling
--
2.37.2
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v4 7/7] powerpc/64: Sanitise user registers on interrupt in pseries, POWERNV
2022-11-29 4:43 ` [PATCH v4 7/7] powerpc/64: Sanitise user registers on interrupt in pseries, POWERNV Rohan McLure
@ 2022-11-29 10:21 ` Nicholas Piggin
0 siblings, 0 replies; 12+ messages in thread
From: Nicholas Piggin @ 2022-11-29 10:21 UTC (permalink / raw)
To: Rohan McLure, linuxppc-dev
On Tue Nov 29, 2022 at 2:43 PM AEST, Rohan McLure wrote:
> Cause pseries and POWERNV platforms to default to zeroising all potentially
> user-defined registers when entering the kernel by means of any interrupt
> source, reducing user-influence of the kernel and the likelihood or
> producing speculation gadgets.
>
> Signed-off-by: Rohan McLure <rmclure@linux.ibm.com>
Acked-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> Resubmitting patches as their own series after v6 partially merged:
> Link: https://lore.kernel.org/all/166488988686.779920.13794870102696416283.b4-ty@ellerman.id.au/t/
> v4: Default on POWERNV as well.
> ---
> arch/powerpc/Kconfig | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index 280c797e0f30..2ab114a02f62 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -534,7 +534,7 @@ config HOTPLUG_CPU
> config INTERRUPT_SANITIZE_REGISTERS
> bool "Clear gprs on interrupt arrival"
> depends on PPC64 && ARCH_HAS_SYSCALL_WRAPPER
> - default PPC_BOOK3E_64
> + default PPC_BOOK3E_64 || PPC_PSERIES || PPC_POWERNV
> help
> Reduce the influence of user register state on interrupt handlers and
> syscalls through clearing user state from registers before handling
> --
> 2.37.2
^ permalink raw reply [flat|nested] 12+ messages in thread