linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] x86/asm/entry/32: Open-code CLEAR_RREGS. No code changes.
@ 2015-06-02 19:04 Denys Vlasenko
  2015-06-02 19:04 ` [PATCH 2/2] x86/asm/entry/32: Open-code LOAD_ARGS32. " Denys Vlasenko
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Denys Vlasenko @ 2015-06-02 19:04 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Denys Vlasenko, Linus Torvalds, Steven Rostedt, Borislav Petkov,
	H. Peter Anvin, Andy Lutomirski, Oleg Nesterov,
	Frederic Weisbecker, Alexei Starovoitov, Will Drewry, Kees Cook,
	x86, linux-kernel

This macro is small, has only four callsites, and one of them is
slightly different using a conditional parameter.

A few saved lines aren't worth the resulting obfuscation.

Generated machine code is identical.

Signed-off-by: Denys Vlasenko <dvlasenk@redhat.com>
CC: Linus Torvalds <torvalds@linux-foundation.org>
CC: Steven Rostedt <rostedt@goodmis.org>
CC: Ingo Molnar <mingo@kernel.org>
CC: Borislav Petkov <bp@alien8.de>
CC: "H. Peter Anvin" <hpa@zytor.com>
CC: Andy Lutomirski <luto@amacapital.net>
CC: Oleg Nesterov <oleg@redhat.com>
CC: Frederic Weisbecker <fweisbec@gmail.com>
CC: Alexei Starovoitov <ast@plumgrid.com>
CC: Will Drewry <wad@chromium.org>
CC: Kees Cook <keescook@chromium.org>
CC: x86@kernel.org
CC: linux-kernel@vger.kernel.org
---

These two patches are on top of "Simplify zeroing of pt_regs->r8..r11" patch.

 arch/x86/ia32/ia32entry.S | 33 ++++++++++++++++++++-------------
 1 file changed, 20 insertions(+), 13 deletions(-)

diff --git a/arch/x86/ia32/ia32entry.S b/arch/x86/ia32/ia32entry.S
index 2801cbe..86cbfe6 100644
--- a/arch/x86/ia32/ia32entry.S
+++ b/arch/x86/ia32/ia32entry.S
@@ -29,15 +29,6 @@
 
 	.section .entry.text, "ax"
 
-	/* clobbers %rax */
-	.macro  CLEAR_RREGS _r9=rax
-	xorl 	%eax,%eax
-	movq	%rax,R11(%rsp)
-	movq	%rax,R10(%rsp)
-	movq	%\_r9,R9(%rsp)
-	movq	%rax,R8(%rsp)
-	.endm
-
 	/*
 	 * Reload arg registers from stack in case ptrace changed them.
 	 * We don't reload %eax because syscall_trace_enter() returned
@@ -243,7 +234,11 @@ sysexit_from_sys_call:
 	TRACE_IRQS_OFF
 	testl %edi, ASM_THREAD_INFO(TI_flags, %rsp, SIZEOF_PTREGS)
 	jz \exit
-	CLEAR_RREGS
+	xorl	%eax, %eax
+	movq	%rax, R11(%rsp)
+	movq	%rax, R10(%rsp)
+	movq	%rax, R9(%rsp)
+	movq	%rax, R8(%rsp)
 	jmp int_with_check
 	.endm
 
@@ -267,7 +262,11 @@ sysenter_tracesys:
 	jz	sysenter_auditsys
 #endif
 	SAVE_EXTRA_REGS
-	CLEAR_RREGS
+	xorl	%eax, %eax
+	movq	%rax, R11(%rsp)
+	movq	%rax, R10(%rsp)
+	movq	%rax, R9(%rsp)
+	movq	%rax, R8(%rsp)
 	movq	%rsp,%rdi        /* &pt_regs -> arg1 */
 	call	syscall_trace_enter
 	LOAD_ARGS32  /* reload args from stack in case ptrace changed it */
@@ -407,7 +406,11 @@ cstar_tracesys:
 #endif
 	xchgl %r9d,%ebp
 	SAVE_EXTRA_REGS
-	CLEAR_RREGS r9
+	xorl	%eax, %eax
+	movq	%rax, R11(%rsp)
+	movq	%rax, R10(%rsp)
+	movq	%r9, R9(%rsp)
+	movq	%rax, R8(%rsp)
 	movq %rsp,%rdi        /* &pt_regs -> arg1 */
 	call syscall_trace_enter
 	LOAD_ARGS32 1	/* reload args from stack in case ptrace changed it */
@@ -422,7 +425,11 @@ ia32_badarg:
 	jmp ia32_sysret
 
 ia32_ret_from_sys_call:
-	CLEAR_RREGS
+	xorl	%eax, %eax
+	movq	%rax, R11(%rsp)
+	movq	%rax, R10(%rsp)
+	movq	%rax, R9(%rsp)
+	movq	%rax, R8(%rsp)
 	jmp int_ret_from_sys_call
 
 /*
-- 
1.8.1.4


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

* [PATCH 2/2] x86/asm/entry/32: Open-code LOAD_ARGS32. No code changes.
  2015-06-02 19:04 [PATCH 1/2] x86/asm/entry/32: Open-code CLEAR_RREGS. No code changes Denys Vlasenko
@ 2015-06-02 19:04 ` Denys Vlasenko
  2015-06-02 19:27   ` Borislav Petkov
  2015-06-07  8:31   ` [tip:x86/asm] x86/asm/entry/32: Open-code LOAD_ARGS32 tip-bot for Denys Vlasenko
  2015-06-02 19:34 ` [PATCH 1/2] x86/asm/entry/32: Open-code CLEAR_RREGS. No code changes Steven Rostedt
  2015-06-07  8:31 ` [tip:x86/asm] x86/asm/entry/32: Open-code CLEAR_RREGS tip-bot for Denys Vlasenko
  2 siblings, 2 replies; 8+ messages in thread
