All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3] x86,entry: Use PUSH_AND_CLEAR_REGS for compat
@ 2022-04-29  9:13 Peter Zijlstra
  2022-04-29 12:00 ` Lai Jiangshan
  0 siblings, 1 reply; 6+ messages in thread
From: Peter Zijlstra @ 2022-04-29  9:13 UTC (permalink / raw)
  To: x86, jpoimboe, brgerst, jiangshanlai, Andrew.Cooper3; +Cc: linux-kernel, peterz


Since the upper regs don't exist for ia32 code, preserving them
doesn't hurt and it simplifies the code.

If there was any attack surface on this, that attack surface already
exists for INT80 and needs to be otherwise dealt with.

Notably:

 - SYSENTER: didn't clear si, dx, cx.
 - SYSCALL, INT80: *do* clear si since the C functions don't take a
   second argument.

Add a clear_rsi argument to CLEAR_REGS for these 3 sites, and have
SYSENTER clear everything (no code relies on those registers not being
cleared and selftests pass).

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---

diff --git a/arch/x86/entry/calling.h b/arch/x86/entry/calling.h
index a4c061fb7c6e..526eba74b84c 100644
--- a/arch/x86/entry/calling.h
+++ b/arch/x86/entry/calling.h
@@ -63,13 +63,15 @@ For 32-bit we have the following conventions - kernel is built with
  * for assembly code:
  */
 
-.macro PUSH_REGS rdx=%rdx rax=%rax save_ret=0
+.macro PUSH_REGS rdx=%rdx rax=%rax save_ret=0 save_rdi=1
 	.if \save_ret
 	pushq	%rsi		/* pt_regs->si */
 	movq	8(%rsp), %rsi	/* temporarily store the return address in %rsi */
 	movq	%rdi, 8(%rsp)	/* pt_regs->di (overwriting original return address) */
 	.else
+	.if \save_rdi
 	pushq   %rdi		/* pt_regs->di */
+	.endif
 	pushq   %rsi		/* pt_regs->si */
 	.endif
 	pushq	\rdx		/* pt_regs->dx */
@@ -92,13 +94,16 @@ For 32-bit we have the following conventions - kernel is built with
 	.endif
 .endm
 
-.macro CLEAR_REGS
+.macro CLEAR_REGS clear_rsi=0
 	/*
 	 * Sanitize registers of values that a speculation attack might
 	 * otherwise want to exploit. The lower registers are likely clobbered
 	 * well before they could be put to use in a speculative execution
 	 * gadget.
 	 */
+	.if \clear_rsi
+	xorl	%esi,  %esi	/* nospec si  */
+	.endif
 	xorl	%edx,  %edx	/* nospec dx  */
 	xorl	%ecx,  %ecx	/* nospec cx  */
 	xorl	%r8d,  %r8d	/* nospec r8  */
@@ -111,12 +116,11 @@ For 32-bit we have the following conventions - kernel is built with
 	xorl	%r13d, %r13d	/* nospec r13 */
 	xorl	%r14d, %r14d	/* nospec r14 */
 	xorl	%r15d, %r15d	/* nospec r15 */
-
 .endm
 
-.macro PUSH_AND_CLEAR_REGS rdx=%rdx rax=%rax save_ret=0
-	PUSH_REGS rdx=\rdx, rax=\rax, save_ret=\save_ret
-	CLEAR_REGS
+.macro PUSH_AND_CLEAR_REGS rdx=%rdx rax=%rax save_ret=0 save_rdi=1 clear_rsi=0
+	PUSH_REGS rdx=\rdx, rax=\rax, save_ret=\save_ret save_rdi=\save_rdi
+	CLEAR_REGS clear_rsi=\clear_rsi
 .endm
 
 .macro POP_REGS pop_rdi=1 skip_r11rcx=0
diff --git a/arch/x86/entry/entry_64_compat.S b/arch/x86/entry/entry_64_compat.S
index 4fdb007cddbd..4ca8d6bfbe6b 100644
--- a/arch/x86/entry/entry_64_compat.S
+++ b/arch/x86/entry/entry_64_compat.S
@@ -83,32 +83,7 @@ SYM_INNER_LABEL(entry_SYSENTER_compat_after_hwframe, SYM_L_GLOBAL)
 	movl	%eax, %eax
 
 	pushq	%rax			/* pt_regs->orig_ax */
-	pushq	%rdi			/* pt_regs->di */
-	pushq	%rsi			/* pt_regs->si */
-	pushq	%rdx			/* pt_regs->dx */
-	pushq	%rcx			/* pt_regs->cx */
-	pushq	$-ENOSYS		/* pt_regs->ax */
-	pushq   $0			/* pt_regs->r8  = 0 */
-	xorl	%r8d, %r8d		/* nospec   r8 */
-	pushq   $0			/* pt_regs->r9  = 0 */
-	xorl	%r9d, %r9d		/* nospec   r9 */
-	pushq   $0			/* pt_regs->r10 = 0 */
-	xorl	%r10d, %r10d		/* nospec   r10 */
-	pushq   $0			/* pt_regs->r11 = 0 */
-	xorl	%r11d, %r11d		/* nospec   r11 */
-	pushq   %rbx                    /* pt_regs->rbx */
-	xorl	%ebx, %ebx		/* nospec   rbx */
-	pushq   %rbp                    /* pt_regs->rbp (will be overwritten) */
-	xorl	%ebp, %ebp		/* nospec   rbp */
-	pushq   $0			/* pt_regs->r12 = 0 */
-	xorl	%r12d, %r12d		/* nospec   r12 */
-	pushq   $0			/* pt_regs->r13 = 0 */
-	xorl	%r13d, %r13d		/* nospec   r13 */
-	pushq   $0			/* pt_regs->r14 = 0 */
-	xorl	%r14d, %r14d		/* nospec   r14 */
-	pushq   $0			/* pt_regs->r15 = 0 */
-	xorl	%r15d, %r15d		/* nospec   r15 */
-
+	PUSH_AND_CLEAR_REGS rax=$-ENOSYS clear_rsi=1
 	UNWIND_HINT_REGS
 
 	cld
@@ -225,35 +200,7 @@ SYM_INNER_LABEL(entry_SYSCALL_compat_safe_stack, SYM_L_GLOBAL)
 SYM_INNER_LABEL(entry_SYSCALL_compat_after_hwframe, SYM_L_GLOBAL)
 	movl	%eax, %eax		/* discard orig_ax high bits */
 	pushq	%rax			/* pt_regs->orig_ax */
-	pushq	%rdi			/* pt_regs->di */
-	pushq	%rsi			/* pt_regs->si */
-	xorl	%esi, %esi		/* nospec   si */
-	pushq	%rdx			/* pt_regs->dx */
-	xorl	%edx, %edx		/* nospec   dx */
-	pushq	%rbp			/* pt_regs->cx (stashed in bp) */
-	xorl	%ecx, %ecx		/* nospec   cx */
-	pushq	$-ENOSYS		/* pt_regs->ax */
-	pushq   $0			/* pt_regs->r8  = 0 */
-	xorl	%r8d, %r8d		/* nospec   r8 */
-	pushq   $0			/* pt_regs->r9  = 0 */
-	xorl	%r9d, %r9d		/* nospec   r9 */
-	pushq   $0			/* pt_regs->r10 = 0 */
-	xorl	%r10d, %r10d		/* nospec   r10 */
-	pushq   $0			/* pt_regs->r11 = 0 */
-	xorl	%r11d, %r11d		/* nospec   r11 */
-	pushq   %rbx                    /* pt_regs->rbx */
-	xorl	%ebx, %ebx		/* nospec   rbx */
-	pushq   %rbp                    /* pt_regs->rbp (will be overwritten) */
-	xorl	%ebp, %ebp		/* nospec   rbp */
-	pushq   $0			/* pt_regs->r12 = 0 */
-	xorl	%r12d, %r12d		/* nospec   r12 */
-	pushq   $0			/* pt_regs->r13 = 0 */
-	xorl	%r13d, %r13d		/* nospec   r13 */
-	pushq   $0			/* pt_regs->r14 = 0 */
-	xorl	%r14d, %r14d		/* nospec   r14 */
-	pushq   $0			/* pt_regs->r15 = 0 */
-	xorl	%r15d, %r15d		/* nospec   r15 */
-
+	PUSH_AND_CLEAR_REGS rax=$-ENOSYS clear_rsi=1
 	UNWIND_HINT_REGS
 
 	movq	%rsp, %rdi
@@ -381,35 +328,7 @@ SYM_CODE_START(entry_INT80_compat)
 	pushq	1*8(%rdi)		/* regs->orig_ax */
 	pushq	(%rdi)			/* pt_regs->di */
 .Lint80_keep_stack:
-
-	pushq	%rsi			/* pt_regs->si */
-	xorl	%esi, %esi		/* nospec   si */
-	pushq	%rdx			/* pt_regs->dx */
-	xorl	%edx, %edx		/* nospec   dx */
-	pushq	%rcx			/* pt_regs->cx */
-	xorl	%ecx, %ecx		/* nospec   cx */
-	pushq	$-ENOSYS		/* pt_regs->ax */
-	pushq   %r8			/* pt_regs->r8 */
-	xorl	%r8d, %r8d		/* nospec   r8 */
-	pushq   %r9			/* pt_regs->r9 */
-	xorl	%r9d, %r9d		/* nospec   r9 */
-	pushq   %r10			/* pt_regs->r10*/
-	xorl	%r10d, %r10d		/* nospec   r10 */
-	pushq   %r11			/* pt_regs->r11 */
-	xorl	%r11d, %r11d		/* nospec   r11 */
-	pushq   %rbx                    /* pt_regs->rbx */
-	xorl	%ebx, %ebx		/* nospec   rbx */
-	pushq   %rbp                    /* pt_regs->rbp */
-	xorl	%ebp, %ebp		/* nospec   rbp */
-	pushq   %r12                    /* pt_regs->r12 */
-	xorl	%r12d, %r12d		/* nospec   r12 */
-	pushq   %r13                    /* pt_regs->r13 */
-	xorl	%r13d, %r13d		/* nospec   r13 */
-	pushq   %r14                    /* pt_regs->r14 */
-	xorl	%r14d, %r14d		/* nospec   r14 */
-	pushq   %r15                    /* pt_regs->r15 */
-	xorl	%r15d, %r15d		/* nospec   r15 */
-
+	PUSH_AND_CLEAR_REGS rax=$-ENOSYS save_rdi=0 clear_rsi=1
 	UNWIND_HINT_REGS
 
 	cld


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v3] x86,entry: Use PUSH_AND_CLEAR_REGS for compat
  2022-04-29  9:13 [PATCH v3] x86,entry: Use PUSH_AND_CLEAR_REGS for compat Peter Zijlstra
@ 2022-04-29 12:00 ` Lai Jiangshan
  2022-04-29 21:12   ` Peter Zijlstra
  0 siblings, 1 reply; 6+ messages in thread
From: Lai Jiangshan @ 2022-04-29 12:00 UTC (permalink / raw)
  To: Peter Zijlstra; +Cc: X86 ML, Josh Poimboeuf, Brian Gerst, Andrew Cooper, LKML

On Fri, Apr 29, 2022 at 5:13 PM Peter Zijlstra <peterz@infradead.org> wrote:
>
>
> Since the upper regs don't exist for ia32 code, preserving them
> doesn't hurt and it simplifies the code.
>
> If there was any attack surface on this, that attack surface already
> exists for INT80 and needs to be otherwise dealt with.
>
> Notably:
>
>  - SYSENTER: didn't clear si, dx, cx.
>  - SYSCALL, INT80: *do* clear si since the C functions don't take a
>    second argument.
>

If CLEAR_REGS for SYSCALL, INT80 clears si, it is better, IMO, to
make CLEAR_REGS clear si unconditionally.

SYSCALL, INT80 will explicitly clear si via calling
syscall_enter_from_user_mode().

But some handlers called from the macro idtentry don't clear
si explicitly, although it is likely to be cleared.

So if %rsi is a concern for SYSCALL, INT80, please make CLEAR_REGS
clear %rsi unconditionally.

Thanks
Lai

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3] x86,entry: Use PUSH_AND_CLEAR_REGS for compat
  2022-04-29 12:00 ` Lai Jiangshan
