Thread (27 messages) 27 messages, 2 authors, 2022-08-24

Re: [PATCH v3 14/18] powerpc/64s: Clear/restore caller gprs in syscall interrupt/return

From: Christophe Leroy <hidden>
Date: 2022-08-19 06:53:12


Le 19/08/2022 à 05:38, Rohan McLure a écrit :
quoted hunk ↗ jump to hunk
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 <redacted>
---
V1 -> V2: Update summary
V2 -> V3: Remove erroneous summary paragraph on syscall_exit_prepare
---
  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..d9625113c7a5 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.
+	 */
+	NULLIFY_GPRS(5, 12)
Macro name has changed.
+	NULLIFY_NVGPRS()
Why clearing non volatile GPRs ? They are supposed to be callee saved so 
I can't see how they could be used for speculation. Do you have any 
exemple ?
quoted hunk ↗ jump to hunk
+
  	/* 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)
What is the link between this change and the description in the commit 
message ?
quoted hunk ↗ jump to hunk
  	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.
+	 */
+	NULLIFY_GPRS(5, 12)
+	NULLIFY_NVGPRS()
+
Name has changed.
quoted hunk ↗ jump to hunk
  	/* 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)
Same question.
quoted hunk ↗ jump to hunk
  	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)
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help