From: Denys Vlasenko @ 2015-06-02 19:04 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Denys Vlasenko, Linus Torvalds, Steven Rostedt, Borislav Petkov,
	H. Peter Anvin, Andy Lutomirski, Oleg Nesterov,
	Frederic Weisbecker, Alexei Starovoitov, Will Drewry, Kees Cook,
	x86, linux-kernel

This macro is small, has only three callsites, and one of them is
slightly different using a conditional parameter.

A few saved lines aren't worth the resulting obfuscation.

Generated machine code is identical.

Signed-off-by: Denys Vlasenko <dvlasenk@redhat.com>
CC: Linus Torvalds <torvalds@linux-foundation.org>
CC: Steven Rostedt <rostedt@goodmis.org>
CC: Ingo Molnar <mingo@kernel.org>
CC: Borislav Petkov <bp@alien8.de>
CC: "H. Peter Anvin" <hpa@zytor.com>
CC: Andy Lutomirski <luto@amacapital.net>
CC: Oleg Nesterov <oleg@redhat.com>
CC: Frederic Weisbecker <fweisbec@gmail.com>
CC: Alexei Starovoitov <ast@plumgrid.com>
CC: Will Drewry <wad@chromium.org>
CC: Kees Cook <keescook@chromium.org>
CC: x86@kernel.org
CC: linux-kernel@vger.kernel.org
---
 arch/x86/ia32/ia32entry.S | 62 ++++++++++++++++++++++++++++-------------------
 1 file changed, 37 insertions(+), 25 deletions(-)

diff --git a/arch/x86/ia32/ia32entry.S b/arch/x86/ia32/ia32entry.S
index 86cbfe6..0ff676a 100644
--- a/arch/x86/ia32/ia32entry.S
+++ b/arch/x86/ia32/ia32entry.S
@@ -29,28 +29,6 @@
 
 	.section .entry.text, "ax"
 