@ 2022-04-29 21:12   ` Peter Zijlstra
  2022-04-29 21:17     ` [PATCH v4] " Peter Zijlstra
  2022-04-29 21:30     ` [PATCH v3] " Linus Torvalds
  0 siblings, 2 replies; 6+ messages in thread
From: Peter Zijlstra @ 2022-04-29 21:12 UTC (permalink / raw)
  To: Lai Jiangshan
  Cc: X86 ML, Josh Poimboeuf, Brian Gerst, Andrew Cooper, LKML, Linus Torvalds

On Fri, Apr 29, 2022 at 08:00:37PM +0800, Lai Jiangshan wrote:
> On Fri, Apr 29, 2022 at 5:13 PM Peter Zijlstra <peterz@infradead.org> wrote:

> > Notably:
> >
> >  - SYSENTER: didn't clear si, dx, cx.
> >  - SYSCALL, INT80: *do* clear si since the C functions don't take a
> >    second argument.
> >
> 
> If CLEAR_REGS for SYSCALL, INT80 clears si, it is better, IMO, to
> make CLEAR_REGS clear si unconditionally.

Well, I didn't want to add the overhead to 64bit native syscalls, but
Linus just suggested the same thing elsewhere. So yeah.

He also suggested cleaning up INT80 like below to get rid of the
save_rdi wart.

(Linus, can I add your SoB to the thing?)

---
Subject: x86/entry: Simplify entry_INT80_compat()
From: Linus Torvalds <torvalds@linuxfoundation.org>
Date: Fri Apr 29 22:52:21 CEST 2022

Instead of playing silly games with rdi, use rax for simpler and more
consistent code.

[peterz: Changelog and fix off-by-one in offsets]
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 arch/x86/entry/entry_64_compat.S |   19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

--- a/arch/x86/entry/entry_64_compat.S
+++ b/arch/x86/entry/entry_64_compat.S
@@ -362,26 +362,25 @@ SYM_CODE_START(entry_INT80_compat)
 
 	/* switch to thread stack expects orig_ax and rdi to be pushed */
 	pushq	%rax			/* pt_regs->orig_ax */
-	pushq	%rdi			/* pt_regs->di */
 
 	/* Need to switch before accessing the thread stack. */
-	SWITCH_TO_KERNEL_CR3 scratch_reg=%rdi
+	SWITCH_TO_KERNEL_CR3 scratch_reg=%rax
 
 	/* In the Xen PV case we already run on the thread stack. */
 	ALTERNATIVE "", "jmp .Lint80_keep_stack", X86_FEATURE_XENPV
 
-	movq	%rsp, %rdi
+	movq	%rsp, %rax
 	movq	PER_CPU_VAR(cpu_current_top_of_stack), %rsp
 
-	pushq	6*8(%rdi)		/* regs->ss */
-	pushq	5*8(%rdi)		/* regs->rsp */
-	pushq	4*8(%rdi)		/* regs->eflags */
-	pushq	3*8(%rdi)		/* regs->cs */
-	pushq	2*8(%rdi)		/* regs->ip */
-	pushq	1*8(%rdi)		/* regs->orig_ax */
-	pushq	(%rdi)			/* pt_regs->di */
+	pushq	5*8(%rax)		/* regs->ss */
+	pushq	4*8(%rax)		/* regs->rsp */
+	pushq	3*8(%rax)		/* regs->eflags */
+	pushq	2*8(%rax)		/* regs->cs */
+	pushq	1*8(%rax)		/* regs->ip */
+	pushq	0*8(%rax)		/* regs->orig_ax */
 .Lint80_keep_stack:
 
+	pushq	%rdi			/* pt_regs->di */
 	pushq	%rsi			/* pt_regs->si */
 	xorl	%esi, %esi		/* nospec   si */
 	pushq	%rdx			/* pt_regs->dx */

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v4] x86,entry: Use PUSH_AND_CLEAR_REGS for compat
  2022-04-29 21:12   ` Peter Zijlstra
