linux-kernel - Re: [PATCH v3] x86,entry: Use PUSH_AND_CLEAR

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 29 Apr 2022 23:12:56 +0200
From:   Peter Zijlstra <peterz@...radead.org>
To:     Lai Jiangshan <jiangshanlai@...il.com>
Cc:     X86 ML <x86@...nel.org>, Josh Poimboeuf <jpoimboe@...hat.com>,
        Brian Gerst <brgerst@...il.com>,
        Andrew Cooper <Andrew.Cooper3@...rix.com>,
        LKML <linux-kernel@...r.kernel.org>,
        Linus Torvalds <torvalds@...ux-foundation.org>
Subject: Re: [PATCH v3] x86,entry: Use PUSH_AND_CLEAR_REGS for compat

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@...radead.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@...uxfoundation.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@...radead.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 */