-	/*
-	 * Reload arg registers from stack in case ptrace changed them.
-	 * We don't reload %eax because syscall_trace_enter() returned
-	 * the %rax value we should see.  Instead, we just truncate that
-	 * value to 32 bits again as we did on entry from user mode.
-	 * If it's a new value set by user_regset during entry tracing,
-	 * this matches the normal truncation of the user-mode value.
-	 * If it's -1 to make us punt the syscall, then (u32)-1 is still
-	 * an appropriately invalid value.
-	 */
-	.macro LOAD_ARGS32 _r9=0
-	.if \_r9
-	movl R9(%rsp),%r9d
-	.endif
-	movl RCX(%rsp),%ecx
-	movl RDX(%rsp),%edx
-	movl RSI(%rsp),%esi
-	movl RDI(%rsp),%edi
-	movl %eax,%eax			/* zero extension */
-	.endm
-	
-
 #ifdef CONFIG_PARAVIRT
 ENTRY(native_usergs_sysret32)
 	swapgs
@@ -269,7 +247,18 @@ sysenter_tracesys:
 	movq	%rax, R8(%rsp)
 	movq	%rsp,%rdi        /* &pt_regs -> arg1 */
 	call	syscall_trace_enter
-	LOAD_ARGS32  /* reload args from stack in case ptrace changed it */
+	/*
+	 * Reload arg registers from stack in case ptrace changed them.
+	 * Don't reload %eax because syscall_trace_enter() returned
+	 * the %rax value we should see.  But do truncate it to 32 bits.
+	 * If it's -1 to make us punt the syscall, then (u32)-1 is still
+	 * an appropriately invalid value.
+	 */
+	movl	RCX(%rsp), %ecx
+	movl	RDX(%rsp), %edx
+	movl	RSI(%rsp), %esi
+	movl	RDI(%rsp), %edi
+	movl	%eax, %eax	/* zero extension */
 	RESTORE_EXTRA_REGS
 	jmp	sysenter_do_call
 ENDPROC(ia32_sysenter_target)
@@ -413,7 +402,19 @@ cstar_tracesys:
 	movq	%rax, R8(%rsp)
 	movq %rsp,%rdi        /* &pt_regs -> arg1 */
 	call syscall_trace_enter
-	LOAD_ARGS32 1	/* reload args from stack in case ptrace changed it */
+	movl	R9(%rsp),%r9d
+	/*
+	 * Reload arg registers from stack in case ptrace changed them.
+	 * Don't reload %eax because syscall_trace_enter() returned
+	 * the %rax value we should see.  But do truncate it to 32 bits.
+	 * If it's -1 to make us punt the syscall, then (u32)-1 is still
+	 * an appropriately invalid value.
+	 */
+	movl	RCX(%rsp), %ecx
+	movl	RDX(%rsp), %edx
+	movl	RSI(%rsp), %esi
+	movl	RDI(%rsp), %edi
+	movl	%eax, %eax	/* zero extension */
 	RESTORE_EXTRA_REGS
 	xchgl %ebp,%r9d
 	jmp cstar_do_call
@@ -502,7 +503,18 @@ ia32_tracesys:
 	SAVE_EXTRA_REGS
 	movq %rsp,%rdi        /* &pt_regs -> arg1 */
 	call syscall_trace_enter
-	LOAD_ARGS32	/* reload args from stack in case ptrace changed it */
+	/*
+	 * Reload arg registers from stack in case ptrace changed them.
+	 * Don't reload %eax because syscall_trace_enter() returned
+	 * the %rax value we should see.  But do truncate it to 32 bits.
+	 * If it's -1 to make us punt the syscall, then (u32)-1 is still
+	 * an appropriately invalid value.
+	 */
+	movl	RCX(%rsp), %ecx
+	movl	RDX(%rsp), %edx
+	movl	RSI(%rsp), %esi
+	movl	RDI(%rsp), %edi
+	movl	%eax, %eax	/* zero extension */
 	RESTORE_EXTRA_REGS
 	jmp ia32_do_call
 END(ia32_syscall)
-- 
1.8.1.4


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