@ 2022-04-29 21:17     ` Peter Zijlstra
  2022-04-29 21:30     ` [PATCH v3] " Linus Torvalds
  1 sibling, 0 replies; 6+ messages in thread
From: Peter Zijlstra @ 2022-04-29 21:17 UTC (permalink / raw)
  To: Lai Jiangshan
  Cc: X86 ML, Josh Poimboeuf, Brian Gerst, Andrew Cooper, LKML, Linus Torvalds

On Fri, Apr 29, 2022 at 11:12:56PM +0200, Peter Zijlstra wrote:
> On Fri, Apr 29, 2022 at 08:00:37PM +0800, Lai Jiangshan wrote:
> > On Fri, Apr 29, 2022 at 5:13 PM Peter Zijlstra <peterz@infradead.org> wrote:
> 
> > > Notably:
> > >
> > >  - SYSENTER: didn't clear si, dx, cx.
> > >  - SYSCALL, INT80: *do* clear si since the C functions don't take a
> > >    second argument.
> > >
> > 
> > If CLEAR_REGS for SYSCALL, INT80 clears si, it is better, IMO, to
> > make CLEAR_REGS clear si unconditionally.
> 
> Well, I didn't want to add the overhead to 64bit native syscalls, but
> Linus just suggested the same thing elsewhere. So yeah.
> 
> He also suggested cleaning up INT80 like below to get rid of the
> save_rdi wart.

