Re: [PATCH v3] x86/entry_32: Move CLEAR_CPU_BUFFERS before restoring segments

[Pawan Gupta]

On Mon, Jul 01, 2024 at 03:22:44PM -0400, Brian Gerst wrote:
> Perhaps I should have been a bit clearer, but I meant adding the SS
> override to the VERW instructions in their present locations, instead
> of moving it into RESTORE_REGS.

I did consider that way, but moving it to RESTORE_REGS saves a callsite for
CLEAR_CPU_BUFFERS. I believe Dave prefered it within RESTORE_REGS, but that
was before the SS override. Unless anyone has a strong preference, I will
change the current callsites to include the SS override, and not move
CLEAR_CPU_BUFFERS to RESTORE_REGS.

> IIRC we don't want data reads (including stack pops) between the VERW
> instruction and IRET/SYSEXIT.

Stack pops are okay if the memory *and* the registers doesn't contain
sensitive data, which doesn't seem to be the case here. But, SS override is
a safer option.

BTW, Dave pointed out that CLEAR_CPU_BUFFERS needs to be done after
RESTORE_ALL_NMI in the NMI return path, that adds an extra callsite:

---
diff --git a/arch/x86/entry/entry_32.S b/arch/x86/entry/entry_32.S
index d3a814efbff6..9b58d2cbdfef 100644
--- a/arch/x86/entry/entry_32.S
+++ b/arch/x86/entry/entry_32.S
@@ -1144,7 +1144,6 @@ SYM_CODE_START(asm_exc_nmi)
 
 	/* Not on SYSENTER stack. */
 	call	exc_nmi
-	CLEAR_CPU_BUFFERS
 	jmp	.Lnmi_return
 
 .Lnmi_from_sysenter_stack:
@@ -1165,6 +1164,7 @@ SYM_CODE_START(asm_exc_nmi)
 
 	CHECK_AND_APPLY_ESPFIX
 	RESTORE_ALL_NMI cr3_reg=%edi pop=4
+	CLEAR_CPU_BUFFERS
 	jmp	.Lirq_return
 
 #ifdef CONFIG_X86_ESPFIX32
@@ -1206,6 +1206,7 @@ SYM_CODE_START(asm_exc_nmi)
 	 *  1 - orig_ax
 	 */
 	lss	(1+5+6)*4(%esp), %esp			# back to espfix stack
+	CLEAR_CPU_BUFFERS
 	jmp	.Lirq_return
 #endif
 SYM_CODE_END(asm_exc_nmi)