* Re: [PATCH 2/2] x86/asm/entry/32: Open-code LOAD_ARGS32. No code changes.
  2015-06-02 19:04 ` [PATCH 2/2] x86/asm/entry/32: Open-code LOAD_ARGS32. " Denys Vlasenko
@ 2015-06-02 19:27   ` Borislav Petkov
  2015-06-07  8:31   ` [tip:x86/asm] x86/asm/entry/32: Open-code LOAD_ARGS32 tip-bot for Denys Vlasenko
  1 sibling, 0 replies; 8+ messages in thread
From: Borislav Petkov @ 2015-06-02 19:27 UTC (permalink / raw)
  To: Denys Vlasenko
  Cc: Ingo Molnar, Linus Torvalds, Steven Rostedt, H. Peter Anvin,
	Andy Lutomirski, Oleg Nesterov, Frederic Weisbecker,
	Alexei Starovoitov, Will Drewry, Kees Cook, x86, linux-kernel

On Tue, Jun 02, 2015 at 09:04:02PM +0200, Denys Vlasenko wrote:
> This macro is small, has only three callsites, and one of them is
> slightly different using a conditional parameter.
> 
> A few saved lines aren't worth the resulting obfuscation.
> 
> Generated machine code is identical.
> 
> Signed-off-by: Denys Vlasenko <dvlasenk@redhat.com>
> CC: Linus Torvalds <torvalds@linux-foundation.org>
> CC: Steven Rostedt <rostedt@goodmis.org>
> CC: Ingo Molnar <mingo@kernel.org>
> CC: Borislav Petkov <bp@alien8.de>
> CC: "H. Peter Anvin" <hpa@zytor.com>
> CC: Andy Lutomirski <luto@amacapital.net>
> CC: Oleg Nesterov <oleg@redhat.com>
> CC: Frederic Weisbecker <fweisbec@gmail.com>
> CC: Alexei Starovoitov <ast@plumgrid.com>
> CC: Will Drewry <wad@chromium.org>
> CC: Kees Cook <keescook@chromium.org>
> CC: x86@kernel.org
> CC: linux-kernel@vger.kernel.org
> ---
>  arch/x86/ia32/ia32entry.S | 62 ++++++++++++++++++++++++++++-------------------
>  1 file changed, 37 insertions(+), 25 deletions(-)
> 
> diff --git a/arch/x86/ia32/ia32entry.S b/arch/x86/ia32/ia32entry.S
> index 86cbfe6..0ff676a 100644
> --- a/arch/x86/ia32/ia32entry.S
> +++ b/arch/x86/ia32/ia32entry.S
> @@ -29,28 +29,6 @@
>  
>  	.section .entry.text, "ax"
>  
> -	/*
> -	 * Reload arg registers from stack in case ptrace changed them.
> -	 * We don't reload %eax because syscall_trace_enter() returned
> -	 * the %rax value we should see.  Instead, we just truncate that
> -	 * value to 32 bits again as we did on entry from user mode.
> -	 * If it's a new value set by user_regset during entry tracing,
> -	 * this matches the normal truncation of the user-mode value.
> -	 * If it's -1 to make us punt the syscall, then (u32)-1 is still
> -	 * an appropriately invalid value.

Can't say that I'm crazy about the replication of (a shorter version of)
that comment three times though.

The macro is in the same file so looking it up is trivial. So what's the
point?

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--

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

* Re: [PATCH 1/2] x86/asm/entry/32: Open-code CLEAR_RREGS. No code changes.
  2015-06-02 19:04 [PATCH 1/2] x86/asm/entry/32: Open-code CLEAR_RREGS. No code changes Denys Vlasenko
  2015-06-02 19:04 ` [PATCH 2/2] x86/asm/entry/32: Open-code LOAD_ARGS32. " Denys Vlasenko
@ 2015-06-02 19:34 ` Steven Rostedt
  2015-06-02 20:25   ` Denys Vlasenko
  2015-06-03  7:04   ` Ingo Molnar
  2015-06-07  8:31 ` [tip:x86/asm] x86/asm/entry/32: Open-code CLEAR_RREGS tip-bot for Denys Vlasenko
  2 siblings, 2 replies; 8+ messages in thread
From: Steven Rostedt @ 2015-06-02 19:34 UTC (permalink / raw)
  To: Denys Vlasenko
  Cc: Ingo Molnar, Linus Torvalds, Borislav Petkov, H. Peter Anvin,
	Andy Lutomirski, Oleg Nesterov, Frederic Weisbecker,
	Alexei Starovoitov, Will Drewry, Kees Cook, x86, linux-kernel