Which then results in...

---
Subject: x86,entry: Use PUSH_AND_CLEAR_REGS for compat
From: Peter Zijlstra <peterz@infradead.org>
Date: Sat, 9 Apr 2022 00:38:27 +0200

Since the upper regs don't exist for ia32 code, preserving them
doesn't hurt and it simplifies the code.

If there was any attack surface on this, that attack surface already
exists for INT80 and needs to be otherwise dealt with.

Notably:

 - 32bit SYSENTER: didn't clear si, dx, cx.
 - 32bit SYSCALL, INT80: *do* clear si since the C functions don't
   take a second argument.
 - 64bit: don't clear si since the C functions take a second argument.

SYSENTER should be clearing all those 3 registers, nothing uses them
and selftests pass. Unconditionally clear rsi since it simplifies
code.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 arch/x86/entry/calling.h         |    1 
 arch/x86/entry/entry_64_compat.S |   87 +--------------------------------------
 2 files changed, 4 insertions(+), 84 deletions(-)

--- a/arch/x86/entry/calling.h
+++ b/arch/x86/entry/calling.h
@@ -99,6 +99,7 @@ For 32-bit we have the following convent
 	 * well before they could be put to use in a speculative execution
 	 * gadget.
 	 */
+	xorl	%esi,  %esi	/* nospec si  */
 	xorl	%edx,  %edx	/* nospec dx  */
 	xorl	%ecx,  %ecx	/* nospec cx  */
 	xorl	%r8d,  %r8d	/* nospec r8  */
