On 20 Sep 2022, at 12:03 pm, Nicholas Piggin <npiggin@gmail.com> = wrote:

On Fri Sep 16, 2022 at 3:32 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. Clear r0 also.
V4 = -> V5: Move to end of patch series.
---

I think it looks okay. I'll have to take a better look with = the series
applied.

Your comments alerted me to the fact that general = interrupt and syscalls

share their exit code in interrupt_return and its = derivatives. Meaning

that disabling INTERRUPT_SANITIZE_REGISTERS also reverts = restores of NVGPRS

to = being optional, which makes it possible to clobber NVGPRS and then = not

restore them. The = cleanest way forward I belive is going to be to cause

INTERRUPT_SANITIZE_REGISTERS to = perform sanitisation on all interrupt sources

rather than continuing with = syscalls as their own special case. I=E2=80=99ll put

this out in a v6 = soon.

arch/powerpc/kernel/interrupt_64.S | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)

diff --git = a/arch/powerpc/kernel/interrupt_64.S = b/arch/powerpc/kernel/interrupt_64.S
index = 16a1b44088e7..40147558e1a6 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 */

These two comment changes could go in your = system_call_exception API
change patch though.

Thanks,
Nick

SAVE_GPRS(3, 8, r1)
= /* Zero r9-r12, this should only be required when restoring all = GPRs */
std r11,GPR9(r1)
@@ -110,6 +110,7 @@ = END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
* 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
@@ -140,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

@@ -243,7 +245,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 */
= SAVE_GPRS(3, 8, r1)
/* Zero r9-r12, this should only = be required when restoring all GPRs */
std = r11,GPR9(r1)
@@ -295,6 +297,7 @@ = END_BTB_FLUSH_SECTION
* 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
@@ -337,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 = */
@@ -364,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