[PATCH v2 1/4] powerpc/64: Add INTERRUPT_SANITIZE_REGISTERS Kconfig

[PATCH v2 1/4] powerpc/64: Add INTERRUPT_SANITIZE_REGISTERS Kconfig

[Rohan McLure]

Add Kconfig option for enabling clearing of registers on arrival in an
interrupt handler. This reduces the speculation influence of registers
on kernel internals. The option will be consumed by 64-bit systems that
feature speculation and wish to implement this mitigation.

This patch only introduces the Kconfig option, no actual mitigations.

The primary overhead of this mitigation lies in an increased number of
registers that must be saved and restored by interrupt handlers on
Book3S systems. Enable by default on Book3E systems, which prior to
this patch eagerly save and restore register state, meaning that the
mitigation when implemented will have minimal overhead.

Acked-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/Kconfig | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 2ca5418457ed..9d3d20c6f365 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -529,6 +529,15 @@ config HOTPLUG_CPU
 
 	  Say N if you are unsure.
 
+config INTERRUPT_SANITIZE_REGISTERS
+	bool "Clear gprs on interrupt arrival"
+	depends on PPC64 && ARCH_HAS_SYSCALL_WRAPPER
+	default PPC_BOOK3E_64
+	help
+	  Reduce the influence of user register state on interrupt handlers and
+	  syscalls through clearing user state from registers before handling
+	  the exception.
+
 config PPC_QUEUED_SPINLOCKS
 	bool "Queued spinlocks" if EXPERT
 	depends on SMP
-- 
2.34.1

[PATCH v2 2/4] powerpc/64s: Clear gprs on interrupt routine entry on Book3S

[Rohan McLure]

Zero 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.

Zero 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.

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.

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/

v2: REST_NVGPRS should be conditional on mitigation in scv handler. Fix
improper multi-line preprocessor macro in interrupt_64.S
---
 arch/powerpc/kernel/exceptions-64s.S | 47 +++++++++++++++++++++-----
 arch/powerpc/kernel/interrupt_64.S   | 36 ++++++++++++++++++++
 2 files changed, 74 insertions(+), 9 deletions(-)

diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
index 651c36b056bd..0605018762d1 100644
--- a/arch/powerpc/kernel/exceptions-64s.S
+++ b/arch/powerpc/kernel/exceptions-64s.S
@@ -21,6 +21,19 @@
 #include <asm/feature-fixups.h>
 #include <asm/kup.h>
 
+/*
+ * macros for handling user register sanitisation
+ */
+#ifdef CONFIG_INTERRUPT_SANITIZE_REGISTERS
+#define SANITIZE_ZEROIZE_NVGPRS()	ZEROIZE_NVGPRS()
+#define SANITIZE_RESTORE_NVGPRS()	REST_NVGPRS(r1)
+#define HANDLER_RESTORE_NVGPRS()
+#else
+#define SANITIZE_ZEROIZE_NVGPRS()
+#define SANITIZE_RESTORE_NVGPRS()
+#define HANDLER_RESTORE_NVGPRS()	REST_NVGPRS(r1)
+#endif /* CONFIG_INTERRUPT_SANITIZE_REGISTERS */
+
 /*
  * Following are fixed section helper macros.
  *
@@ -111,6 +124,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 +190,9 @@ do_define_int n
 	.ifndef IKUAP
 		IKUAP=1
 	.endif
+	.ifndef IMSR_R12
+		IMSR_R12=0
+	.endif
 .endm
 
 /*
@@ -502,6 +519,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
@@ -544,8 +562,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
@@ -577,8 +601,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)
@@ -696,6 +720,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. */
@@ -1441,7 +1466,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
 
 
@@ -1667,7 +1692,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
 
 
@@ -1733,7 +1758,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
 
 
@@ -1751,6 +1776,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)
@@ -2164,7 +2190,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
 
 
@@ -2384,6 +2410,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)
@@ -2433,6 +2460,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)
@@ -2494,7 +2522,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
 
 
@@ -2522,7 +2550,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
 
 
@@ -2748,7 +2777,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 a019ed6fc839..a262fe6964f3 100644
--- a/arch/powerpc/kernel/interrupt_64.S
+++ b/arch/powerpc/kernel/interrupt_64.S
@@ -13,6 +13,20 @@
 #include <asm/ppc_asm.h>
 #include <asm/ptrace.h>
 