--- a/arch/x86/entry/entry_64_compat.S
+++ b/arch/x86/entry/entry_64_compat.S
@@ -83,32 +83,7 @@ SYM_INNER_LABEL(entry_SYSENTER_compat_af
 	movl	%eax, %eax
 
 	pushq	%rax			/* pt_regs->orig_ax */
-	pushq	%rdi			/* pt_regs->di */
-	pushq	%rsi			/* pt_regs->si */
-	pushq	%rdx			/* pt_regs->dx */
-	pushq	%rcx			/* pt_regs->cx */
-	pushq	$-ENOSYS		/* pt_regs->ax */
-	pushq   $0			/* pt_regs->r8  = 0 */
-	xorl	%r8d, %r8d		/* nospec   r8 */
-	pushq   $0			/* pt_regs->r9  = 0 */
-	xorl	%r9d, %r9d		/* nospec   r9 */
-	pushq   $0			/* pt_regs->r10 = 0 */
-	xorl	%r10d, %r10d		/* nospec   r10 */
-	pushq   $0			/* pt_regs->r11 = 0 */
-	xorl	%r11d, %r11d		/* nospec   r11 */
-	pushq   %rbx                    /* pt_regs->rbx */
-	xorl	%ebx, %ebx		/* nospec   rbx */
-	pushq   %rbp                    /* pt_regs->rbp (will be overwritten) */
-	xorl	%ebp, %ebp		/* nospec   rbp */
-	pushq   $0			/* pt_regs->r12 = 0 */
-	xorl	%r12d, %r12d		/* nospec   r12 */
-	pushq   $0			/* pt_regs->r13 = 0 */
-	xorl	%r13d, %r13d		/* nospec   r13 */
-	pushq   $0			/* pt_regs->r14 = 0 */
-	xorl	%r14d, %r14d		/* nospec   r14 */
-	pushq   $0			/* pt_regs->r15 = 0 */
-	xorl	%r15d, %r15d		/* nospec   r15 */
-
+	PUSH_AND_CLEAR_REGS rax=$-ENOSYS
 	UNWIND_HINT_REGS
 
 	cld
@@ -225,35 +200,7 @@ SYM_INNER_LABEL(entry_SYSCALL_compat_saf
 SYM_INNER_LABEL(entry_SYSCALL_compat_after_hwframe, SYM_L_GLOBAL)
 	movl	%eax, %eax		/* discard orig_ax high bits */
 	pushq	%rax			/* pt_regs->orig_ax */
-	pushq	%rdi			/* pt_regs->di */
-	pushq	%rsi			/* pt_regs->si */
-	xorl	%esi, %esi		/* nospec   si */
-	pushq	%rdx			/* pt_regs->dx */
-	xorl	%edx, %edx		/* nospec   dx */
-	pushq	%rbp			/* pt_regs->cx (stashed in bp) */
-	xorl	%ecx, %ecx		/* nospec   cx */
-	pushq	$-ENOSYS		/* pt_regs->ax */
-	pushq   $0			/* pt_regs->r8  = 0 */
-	xorl	%r8d, %r8d		/* nospec   r8 */
-	pushq   $0			/* pt_regs->r9  = 0 */
-	xorl	%r9d, %r9d		/* nospec   r9 */
-	pushq   $0			/* pt_regs->r10 = 0 */
-	xorl	%r10d, %r10d		/* nospec   r10 */
-	pushq   $0			/* pt_regs->r11 = 0 */
-	xorl	%r11d, %r11d		/* nospec   r11 */
-	pushq   %rbx                    /* pt_regs->rbx */
-	xorl	%ebx, %ebx		/* nospec   rbx */
-	pushq   %rbp                    /* pt_regs->rbp (will be overwritten) */
-	xorl	%ebp, %ebp		/* nospec   rbp */
-	pushq   $0			/* pt_regs->r12 = 0 */
-	xorl	%r12d, %r12d		/* nospec   r12 */
-	pushq   $0			/* pt_regs->r13 = 0 */
-	xorl	%r13d, %r13d		/* nospec   r13 */
-	pushq   $0			/* pt_regs->r14 = 0 */
-	xorl	%r14d, %r14d		/* nospec   r14 */
-	pushq   $0			/* pt_regs->r15 = 0 */
-	xorl	%r15d, %r15d		/* nospec   r15 */
-
+	PUSH_AND_CLEAR_REGS rax=$-ENOSYS
 	UNWIND_HINT_REGS
 
 	movq	%rsp, %rdi
@@ -380,35 +327,7 @@ SYM_CODE_START(entry_INT80_compat)
 	pushq	0*8(%rax)		/* regs->orig_ax */
 .Lint80_keep_stack:
 
-	pushq	%rdi			/* pt_regs->di */
-	pushq	%rsi			/* pt_regs->si */
-	xorl	%esi, %esi		/* nospec   si */
-	pushq	%rdx			/* pt_regs->dx */
-	xorl	%edx, %edx		/* nospec   dx */
-	pushq	%rcx			/* pt_regs->cx */
-	xorl	%ecx, %ecx		/* nospec   cx */
-	pushq	$-ENOSYS		/* pt_regs->ax */
-	pushq   %r8			/* pt_regs->r8 */
-	xorl	%r8d, %r8d		/* nospec   r8 */
-	pushq   %r9			/* pt_regs->r9 */
-	xorl	%r9d, %r9d		/* nospec   r9 */
-	pushq   %r10			/* pt_regs->r10*/
-	xorl	%r10d, %r10d		/* nospec   r10 */
-	pushq   %r11			/* pt_regs->r11 */
-	xorl	%r11d, %r11d		/* nospec   r11 */
-	pushq   %rbx                    /* pt_regs->rbx */
-	xorl	%ebx, %ebx		/* nospec   rbx */
-	pushq   %rbp                    /* pt_regs->rbp */
-	xorl	%ebp, %ebp		/* nospec   rbp */
-	pushq   %r12                    /* pt_regs->r12 */
-	xorl	%r12d, %r12d		/* nospec   r12 */
-	pushq   %r13                    /* pt_regs->r13 */
-	xorl	%r13d, %r13d		/* nospec   r13 */
-	pushq   %r14                    /* pt_regs->r14 */
-	xorl	%r14d, %r14d		/* nospec   r14 */
-	pushq   %r15                    /* pt_regs->r15 */
-	xorl	%r15d, %r15d		/* nospec   r15 */
-
+	PUSH_AND_CLEAR_REGS rax=$-ENOSYS
 	UNWIND_HINT_REGS
 
 	cld

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3] x86,entry: Use PUSH_AND_CLEAR_REGS for compat
  2022-04-29 21:12   ` Peter Zijlstra
  2022-04-29 21:17     ` [PATCH v4] " Peter Zijlstra
@ 2022-04-29 21:30     ` Linus Torvalds
  2022-04-29 22:14       ` Peter Zijlstra
  1 sibling, 1 reply; 6+ messages in thread
From: Linus Torvalds @ 2022-04-29 21:30 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Lai Jiangshan, X86 ML, Josh Poimboeuf, Brian Gerst, Andrew Cooper, LKML

On Fri, Apr 29, 2022 at 2:13 PM Peter Zijlstra <peterz@infradead.org> wrote:
>
> (Linus, can I add your SoB to the thing?)

If you teste this with some actual old int80 compat syscalls, then absolutely:

   Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

> +       pushq   %rdi                    /* pt_regs->di */
>         pushq   %rsi                    /* pt_regs->si */
>         xorl    %esi, %esi              /* nospec   si */

It would probably make sense to add a comment about why %rdi isn't
cleared when pushed, like all the other registers are.

Even if that comment is just "%rdi will be overwritten as arg0 of the
call to C, so no need to clear it".

Maybe as part of the PUSH_AND_CLEAR_REGS changes?

            Linus

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3] x86,entry: Use PUSH_AND_CLEAR_REGS for compat
  2022-04-29 21:30     ` [PATCH v3] " Linus Torvalds
@ 2022-04-29 22:14       ` Peter Zijlstra
  0 siblings, 0 replies; 6+ messages in thread