On Tue,  2 Jun 2015 21:04:01 +0200
Denys Vlasenko <dvlasenk@redhat.com> wrote:

> This macro is small, has only four callsites, and one of them is
> slightly different using a conditional parameter.
> 
> A few saved lines aren't worth the resulting obfuscation.

I'm curious, why? Did someone recommend this change? I don't see it as
obfuscation at all.

-- Steve


> 
> Generated machine code is identical.

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

* Re: [PATCH 1/2] x86/asm/entry/32: Open-code CLEAR_RREGS. No code changes.
  2015-06-02 19:34 ` [PATCH 1/2] x86/asm/entry/32: Open-code CLEAR_RREGS. No code changes Steven Rostedt
@ 2015-06-02 20:25   ` Denys Vlasenko
  2015-06-03  7:04   ` Ingo Molnar
  1 sibling, 0 replies; 8+ messages in thread
From: Denys Vlasenko @ 2015-06-02 20:25 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Ingo Molnar, Linus Torvalds, Borislav Petkov, H. Peter Anvin,
	Andy Lutomirski, Oleg Nesterov, Frederic Weisbecker,
	Alexei Starovoitov, Will Drewry, Kees Cook, x86, linux-kernel

On 06/02/2015 09:34 PM, Steven Rostedt wrote:
> On Tue,  2 Jun 2015 21:04:01 +0200
> Denys Vlasenko <dvlasenk@redhat.com> wrote:
> 
>> This macro is small, has only four callsites, and one of them is
>> slightly different using a conditional parameter.
>>
>> A few saved lines aren't worth the resulting obfuscation.
> 
> I'm curious, why? Did someone recommend this change?

I'm proposing to do this. Of course, I don't expect that
any my patch must be accepted.


> I don't see it as obfuscation at all.

Riddle me this, looking at the current code. What's up with that
strange manipulations of %r9 register? Why the code in
SYSCALL and SYSENTER entry points is not the same,
why "r9 dance" is done only in one of these entry points?


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

* Re: [PATCH 1/2] x86/asm/entry/32: Open-code CLEAR_RREGS. No code changes.
  2015-06-02 19:34 ` [PATCH 1/2] x86/asm/entry/32: Open-code CLEAR_RREGS. No code changes Steven Rostedt
  2015-06-02 20:25   ` Denys Vlasenko
@ 2015-06-03  7:04   ` Ingo Molnar
  1 sibling, 0 replies; 8+ messages in thread
From: Ingo Molnar @ 2015-06-03  7:04 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Denys Vlasenko, Linus Torvalds, Borislav Petkov, H. Peter Anvin,
	Andy Lutomirski, Oleg Nesterov, Frederic Weisbecker,
	Alexei Starovoitov, Will Drewry, Kees Cook, x86, linux-kernel


* Steven Rostedt <rostedt@goodmis.org> wrote:

> On Tue,  2 Jun 2015 21:04:01 +0200
> Denys Vlasenko <dvlasenk@redhat.com> wrote:
> 
> > This macro is small, has only four callsites, and one of them is slightly 
> > different using a conditional parameter.
> > 
> > A few saved lines aren't worth the resulting obfuscation.
> 
> I'm curious, why? Did someone recommend this change? I don't see it as 
> obfuscation at all.

So here are a few easy questions, I'm wondering how many minutes it takes for you 
to answer them correctly:

 - What does the CLEAR_RREGS name stand for?

 - What is this macro's purpose?

 - In a single case CLEAR_RREGS takes a 'r9' argument:

    arch/x86/ia32/ia32entry.S:	CLEAR_RREGS
    arch/x86/ia32/ia32entry.S:	CLEAR_RREGS
    arch/x86/ia32/ia32entry.S:	CLEAR_RREGS r9
    arch/x86/ia32/ia32entry.S:	CLEAR_RREGS
    arch/x86/ia32/ia32entry.S:	CLEAR_RREGS

   What is the 'r9' argument's purpose and why is activated in the place where
   it's activated?