+/*
+ * 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_RESTORE_NVGPRS()	REST_NVGPRS(r1)
+#else
+#define SANITIZE_ZEROIZE_SYSCALL_GPRS()
+#define SANITIZE_RESTORE_NVGPRS()
+#endif /* CONFIG_INTERRUPT_SANITIZE_REGISTERS */
+
 	.align 7
 
 .macro DEBUG_SRR_VALID srr
@@ -96,6 +110,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 +143,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 +179,9 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
 	ld	r4,_LINK(r1)
 	ld	r5,_XER(r1)
 
+#ifndef CONFIG_INTERRUPT_SANITIZE_REGISTERS
 	REST_NVGPRS(r1)
+#endif
 	REST_GPR(0, r1)
 	mtcr	r2
 	mtctr	r3
@@ -275,6 +297,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 +342,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 +370,9 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
 .Lsyscall_restore_regs:
 	ld	r3,_CTR(r1)
 	ld	r4,_XER(r1)
+#ifndef CONFIG_INTERRUPT_SANITIZE_REGISTERS
 	REST_NVGPRS(r1)
+#endif
 	mtctr	r3
 	mtspr	SPRN_XER,r4
 	REST_GPR(0, r1)
@@ -408,9 +438,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 +456,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 +526,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:
@@ -585,6 +620,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.34.1

[PATCH v2 3/4] powerpc/64e: Clear gprs on interrupt routine entry on Book3E

[Rohan McLure]

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.

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 | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/kernel/exceptions-64e.S b/arch/powerpc/kernel/exceptions-64e.S
index 2f68fb2ee4fc..91d8019123c2 100644
--- a/arch/powerpc/kernel/exceptions-64e.S
+++ b/arch/powerpc/kernel/exceptions-64e.S
@@ -358,6 +358,11 @@ ret_from_mc_except:
 	std	r14,PACA_EXMC+EX_R14(r13);				    \
 	std	r15,PACA_EXMC+EX_R15(r13)
 
+#ifdef CONFIG_INTERRUPT_SANITIZE_REGISTERS
+#define SANITIZE_ZEROIZE_NVGPRS()	ZEROIZE_NVGPRS()
+#else
+#define SANITIZE_ZEROIZE_NVGPRS()
+#endif
 
 /* Core exception code for all exceptions except TLB misses. */
 #define EXCEPTION_COMMON_LVL(n, scratch, excf)				    \
@@ -394,7 +399,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.34.1

[PATCH v2 4/4] powerpc/64s: Sanitise user registers on interrupt in pseries

[Rohan McLure]

Cause pseries 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/
---
 arch/powerpc/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 9d3d20c6f365..2eb328b25e49 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -532,7 +532,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
 	help
 	  Reduce the influence of user register state on interrupt handlers and
 	  syscalls through clearing user state from registers before handling
-- 
2.34.1

Re: [PATCH v2 1/4] powerpc/64: Add INTERRUPT_SANITIZE_REGISTERS Kconfig

[Christophe Leroy]

Le 07/11/2022 à 04:31, Rohan McLure a écrit :
> Add Kconfig option for enabling clearing of registers on arrival in an
> interrupt handler. This reduces the speculation influence of registers
> on kernel internals. The option will be consumed by 64-bit systems that
> feature speculation and wish to implement this mitigation.
> 
> This patch only introduces the Kconfig option, no actual mitigations.

If that has to do with speculation, do we need a new Kconfig option ? 
Can't we use CONFIG_PPC_BARRIER_NOSPEC for that ?

> 
> The primary overhead of this mitigation lies in an increased number of
> registers that must be saved and restored by interrupt handlers on
> Book3S systems. Enable by default on Book3E systems, which prior to
> this patch eagerly save and restore register state, meaning that the
> mitigation when implemented will have minimal overhead.
> 
> Acked-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/Kconfig | 9 +++++++++
>   1 file changed, 9 insertions(+)
> 
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index 2ca5418457ed..9d3d20c6f365 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -529,6 +529,15 @@ config HOTPLUG_CPU
>   
>   	  Say N if you are unsure.
>   
> +config INTERRUPT_SANITIZE_REGISTERS
> +	bool "Clear gprs on interrupt arrival"
> +	depends on PPC64 && ARCH_HAS_SYSCALL_WRAPPER
> +	default PPC_BOOK3E_64
> +	help
> +	  Reduce the influence of user register state on interrupt handlers and
> +	  syscalls through clearing user state from registers before handling
> +	  the exception.
> +
>   config PPC_QUEUED_SPINLOCKS
>   	bool "Queued spinlocks" if EXPERT
>   	depends on SMP