From: Peter Zijlstra @ 2022-04-29 22:14 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Lai Jiangshan, X86 ML, Josh Poimboeuf, Brian Gerst, Andrew Cooper, LKML

On Fri, Apr 29, 2022 at 02:30:45PM -0700, Linus Torvalds wrote:
> On Fri, Apr 29, 2022 at 2:13 PM Peter Zijlstra <peterz@infradead.org> wrote:
> >
> > (Linus, can I add your SoB to the thing?)
> 
> If you teste this with some actual old int80 compat syscalls, then absolutely:

I ran tools/testing/selftests/x86/*_32 on it. That definitely tickles
the int80 path. Also, without the off-by-one fixed that gives some
generous helpings of segfault.

>    Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

Thanks!

> > +       pushq   %rdi                    /* pt_regs->di */
> >         pushq   %rsi                    /* pt_regs->si */
> >         xorl    %esi, %esi              /* nospec   si */
> 
> It would probably make sense to add a comment about why %rdi isn't
> cleared when pushed, like all the other registers are.
> 
> Even if that comment is just "%rdi will be overwritten as arg0 of the
> call to C, so no need to clear it".
> 
> Maybe as part of the PUSH_AND_CLEAR_REGS changes?

I'll stick the comment on.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2022-04-29 22:14 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-29  9:13 [PATCH v3] x86,entry: Use PUSH_AND_CLEAR_REGS for compat Peter Zijlstra
2022-04-29 12:00 ` Lai Jiangshan
2022-04-29 21:12   ` Peter Zijlstra
2022-04-29 21:17     ` [PATCH v4] " Peter Zijlstra
2022-04-29 21:30     ` [PATCH v3] " Linus Torvalds
2022-04-29 22:14       ` Peter Zijlstra

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.