The CLEAR_RREGS macro has zero comments. If it takes more than a quick glance to 
determine all these three first-order questions from the source code, then it's an 
obvious code cleanliness fail which needs to be improved.

Thanks,

	Ingo

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

* [tip:x86/asm] x86/asm/entry/32: Open-code CLEAR_RREGS
  2015-06-02 19:04 [PATCH 1/2] x86/asm/entry/32: Open-code CLEAR_RREGS. No code changes Denys Vlasenko
  2015-06-02 19:04 ` [PATCH 2/2] x86/asm/entry/32: Open-code LOAD_ARGS32. " Denys Vlasenko
  2015-06-02 19:34 ` [PATCH 1/2] x86/asm/entry/32: Open-code CLEAR_RREGS. No code changes Steven Rostedt
@ 2015-06-07  8:31 ` tip-bot for Denys Vlasenko
  2 siblings, 0 replies; 8+ messages in thread
From: tip-bot for Denys Vlasenko @ 2015-06-07  8:31 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: fweisbec, peterz, ast, linux-kernel, oleg, brgerst, keescook,
	rostedt, tglx, wad, akpm, mingo, luto, torvalds, hpa, bp,
	dvlasenk

Commit-ID:  ef0cd5dc25404594f832dad9133abae52e3b2fa3
Gitweb:     http://git.kernel.org/tip/ef0cd5dc25404594f832dad9133abae52e3b2fa3
Author:     Denys Vlasenko <dvlasenk@redhat.com>
AuthorDate: Tue, 2 Jun 2015 21:04:01 +0200
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Fri, 5 Jun 2015 13:22:22 +0200

x86/asm/entry/32: Open-code CLEAR_RREGS

This macro is small, has only four callsites, and one of them is
slightly different using a conditional parameter.

A few saved lines aren't worth the resulting obfuscation.

Generated machine code is identical.

Signed-off-by: Denys Vlasenko <dvlasenk@redhat.com>
[ Added comments. ]
Cc: Alexei Starovoitov <ast@plumgrid.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Brian Gerst <brgerst@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Will Drewry <wad@chromium.org>
Link: http://lkml.kernel.org/r/1433271842-9139-1-git-send-email-dvlasenk@redhat.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/entry/ia32entry.S | 33 ++++++++++++++++++++-------------
 1 file changed, 20 insertions(+), 13 deletions(-)

diff --git a/arch/x86/entry/ia32entry.S b/arch/x86/entry/ia32entry.S
index f00a409..8a45d2c 100644
--- a/arch/x86/entry/ia32entry.S
+++ b/arch/x86/entry/ia32entry.S
@@ -29,15 +29,6 @@
 
 	.section .entry.text, "ax"
 
-	/* clobbers %rax */
-	.macro  CLEAR_RREGS _r9=rax
-	xorl 	%eax,%eax
-	movq	%rax,R11(%rsp)
-	movq	%rax,R10(%rsp)
-	movq	%\_r9,R9(%rsp)
-	movq	%rax,R8(%rsp)
-	.endm
-
 	/*
 	 * Reload arg registers from stack in case ptrace changed them.
 	 * We don't reload %eax because syscall_trace_enter() returned
@@ -243,7 +234,11 @@ sysexit_from_sys_call:
 	TRACE_IRQS_OFF
 	testl %edi, ASM_THREAD_INFO(TI_flags, %rsp, SIZEOF_PTREGS)
 	jz \exit
-	CLEAR_RREGS
+	xorl	%eax, %eax	/* do not leak kernel information */
+	movq	%rax, R11(%rsp)
+	movq	%rax, R10(%rsp)
+	movq	%rax, R9(%rsp)
+	movq	%rax, R8(%rsp)
 	jmp int_with_check
 	.endm
 
@@ -267,7 +262,11 @@ sysenter_tracesys:
 	jz	sysenter_auditsys
 #endif
 	SAVE_EXTRA_REGS
-	CLEAR_RREGS
+	xorl	%eax, %eax	/* do not leak kernel information */
+	movq	%rax, R11(%rsp)
+	movq	%rax, R10(%rsp)
+	movq	%rax, R9(%rsp)
+	movq	%rax, R8(%rsp)
 	movq	%rsp,%rdi        /* &pt_regs -> arg1 */
 	call	syscall_trace_enter
 	LOAD_ARGS32  /* reload args from stack in case ptrace changed it */
@@ -407,7 +406,11 @@ cstar_tracesys:
 #endif
 	xchgl %r9d,%ebp
 	SAVE_EXTRA_REGS
-	CLEAR_RREGS r9
+	xorl	%eax, %eax	/* do not leak kernel information */
+	movq	%rax, R11(%rsp)
+	movq	%rax, R10(%rsp)
+	movq	%r9,  R9(%rsp)
+	movq	%rax, R8(%rsp)
 	movq %rsp,%rdi        /* &pt_regs -> arg1 */
 	call syscall_trace_enter
 	LOAD_ARGS32 1	/* reload args from stack in case ptrace changed it */
@@ -422,7 +425,11 @@ ia32_badarg:
 	jmp ia32_sysret
 
 ia32_ret_from_sys_call:
-	CLEAR_RREGS
+	xorl	%eax, %eax	/* do not leak kernel information */
+	movq	%rax, R11(%rsp)
+	movq	%rax, R10(%rsp)
+	movq	%rax, R9(%rsp)
+	movq	%rax, R8(%rsp)
 	jmp int_ret_from_sys_call
 
 /*

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

* [tip:x86/asm] x86/asm/entry/32: Open-code LOAD_ARGS32
  2015-06-02 19:04 ` [PATCH 2/2] x86/asm/entry/32: Open-code LOAD_ARGS32. " Denys Vlasenko
  2015-06-02 19:27   ` Borislav Petkov
@ 2015-06-07  8:31   ` tip-bot for Denys Vlasenko
  1 sibling, 0 replies; 8+ messages in thread
