| Message ID | 20220725063111.120926-1-rmclure@linux.ibm.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | powerpc: Syscall wrapper and register clearing | expand |
On Mon, 2022-07-25 at 16:31 +1000, 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. > > One function of syscall_exit_prepare is to determine when non- > volatile > regs must be restored, and it still serves that purpose on 32-bit. > Use > it now for determining where to find XER, CTR, CR. I'm not sure exactly how syscall_exit_prepare comes into this? > > Signed-off-by: Rohan McLure <rmclure@linux.ibm.com> > --- > V1 -> V2: Update summary > --- > 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 3e8a811e09c4..34167cfa5d60 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) > @@ -108,6 +108,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. > + */ > + NULLIFY_GPRS(5, 12) > + NULLIFY_NVGPRS() > + > /* Calling convention has r3 = orig r0, r4 = regs */ > mr r3,r0 > bl system_call_exception > @@ -138,6 +145,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 > > @@ -180,7 +188,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 > @@ -248,7 +255,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) > @@ -298,6 +305,13 @@ END_BTB_FLUSH_SECTION > wrteei 1 > #endif > > + /* > + * Zero user registers to prevent influencing speculative > execution > + * state of kernel code. > + */ > + NULLIFY_GPRS(5, 12) > + NULLIFY_NVGPRS() > + > /* Calling convention has r3 = orig r0, r4 = regs */ > mr r3,r0 > bl system_call_exception > @@ -340,6 +354,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 > */ > @@ -367,7 +382,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)
Hi! On Mon, Jul 25, 2022 at 04:31:11PM +1000, Rohan McLure wrote: > + /* > + * Zero user registers to prevent influencing speculative execution > + * state of kernel code. > + */ > + NULLIFY_GPRS(5, 12) > + NULLIFY_NVGPRS() "Nullify" means "invalidate", which is not what this does or is for :-( Segher
Le 11/08/2022 à 17:13, Segher Boessenkool a écrit : > Hi! > > On Mon, Jul 25, 2022 at 04:31:11PM +1000, Rohan McLure wrote: >> + /* >> + * Zero user registers to prevent influencing speculative execution >> + * state of kernel code. >> + */ >> + NULLIFY_GPRS(5, 12) >> + NULLIFY_NVGPRS() > > "Nullify" means "invalidate", which is not what this does or is for :-( > Would "Zeroise" be more appropriate ? Christophe
On Thu, Aug 11, 2022 at 03:39:58PM +0000, Christophe Leroy wrote: > > > Le 11/08/2022 à 17:13, Segher Boessenkool a écrit : > > Hi! > > > > On Mon, Jul 25, 2022 at 04:31:11PM +1000, Rohan McLure wrote: > >> + /* > >> + * Zero user registers to prevent influencing speculative execution > >> + * state of kernel code. > >> + */ > >> + NULLIFY_GPRS(5, 12) > >> + NULLIFY_NVGPRS() > > > > "Nullify" means "invalidate", which is not what this does or is for :-( > > Would "Zeroise" be more appropriate ? That is probably a good compromise, yes. It obviously is a verb, its meaning is clear and unamiguous, and there is precedent for it even :-) Thanks, Segher
>>> "Nullify" means "invalidate", which is not what this does or is for :-( >> >> Would "Zeroise" be more appropriate ? > > That is probably a good compromise, yes. It obviously is a verb, its > meaning is clear and unamiguous, and there is precedent for it even :-) Zeroise it is. The ‘zeroize’ spelling exists already in the kernel so I’ll use that. Rohan
> On 11 Aug 2022, at 8:11 pm, Andrew Donnellan <ajd@linux.ibm.com> wrote: > > On Mon, 2022-07-25 at 16:31 +1000, 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. >> >> One function of syscall_exit_prepare is to determine when non- >> volatile >> regs must be restored, and it still serves that purpose on 32-bit. >> Use >> it now for determining where to find XER, CTR, CR. > > I'm not sure exactly how syscall_exit_prepare comes into this? Apologies, this comment belongs in patch 14 and concerns interrupt_exit_user_prepare.
Le 15/08/2022 à 01:59, Rohan McLure a écrit : >>>> "Nullify" means "invalidate", which is not what this does or is for :-( >>> >>> Would "Zeroise" be more appropriate ? >> >> That is probably a good compromise, yes. It obviously is a verb, its >> meaning is clear and unamiguous, and there is precedent for it even :-) > > Zeroise it is. The ‘zeroize’ spelling exists already in the kernel so I’ll use that. I have some preference for British spelling over American spelling, but never mind. Christophe
diff --git a/arch/powerpc/kernel/interrupt_64.S b/arch/powerpc/kernel/interrupt_64.S index 3e8a811e09c4..34167cfa5d60 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) @@ -108,6 +108,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. + */ + NULLIFY_GPRS(5, 12) + NULLIFY_NVGPRS() + /* Calling convention has r3 = orig r0, r4 = regs */ mr r3,r0 bl system_call_exception @@ -138,6 +145,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 @@ -180,7 +188,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 @@ -248,7 +255,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) @@ -298,6 +305,13 @@ END_BTB_FLUSH_SECTION wrteei 1 #endif + /* + * Zero user registers to prevent influencing speculative execution + * state of kernel code. + */ + NULLIFY_GPRS(5, 12) + NULLIFY_NVGPRS() + /* Calling convention has r3 = orig r0, r4 = regs */ mr r3,r0 bl system_call_exception @@ -340,6 +354,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 */ @@ -367,7 +382,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)
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. One function of syscall_exit_prepare is to determine when non-volatile regs must be restored, and it still serves that purpose on 32-bit. Use it now for determining where to find XER, CTR, CR. Signed-off-by: Rohan McLure <rmclure@linux.ibm.com> --- V1 -> V2: Update summary --- arch/powerpc/kernel/interrupt_64.S | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-)