On Fri Sep 16, 2022 at 3:32 PM AEST, Rohan McLure wrote:
> Zero GPRS r0, r2-r11, r14-r31, on entry into the kernel for all
> other interrupt sources to limit influence of user-space values
> in potential speculation gadgets. 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. Remove explicit REST_NVGPRS
> invocations as non-volatiles must now always be restored. 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 disabled by default, but enabled with
> CONFIG_INTERRUPT_SANITIZE_REGISTERS.
>
> Signed-off-by: Rohan McLure <rmcl...@linux.ibm.com>
> ---
> V1 -> V2: Add benchmark data
> V2 -> V3: Use ZEROIZE_GPR{,S} macro renames, clarify
> interrupt_exit_user_prepare changes in summary.
> V4 -> V5: Configurable now with INTERRUPT_SANITIZE_REGISTERS. Zero r12
> (containing MSR) from common macro on per-interrupt basis with IOPTION.Thanks, this is looking pretty good. I'll have a closer look at the asm though. Possibly here and in some of the previous patches as well there could be some macros that hide some of the ifdefs. e.g., SANITIZE_ZEROIZE_NVGPRS. And HANDLER_RESTORE_NVGPRS() could do the restore if !SANITIZE, etc. Also I just realised you're Americanising arch/powerpc. Lizzy would not have been amused. Thanks, Nick > --- > arch/powerpc/kernel/exceptions-64s.S | 37 ++++++++++++++++++++++++-- > arch/powerpc/kernel/interrupt_64.S | 10 +++++++ > 2 files changed, 45 insertions(+), 2 deletions(-) > > diff --git a/arch/powerpc/kernel/exceptions-64s.S > b/arch/powerpc/kernel/exceptions-64s.S > index a3b51441b039..be5e72caada1 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 > > /* > @@ -502,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 > @@ -544,8 +549,16 @@ 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) > +#ifdef CONFIG_INTERRUPT_SANITIZE_REGISTERS > + ZEROIZE_NVGPRS() > +#endif > > .if IDAR > .if IISIDE > @@ -577,8 +590,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 */ > ld r2,PACATOC(r13) /* get kernel TOC into r2 */ > std r9,_LINK(r1) > @@ -696,6 +709,9 @@ END_FTR_SECTION_IFSET(CPU_FTR_CFAR) > mtlr r9 > ld r9,_CCR(r1) > mtcr r9 > +#ifdef CONFIG_INTERRUPT_SANITIZE_REGISTERS > + REST_NVGPRS(r1) > +#endif > REST_GPRS(2, 13, r1) > REST_GPR(0, r1) > /* restore original r1. */ > @@ -1368,11 +1384,13 @@ ALT_MMU_FTR_SECTION_END_IFCLR(MMU_FTR_TYPE_RADIX) > b interrupt_return_srr > > 1: bl do_break > +#ifndef CONFIG_INTERRUPT_SANITIZE_REGISTERS > /* > * 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) > +#endif > b interrupt_return_srr > > > @@ -1598,7 +1616,9 @@ EXC_COMMON_BEGIN(alignment_common) > GEN_COMMON alignment > addi r3,r1,STACK_FRAME_OVERHEAD > bl alignment_exception > +#ifndef CONFIG_INTERRUPT_SANITIZE_REGISTERS > REST_NVGPRS(r1) /* instruction emulation may change GPRs */ > +#endif > b interrupt_return_srr > > > @@ -1708,7 +1728,9 @@ EXC_COMMON_BEGIN(program_check_common) > .Ldo_program_check: > addi r3,r1,STACK_FRAME_OVERHEAD > bl program_check_exception > +#ifndef CONFIG_INTERRUPT_SANITIZE_REGISTERS > REST_NVGPRS(r1) /* instruction emulation may change GPRs */ > +#endif > b interrupt_return_srr > > > @@ -1726,6 +1748,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) > @@ -2139,7 +2162,9 @@ EXC_COMMON_BEGIN(emulation_assist_common) > GEN_COMMON emulation_assist > addi r3,r1,STACK_FRAME_OVERHEAD > bl emulation_assist_interrupt > +#ifndef CONFIG_INTERRUPT_SANITIZE_REGISTERS > REST_NVGPRS(r1) /* instruction emulation may change GPRs */ > +#endif > b interrupt_return_hsrr > > > @@ -2347,6 +2372,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) > @@ -2396,6 +2422,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) > @@ -2457,7 +2484,9 @@ EXC_COMMON_BEGIN(facility_unavailable_common) > GEN_COMMON facility_unavailable > addi r3,r1,STACK_FRAME_OVERHEAD > bl facility_unavailable_exception > +#ifndef CONFIG_INTERRUPT_SANITIZE_REGISTERS > REST_NVGPRS(r1) /* instruction emulation may change GPRs */ > +#endif > b interrupt_return_srr > > > @@ -2485,7 +2514,9 @@ EXC_COMMON_BEGIN(h_facility_unavailable_common) > GEN_COMMON h_facility_unavailable > addi r3,r1,STACK_FRAME_OVERHEAD > bl facility_unavailable_exception > +#ifndef CONFIG_INTERRUPT_SANITIZE_REGISTERS > REST_NVGPRS(r1) /* XXX Shouldn't be necessary in practice */ > +#endif > b interrupt_return_hsrr > > > @@ -2711,7 +2742,9 @@ EXC_COMMON_BEGIN(altivec_assist_common) > addi r3,r1,STACK_FRAME_OVERHEAD > #ifdef CONFIG_ALTIVEC > bl altivec_assist_exception > +#ifndef CONFIG_INTERRUPT_SANITIZE_REGISTERS > REST_NVGPRS(r1) /* instruction emulation may change GPRs */ > +#endif > #else > bl unknown_exception > #endif > diff --git a/arch/powerpc/kernel/interrupt_64.S > b/arch/powerpc/kernel/interrupt_64.S > index 40147558e1a6..edad0c17e47a 100644 > --- a/arch/powerpc/kernel/interrupt_64.S > +++ b/arch/powerpc/kernel/interrupt_64.S > @@ -433,9 +433,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: > @@ -449,6 +451,9 @@ _ASM_NOKPROBE_SYMBOL(interrupt_return_\srr\()_user) > stb r11,PACAIRQHAPPENED(r13) # clear out possible HARD_DIS > > .Lfast_user_interrupt_return_\srr\(): > +#ifdef CONFIG_INTERRUPT_SANITIZE_REGISTERS > + REST_NVGPRS(r1) > +#endif > #ifdef CONFIG_PPC_BOOK3S > .ifc \srr,srr > lbz r4,PACASRR_VALID(r13) > @@ -518,9 +523,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: > @@ -562,6 +569,9 @@ _ASM_NOKPROBE_SYMBOL(interrupt_return_\srr\()_kernel) > 1: > > .Lfast_kernel_interrupt_return_\srr\(): > +#ifdef CONFIG_INTERRUPT_SANITIZE_REGISTERS > + REST_NVGPRS(r1) > +#endif > cmpdi cr1,r3,0 > #ifdef CONFIG_PPC_BOOK3S > .ifc \srr,srr > -- > 2.34.1