Re: [PATCH v2 1/4] powerpc/64: Add INTERRUPT_SANITIZE_REGISTERS Kconfig

[Segher Boessenkool]

Hi!

On Mon, Nov 07, 2022 at 02:31:59PM +1100, Rohan McLure wrote:
> Add Kconfig option for enabling clearing of registers on arrival in an
> interrupt handler. This reduces the speculation influence of registers
> on kernel internals.

Assuming you are talking about existing PowerPC CPUs from the last 30
years:

There is no data speculation.  At all.  Ever.

There is branch prediction, but that is not influenced by register
contents, either (for any current CPUs at least).  (Except when you get
a flush because of a mispredict, but if this zeroing changes anything,
we will have used wild (but user controlled) values in the old
non-zeroing situation, and that is a much bigger problem itself already,
also for security!  This can be an unlikely kernel bug, or a very
unlikely compiler bug.)

All GPRs are renamed, always.  If you zero all GPRs on interrupt entry
(which is context synchronising, importantly), this will guarantee there
can be no timing influence from the GPRs, because all of the physical
registers depend on nothing that happened before.  So that is good, at
least it can give some peace of mind.  Except that this makes 30 new
registers in just a few cycles, which *itself* can cause stalls, if the
renaming things are still busy.  Context synchronising does not
necessarily help there, the renaming machinery can do stuff *after* an
insn completes.

I don't see how this helps anything.  If it does, "reduces speculation
influence" is not a good description of what it does, afaics?


Segher

Re: [PATCH v2 1/4] powerpc/64: Add INTERRUPT_SANITIZE_REGISTERS Kconfig

[Nicholas Piggin]

On Tue Nov 8, 2022 at 2:39 AM AEST, Segher Boessenkool wrote:
> Hi!
>
> On Mon, Nov 07, 2022 at 02:31:59PM +1100, Rohan McLure wrote:
> > Add Kconfig option for enabling clearing of registers on arrival in an
> > interrupt handler. This reduces the speculation influence of registers
> > on kernel internals.
>
> Assuming you are talking about existing PowerPC CPUs from the last 30
> years:
>
> There is no data speculation.  At all.  Ever.
>
> There is branch prediction, but that is not influenced by register
> contents, either (for any current CPUs at least).  (Except when you get
> a flush because of a mispredict, but if this zeroing changes anything,
> we will have used wild (but user controlled) values in the old
> non-zeroing situation, and that is a much bigger problem itself already,
> also for security!  This can be an unlikely kernel bug, or a very
> unlikely compiler bug.)

This is not about data speculation, it is about speculative execution
in kernel mode that uses architected value that were set by user, and
that value being used to influence a speculative gadget that can expose
data to user via a side channel.

> All GPRs are renamed, always.  If you zero all GPRs on interrupt entry
> (which is context synchronising, importantly), this will guarantee there
> can be no timing influence from the GPRs, because all of the physical
> registers depend on nothing that happened before. So that is good, at
> least it can give some peace of mind.  Except that this makes 30 new
> registers in just a few cycles, which *itself* can cause stalls, if the
> renaming things are still busy.

It will have some pipeline effect like any instruction I suppose. At
least in latest processors, zeroing idiom should release registers
AFAIK.

> Context synchronising does not
> necessarily help there, the renaming machinery can do stuff *after* an
> insn completes.

Possibly context synchronization does not push everything prior to
completion, certainly it does not drain prior stores from store queues
even if they had previously completed, so there can be things going on
there. Software does not really have the ability to do anything about
that though, so that's more of a hardware problem if that exposes a
security issue IMO. Or at least a separate issue if there is some
architecture that could deal with it.

Thanks,
Nick

Re: [PATCH v2 1/4] powerpc/64: Add INTERRUPT_SANITIZE_REGISTERS Kconfig

[Nicholas Piggin]