From: tip-bot for Denys Vlasenko @ 2015-06-07  8:31 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: tglx, brgerst, linux-kernel, fweisbec, ast, keescook, akpm,
	rostedt, hpa, luto, peterz, torvalds, oleg, wad, dvlasenk, bp,
	mingo

Commit-ID:  73cbf687914fd5f4ef88a42a55784fd28b7450cf
Gitweb:     http://git.kernel.org/tip/73cbf687914fd5f4ef88a42a55784fd28b7450cf
Author:     Denys Vlasenko <dvlasenk@redhat.com>
AuthorDate: Tue, 2 Jun 2015 21:04:02 +0200
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Fri, 5 Jun 2015 13:22:22 +0200

x86/asm/entry/32: Open-code LOAD_ARGS32

This macro is small, has only three callsites, and one of them
is slightly different using a conditional parameter.

A few saved lines aren't worth the resulting obfuscation.

Generated machine code is identical.

Signed-off-by: Denys Vlasenko <dvlasenk@redhat.com>
Cc: Alexei Starovoitov <ast@plumgrid.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Brian Gerst <brgerst@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Will Drewry <wad@chromium.org>
Link: http://lkml.kernel.org/r/1433271842-9139-2-git-send-email-dvlasenk@redhat.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/entry/ia32entry.S | 54 +++++++++++++++++++++++++---------------------
 1 file changed, 29 insertions(+), 25 deletions(-)

diff --git a/arch/x86/entry/ia32entry.S b/arch/x86/entry/ia32entry.S
index 8a45d2c..56f819e 100644
--- a/arch/x86/entry/ia32entry.S
+++ b/arch/x86/entry/ia32entry.S
@@ -29,28 +29,6 @@
 
 	.section .entry.text, "ax"
 
