On Wed Sep 21, 2022 at 4:56 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 <rmcl...@linux.ibm.com> > --- > V2: Update summary > V3: Remove erroneous summary paragraph on syscall_exit_prepare > V4: Use ZEROIZE instead of NULLIFY. Clear r0 also. > V5: Move to end of patch series. > V6: Include clears which were previously in the syscall wrapper patch. > Move comment on r3-r8 register save to when we alter the calling > convention for system_call_exception.
The series looks good to here, I just need to find a bit more time to look at the code and do some tests with the next few patches. I don't see much problem with them, looks a lot better now with fewer ifdefs so that's good. Possibly you could share some of those new sanitize macros in a header file but that's a minor nit. Coud we have this zeroize also under the same config option as the next? I figure if we care about speculative security we want both, and if we don't we need neither. Thanks, Nick > --- > arch/powerpc/kernel/interrupt_64.S | 17 ++++++++++++++++- > 1 file changed, 16 insertions(+), 1 deletion(-) > > diff --git a/arch/powerpc/kernel/interrupt_64.S > b/arch/powerpc/kernel/interrupt_64.S > index a5dd78bdbe6d..40147558e1a6 100644 > --- a/arch/powerpc/kernel/interrupt_64.S > +++ b/arch/powerpc/kernel/interrupt_64.S > @@ -106,6 +106,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_GPR(0) > + ZEROIZE_GPRS(5, 12) > + ZEROIZE_NVGPRS() > bl system_call_exception > > .Lsyscall_vectored_\name\()_exit: > @@ -134,6 +141,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 > > @@ -285,6 +293,13 @@ END_BTB_FLUSH_SECTION > wrteei 1 > #endif > > + /* > + * Zero user registers to prevent influencing speculative execution > + * state of kernel code. > + */ > + ZEROIZE_GPR(0) > + ZEROIZE_GPRS(5, 12) > + ZEROIZE_NVGPRS() > bl system_call_exception > > .Lsyscall_exit: > @@ -325,6 +340,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 */ > @@ -352,7 +368,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 > REST_GPR(0, r1) > -- > 2.34.1