On Tue Nov 8, 2022 at 12:28 AM AEST, Christophe Leroy wrote:
>
>
> Le 07/11/2022 à 04:31, Rohan McLure a écrit :
> > Add Kconfig option for enabling clearing of registers on arrival in an
> > interrupt handler. This reduces the speculation influence of registers
> > on kernel internals. The option will be consumed by 64-bit systems that
> > feature speculation and wish to implement this mitigation.
> > 
> > This patch only introduces the Kconfig option, no actual mitigations.
>
> If that has to do with speculation, do we need a new Kconfig option ? 
> Can't we use CONFIG_PPC_BARRIER_NOSPEC for that ?

NOSPEC barrier adds runtime-patchable hardware barrier and that config
is a build implementation detail. Also that spec barrier is for bounds
checks speculation that is easy to get the kernel to do something like
speculatively branch to arbitrary address.

Interrupt/syscall register sanitization is more handwavy. It could be
a bandaid for cases where the above speculation barrier was missed
for exampel. But at some point, at least for syscalls, registers have to
contain some values influenced by userspace so if we were paranoid
we would have to put barriers before every branch while any registers
contained a value from userspace.

A security option menu might be a good idea though. There's some other
build time options like rop protection that we might want to add.

Thanks,
Nick

Re: [PATCH v2 2/4] powerpc/64s: Clear gprs on interrupt routine entry on Book3S

[Nicholas Piggin]

On Mon Nov 7, 2022 at 1:32 PM AEST, Rohan McLure wrote:
> Zero 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.
>
> Zero 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.
>
> 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.

I think it looks good. You could put those macros into a .h file shared
by exceptions-64s.S and interrupt_64.S. Also interrupt_64.S could use
the HANDLER_RESTORE_NVGPRS macro to kill a few ifdefs I think? The
IMSR_R12 change *could* be done in a separate patch, if you're doing
another spin... sorry for the late feedback.

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/
>
> v2: REST_NVGPRS should be conditional on mitigation in scv handler. Fix
> improper multi-line preprocessor macro in interrupt_64.S
> ---
>  arch/powerpc/kernel/exceptions-64s.S | 47 +++++++++++++++++++++-----
>  arch/powerpc/kernel/interrupt_64.S   | 36 ++++++++++++++++++++
>  2 files changed, 74 insertions(+), 9 deletions(-)
>
> diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
> index 651c36b056bd..0605018762d1 100644
> --- a/arch/powerpc/kernel/exceptions-64s.S
> +++ b/arch/powerpc/kernel/exceptions-64s.S
> @@ -21,6 +21,19 @@
>  #include <asm/feature-fixups.h>
>  #include <asm/kup.h>
>  
> +/*
> + * macros for handling user register sanitisation
> + */
> +#ifdef CONFIG_INTERRUPT_SANITIZE_REGISTERS
> +#define SANITIZE_ZEROIZE_NVGPRS()	ZEROIZE_NVGPRS()
> +#define SANITIZE_RESTORE_NVGPRS()	REST_NVGPRS(r1)
> +#define HANDLER_RESTORE_NVGPRS()
> +#else
> +#define SANITIZE_ZEROIZE_NVGPRS()
> +#define SANITIZE_RESTORE_NVGPRS()
> +#define HANDLER_RESTORE_NVGPRS()	REST_NVGPRS(r1)
> +#endif /* CONFIG_INTERRUPT_SANITIZE_REGISTERS */
> +
>  /*
>   * Following are fixed section helper macros.
>   *
> @@ -111,6 +124,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 +190,9 @@ do_define_int n
>  	.ifndef IKUAP
>  		IKUAP=1
>  	.endif
> +	.ifndef IMSR_R12
> +		IMSR_R12=0
> +	.endif
>  .endm
>  
>  /*
> @@ -502,6 +519,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
> @@ -544,8 +562,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
> @@ -577,8 +601,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)
> @@ -696,6 +720,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. */
> @@ -1441,7 +1466,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
>  
>  
> @@ -1667,7 +1692,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
>  
>  
> @@ -1733,7 +1758,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
>  
>  
> @@ -1751,6 +1776,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)
> @@ -2164,7 +2190,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
>  
>  
> @@ -2384,6 +2410,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)
> @@ -2433,6 +2460,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)
> @@ -2494,7 +2522,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
>  
>  
> @@ -2522,7 +2550,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
>  
>  
> @@ -2748,7 +2777,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 a019ed6fc839..a262fe6964f3 100644
> --- a/arch/powerpc/kernel/interrupt_64.S
> +++ b/arch/powerpc/kernel/interrupt_64.S
> @@ -13,6 +13,20 @@
>  #include <asm/ppc_asm.h>
>  #include <asm/ptrace.h>
>  
> +/*
> + * 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_RESTORE_NVGPRS()	REST_NVGPRS(r1)
> +#else
> +#define SANITIZE_ZEROIZE_SYSCALL_GPRS()
> +#define SANITIZE_RESTORE_NVGPRS()
> +#endif /* CONFIG_INTERRUPT_SANITIZE_REGISTERS */
> +
>  	.align 7
>  
>  .macro DEBUG_SRR_VALID srr
> @@ -96,6 +110,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 +143,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 +179,9 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
>  	ld	r4,_LINK(r1)
>  	ld	r5,_XER(r1)
>  
> +#ifndef CONFIG_INTERRUPT_SANITIZE_REGISTERS
>  	REST_NVGPRS(r1)
> +#endif
>  	REST_GPR(0, r1)
>  	mtcr	r2
>  	mtctr	r3
> @@ -275,6 +297,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 +342,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 +370,9 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
>  .Lsyscall_restore_regs:
>  	ld	r3,_CTR(r1)
>  	ld	r4,_XER(r1)
> +#ifndef CONFIG_INTERRUPT_SANITIZE_REGISTERS
>  	REST_NVGPRS(r1)
> +#endif
>  	mtctr	r3
>  	mtspr	SPRN_XER,r4
>  	REST_GPR(0, r1)
> @@ -408,9 +438,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 +456,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 +526,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:
> @@ -585,6 +620,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.34.1

