| Message ID | 20220824020548.62625-15-rmclure@linux.ibm.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | powerpc: Syscall wrapper and register clearing | expand |
On Wed Aug 24, 2022 at 12:05 PM AEST, Rohan McLure wrote: > Clear 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. > > Signed-off-by: Rohan McLure <rmclure@linux.ibm.com> > --- > V1 -> V2: Update summary > V2 -> V3: Remove erroneous summary paragraph on syscall_exit_prepare > V3 -> V4: Use ZEROIZE instead of NULLIFY > --- > arch/powerpc/kernel/interrupt_64.S | 22 ++++++++++++++++++---- > 1 file changed, 18 insertions(+), 4 deletions(-) > > diff --git a/arch/powerpc/kernel/interrupt_64.S b/arch/powerpc/kernel/interrupt_64.S > index 0178aeba3820..a8065209dd8c 100644 > --- a/arch/powerpc/kernel/interrupt_64.S > +++ b/arch/powerpc/kernel/interrupt_64.S > @@ -70,7 +70,7 @@ _ASM_NOKPROBE_SYMBOL(system_call_vectored_\name) > ld r2,PACATOC(r13) > mfcr r12 > li r11,0 > - /* Can we avoid saving r3-r8 in common case? */ > + /* Save syscall parameters in r3-r8 */ > std r3,GPR3(r1) > std r4,GPR4(r1) > std r5,GPR5(r1) > @@ -109,6 +109,13 @@ 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. > + */ > + ZEROIZE_GPRS(5, 12) > + ZEROIZE_NVGPRS() > + > /* Calling convention has r3 = orig r0, r4 = regs */ > mr r3,r0 Ther's not much reason for this to go right against system_call_exception. You could set it up above where you set r4, and zero r0 as well with the rest of the volatiles? I guess it'll be overwritten by mflr r0 right away... although that could be an implementation detail not everything requires it and mcount call gets noped out. So probably doesn't hurt to zero it, while we're being paranoid. Thanks, Nick > bl system_call_exception > @@ -139,6 +146,7 @@ BEGIN_FTR_SECTION > HMT_MEDIUM_LOW > END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR) > > + REST_NVGPRS(r1) > cmpdi r3,0 > bne .Lsyscall_vectored_\name\()_restore_regs > > @@ -181,7 +189,6 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR) > ld r4,_LINK(r1) > ld r5,_XER(r1) > > - REST_NVGPRS(r1) > ld r0,GPR0(r1) > mtcr r2 > mtctr r3 > @@ -249,7 +256,7 @@ END_BTB_FLUSH_SECTION > ld r2,PACATOC(r13) > mfcr r12 > li r11,0 > - /* Can we avoid saving r3-r8 in common case? */ > + /* Save syscall parameters in r3-r8 */ > std r3,GPR3(r1) > std r4,GPR4(r1) > std r5,GPR5(r1) > @@ -300,6 +307,13 @@ END_BTB_FLUSH_SECTION > wrteei 1 > #endif > > + /* > + * Zero user registers to prevent influencing speculative execution > + * state of kernel code. > + */ > + ZEROIZE_GPRS(5, 12) > + ZEROIZE_NVGPRS() > + > /* Calling convention has r3 = orig r0, r4 = regs */ > mr r3,r0 > bl system_call_exception > @@ -342,6 +356,7 @@ BEGIN_FTR_SECTION > stdcx. r0,0,r1 /* to clear the reservation */ > END_FTR_SECTION_IFCLR(CPU_FTR_STCX_CHECKS_ADDRESS) > > + REST_NVGPRS(r1) > cmpdi r3,0 > bne .Lsyscall_restore_regs > /* Zero volatile regs that may contain sensitive kernel data */ > @@ -377,7 +392,6 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR) > .Lsyscall_restore_regs: > ld r3,_CTR(r1) > ld r4,_XER(r1) > - REST_NVGPRS(r1) > mtctr r3 > mtspr SPRN_XER,r4 > ld r0,GPR0(r1) > -- > 2.34.1
diff --git a/arch/powerpc/kernel/interrupt_64.S b/arch/powerpc/kernel/interrupt_64.S index 0178aeba3820..a8065209dd8c 100644 --- a/arch/powerpc/kernel/interrupt_64.S +++ b/arch/powerpc/kernel/interrupt_64.S @@ -70,7 +70,7 @@ _ASM_NOKPROBE_SYMBOL(system_call_vectored_\name) ld r2,PACATOC(r13) mfcr r12 li r11,0 - /* Can we avoid saving r3-r8 in common case? */ + /* Save syscall parameters in r3-r8 */ std r3,GPR3(r1) std r4,GPR4(r1) std r5,GPR5(r1) @@ -109,6 +109,13 @@ 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. + */ + ZEROIZE_GPRS(5, 12) + ZEROIZE_NVGPRS() + /* Calling convention has r3 = orig r0, r4 = regs */ mr r3,r0 bl system_call_exception @@ -139,6 +146,7 @@ BEGIN_FTR_SECTION HMT_MEDIUM_LOW END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR) + REST_NVGPRS(r1) cmpdi r3,0 bne .Lsyscall_vectored_\name\()_restore_regs @@ -181,7 +189,6 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR) ld r4,_LINK(r1) ld r5,_XER(r1) - REST_NVGPRS(r1) ld r0,GPR0(r1) mtcr r2 mtctr r3 @@ -249,7 +256,7 @@ END_BTB_FLUSH_SECTION ld r2,PACATOC(r13) mfcr r12 li r11,0 - /* Can we avoid saving r3-r8 in common case? */ + /* Save syscall parameters in r3-r8 */ std r3,GPR3(r1) std r4,GPR4(r1) std r5,GPR5(r1) @@ -300,6 +307,13 @@ END_BTB_FLUSH_SECTION wrteei 1 #endif + /* + * Zero user registers to prevent influencing speculative execution + * state of kernel code. + */ + ZEROIZE_GPRS(5, 12) + ZEROIZE_NVGPRS() + /* Calling convention has r3 = orig r0, r4 = regs */ mr r3,r0 bl system_call_exception @@ -342,6 +356,7 @@ BEGIN_FTR_SECTION stdcx. r0,0,r1 /* to clear the reservation */ END_FTR_SECTION_IFCLR(CPU_FTR_STCX_CHECKS_ADDRESS) + REST_NVGPRS(r1) cmpdi r3,0 bne .Lsyscall_restore_regs /* Zero volatile regs that may contain sensitive kernel data */ @@ -377,7 +392,6 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR) .Lsyscall_restore_regs: ld r3,_CTR(r1) ld r4,_XER(r1) - REST_NVGPRS(r1) mtctr r3 mtspr SPRN_XER,r4 ld r0,GPR0(r1)
Clear 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. Signed-off-by: Rohan McLure <rmclure@linux.ibm.com> --- V1 -> V2: Update summary V2 -> V3: Remove erroneous summary paragraph on syscall_exit_prepare V3 -> V4: Use ZEROIZE instead of NULLIFY --- arch/powerpc/kernel/interrupt_64.S | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-)