-	/*
-	 * Reload arg registers from stack in case ptrace changed them.
-	 * We don't reload %eax because syscall_trace_enter() returned
-	 * the %rax value we should see.  Instead, we just truncate that
-	 * value to 32 bits again as we did on entry from user mode.
-	 * If it's a new value set by user_regset during entry tracing,
-	 * this matches the normal truncation of the user-mode value.
-	 * If it's -1 to make us punt the syscall, then (u32)-1 is still
-	 * an appropriately invalid value.
-	 */
-	.macro LOAD_ARGS32 _r9=0
-	.if \_r9
-	movl R9(%rsp),%r9d
-	.endif
-	movl RCX(%rsp),%ecx
-	movl RDX(%rsp),%edx
-	movl RSI(%rsp),%esi
-	movl RDI(%rsp),%edi
-	movl %eax,%eax			/* zero extension */
-	.endm
-	
-
 #ifdef CONFIG_PARAVIRT
 ENTRY(native_usergs_sysret32)
 	swapgs
@@ -269,7 +247,14 @@ sysenter_tracesys:
 	movq	%rax, R8(%rsp)
 	movq	%rsp,%rdi        /* &pt_regs -> arg1 */
 	call	syscall_trace_enter
-	LOAD_ARGS32  /* reload args from stack in case ptrace changed it */
+
+	/* Reload arg registers from stack. (see sysenter_tracesys) */
+	movl	RCX(%rsp), %ecx
+	movl	RDX(%rsp), %edx
+	movl	RSI(%rsp), %esi
+	movl	RDI(%rsp), %edi
+	movl	%eax, %eax	/* zero extension */
+
 	RESTORE_EXTRA_REGS
 	jmp	sysenter_do_call
 ENDPROC(ia32_sysenter_target)
@@ -413,7 +398,15 @@ cstar_tracesys:
 	movq	%rax, R8(%rsp)
 	movq %rsp,%rdi        /* &pt_regs -> arg1 */
 	call syscall_trace_enter
-	LOAD_ARGS32 1	/* reload args from stack in case ptrace changed it */
+	movl	R9(%rsp),%r9d
+
+	/* Reload arg registers from stack. (see sysenter_tracesys) */
+	movl	RCX(%rsp), %ecx
+	movl	RDX(%rsp), %edx
+	movl	RSI(%rsp), %esi
+	movl	RDI(%rsp), %edi
+	movl	%eax, %eax	/* zero extension */
+
 	RESTORE_EXTRA_REGS
 	xchgl %ebp,%r9d
 	jmp cstar_do_call
@@ -502,7 +495,18 @@ ia32_tracesys:
 	SAVE_EXTRA_REGS
 	movq %rsp,%rdi        /* &pt_regs -> arg1 */
 	call syscall_trace_enter
-	LOAD_ARGS32	/* reload args from stack in case ptrace changed it */
+	/*
+	 * Reload arg registers from stack in case ptrace changed them.
+	 * Don't reload %eax because syscall_trace_enter() returned
+	 * the %rax value we should see.  But do truncate it to 32 bits.
+	 * If it's -1 to make us punt the syscall, then (u32)-1 is still
+	 * an appropriately invalid value.
+	 */
+	movl	RCX(%rsp), %ecx
+	movl	RDX(%rsp), %edx
+	movl	RSI(%rsp), %esi
+	movl	RDI(%rsp), %edi
+	movl	%eax, %eax	/* zero extension */
 	RESTORE_EXTRA_REGS
 	jmp ia32_do_call
 END(ia32_syscall)

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

end of thread, other threads:[~2015-06-07  8:32 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-06-02 19:04 [PATCH 1/2] x86/asm/entry/32: Open-code CLEAR_RREGS. No code changes Denys Vlasenko
2015-06-02 19:04 ` [PATCH 2/2] x86/asm/entry/32: Open-code LOAD_ARGS32. " Denys Vlasenko
2015-06-02 19:27   ` Borislav Petkov
2015-06-07  8:31   ` [tip:x86/asm] x86/asm/entry/32: Open-code LOAD_ARGS32 tip-bot for Denys Vlasenko
2015-06-02 19:34 ` [PATCH 1/2] x86/asm/entry/32: Open-code CLEAR_RREGS. No code changes Steven Rostedt
2015-06-02 20:25   ` Denys Vlasenko
2015-06-03  7:04   ` Ingo Molnar
2015-06-07  8:31 ` [tip:x86/asm] x86/asm/entry/32: Open-code CLEAR_RREGS tip-bot for Denys Vlasenko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).