Re: [PATCH v2 3/4] powerpc/64e: Clear gprs on interrupt routine entry on Book3E

[Nicholas Piggin]

On Mon Nov 7, 2022 at 1:32 PM AEST, Rohan McLure wrote:
> 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.

In the case of ret_from_crit_except and ret_from_mc_except, it looks
like those are restored by ret_from_level_except, so that's fine.
And fast_interrupt_return you added NVGPRS restore in the previous
patch too.

Maybe actually you could move that interrupt_64.h code that applies to
both 64s and 64e in patch 1. So then the 64s/e enablement patches are
independent and apply to exactly that subarch.

But code-wise I think this looks good.

Reviewed-by: Nicholas Piggin <npiggin@gmail.com>

> Mitigation defaults to enabled by INTERRUPT_SANITIZE_REGISTERS.
>
> 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 | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/kernel/exceptions-64e.S b/arch/powerpc/kernel/exceptions-64e.S
> index 2f68fb2ee4fc..91d8019123c2 100644
> --- a/arch/powerpc/kernel/exceptions-64e.S
> +++ b/arch/powerpc/kernel/exceptions-64e.S
> @@ -358,6 +358,11 @@ ret_from_mc_except:
>  	std	r14,PACA_EXMC+EX_R14(r13);				    \
>  	std	r15,PACA_EXMC+EX_R15(r13)
>  
> +#ifdef CONFIG_INTERRUPT_SANITIZE_REGISTERS
> +#define SANITIZE_ZEROIZE_NVGPRS()	ZEROIZE_NVGPRS()
> +#else
> +#define SANITIZE_ZEROIZE_NVGPRS()
> +#endif

Could possibly share these macros.

>  
>  /* Core exception code for all exceptions except TLB misses. */
>  #define EXCEPTION_COMMON_LVL(n, scratch, excf)				    \
> @@ -394,7 +399,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.34.1

Re: [PATCH v2 4/4] powerpc/64s: Sanitise user registers on interrupt in pseries

[Nicholas Piggin]

On Mon Nov 7, 2022 at 1:32 PM AEST, Rohan McLure wrote:
> Cause pseries 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.

For POWERNV as well?

Thanks,
Nick

>
> 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/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index 9d3d20c6f365..2eb328b25e49 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -532,7 +532,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
>  	help
>  	  Reduce the influence of user register state on interrupt handlers and
>  	  syscalls through clearing user state from registers before handling
> -- 
> 2.34.1