Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1541678 > unrolled thread

[PATCH 1/2] x86/boot/64: use 'push' instead of 'call' in start_cpu()

Started byJosh Poimboeuf <jpoimboe@redhat.com>
First post2016-12-14 04:30 +0100
Last post2016-12-14 21:20 +0100
Articles 4 — 3 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  [PATCH 1/2] x86/boot/64: use 'push' instead of 'call' in start_cpu() Josh Poimboeuf <jpoimboe@redhat.com> - 2016-12-14 04:30 +0100
    [tip:x86/urgent] x86/boot/64: Use 'push' instead of 'call' in  start_cpu() tip-bot for Josh Poimboeuf <tipbot@zytor.com> - 2016-12-14 09:40 +0100
      Re: [tip:x86/urgent] x86/boot/64: Use 'push' instead of 'call' in start_cpu() hpa@zytor.com - 2016-12-14 20:30 +0100
        Re: [tip:x86/urgent] x86/boot/64: Use 'push' instead of 'call' in  start_cpu() Josh Poimboeuf <jpoimboe@redhat.com> - 2016-12-14 21:20 +0100

#1541678 — [PATCH 1/2] x86/boot/64: use 'push' instead of 'call' in start_cpu()

FromJosh Poimboeuf <jpoimboe@redhat.com>
Date2016-12-14 04:30 +0100
Subject[PATCH 1/2] x86/boot/64: use 'push' instead of 'call' in start_cpu()
Message-ID<sO8qu-1gQ-17@gated-at.bofh.it>
start_cpu() pushes a text address on the stack so that stack traces from
idle tasks will show start_cpu() at the end.  But it uses a call
instruction to do that, which is rather obtuse.  Use a straightforward
push instead.

Suggested-by: Borislav Petkov <bp@alien8.de>
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
---
 arch/x86/kernel/head_64.S | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kernel/head_64.S b/arch/x86/kernel/head_64.S
index 90de288..1facaf4 100644
--- a/arch/x86/kernel/head_64.S
+++ b/arch/x86/kernel/head_64.S
@@ -298,7 +298,7 @@ ENTRY(start_cpu)
 	 *	REX.W + FF /5 JMP m16:64 Jump far, absolute indirect,
 	 *		address given in m16:64.
 	 */
-	call	1f		# put return address on stack for unwinder
+	pushq	$1f		# put return address on stack for unwinder
 1:	xorq	%rbp, %rbp	# clear frame pointer
 	movq	initial_code(%rip), %rax
 	pushq	$__KERNEL_CS	# set correct cs
-- 
2.7.4

[toc] | [next] | [standalone]


#1541758 — [tip:x86/urgent] x86/boot/64: Use 'push' instead of 'call' in start_cpu()

Fromtip-bot for Josh Poimboeuf <tipbot@zytor.com>
Date2016-12-14 09:40 +0100
Subject[tip:x86/urgent] x86/boot/64: Use 'push' instead of 'call' in start_cpu()
Message-ID<sOdgu-4cB-1@gated-at.bofh.it>
In reply to#1541678
Commit-ID:  ec2d86a9b646d93f1948569f368e2c6f5449e6c7
Gitweb:     http://git.kernel.org/tip/ec2d86a9b646d93f1948569f368e2c6f5449e6c7
Author:     Josh Poimboeuf <jpoimboe@redhat.com>
AuthorDate: Tue, 13 Dec 2016 21:25:35 -0600
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Wed, 14 Dec 2016 08:48:05 +0100

x86/boot/64: Use 'push' instead of 'call' in start_cpu()

start_cpu() pushes a text address on the stack so that stack traces from
idle tasks will show start_cpu() at the end.  But it uses a call
instruction to do that, which is rather obtuse.  Use a straightforward
push instead.

Suggested-by: Borislav Petkov <bp@alien8.de>
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Brian Gerst <brgerst@gmail.com>
Cc: Denys Vlasenko <dvlasenk@redhat.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Link: http://lkml.kernel.org/r/4d8a1952759721d42d1e62ba9e4a7e3ac5df8574.1481685203.git.jpoimboe@redhat.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/kernel/head_64.S | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kernel/head_64.S b/arch/x86/kernel/head_64.S
index 90de288..1facaf4 100644
--- a/arch/x86/kernel/head_64.S
+++ b/arch/x86/kernel/head_64.S
@@ -298,7 +298,7 @@ ENTRY(start_cpu)
 	 *	REX.W + FF /5 JMP m16:64 Jump far, absolute indirect,
 	 *		address given in m16:64.
 	 */
-	call	1f		# put return address on stack for unwinder
+	pushq	$1f		# put return address on stack for unwinder
 1:	xorq	%rbp, %rbp	# clear frame pointer
 	movq	initial_code(%rip), %rax
 	pushq	$__KERNEL_CS	# set correct cs

[toc] | [prev] | [next] | [standalone]


#1542140 — Re: [tip:x86/urgent] x86/boot/64: Use 'push' instead of 'call' in start_cpu()

Fromhpa@zytor.com
Date2016-12-14 20:30 +0100
SubjectRe: [tip:x86/urgent] x86/boot/64: Use 'push' instead of 'call' in start_cpu()
Message-ID<sOnpv-2DO-9@gated-at.bofh.it>
In reply to#1541758
On December 14, 2016 12:36:58 AM PST, tip-bot for Josh Poimboeuf <tipbot@zytor.com> wrote:
>Commit-ID:  ec2d86a9b646d93f1948569f368e2c6f5449e6c7
>Gitweb:    
>http://git.kernel.org/tip/ec2d86a9b646d93f1948569f368e2c6f5449e6c7
>Author:     Josh Poimboeuf <jpoimboe@redhat.com>
>AuthorDate: Tue, 13 Dec 2016 21:25:35 -0600
>Committer:  Ingo Molnar <mingo@kernel.org>
>CommitDate: Wed, 14 Dec 2016 08:48:05 +0100
>
>x86/boot/64: Use 'push' instead of 'call' in start_cpu()
>
>start_cpu() pushes a text address on the stack so that stack traces
>from
>idle tasks will show start_cpu() at the end.  But it uses a call
>instruction to do that, which is rather obtuse.  Use a straightforward
>push instead.
>
>Suggested-by: Borislav Petkov <bp@alien8.de>
>Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
>Cc: Andy Lutomirski <luto@kernel.org>
>Cc: Brian Gerst <brgerst@gmail.com>
>Cc: Denys Vlasenko <dvlasenk@redhat.com>
>Cc: H. Peter Anvin <hpa@zytor.com>
>Cc: Linus Torvalds <torvalds@linux-foundation.org>
>Cc: Peter Zijlstra <peterz@infradead.org>
>Cc: Thomas Gleixner <tglx@linutronix.de>
>Link:
>http://lkml.kernel.org/r/4d8a1952759721d42d1e62ba9e4a7e3ac5df8574.1481685203.git.jpoimboe@redhat.com
>Signed-off-by: Ingo Molnar <mingo@kernel.org>
>---
> arch/x86/kernel/head_64.S | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/arch/x86/kernel/head_64.S b/arch/x86/kernel/head_64.S
>index 90de288..1facaf4 100644
>--- a/arch/x86/kernel/head_64.S
>+++ b/arch/x86/kernel/head_64.S
>@@ -298,7 +298,7 @@ ENTRY(start_cpu)
> 	 *	REX.W + FF /5 JMP m16:64 Jump far, absolute indirect,
> 	 *		address given in m16:64.
> 	 */
>-	call	1f		# put return address on stack for unwinder
>+	pushq	$1f		# put return address on stack for unwinder
> 1:	xorq	%rbp, %rbp	# clear frame pointer
> 	movq	initial_code(%rip), %rax
> 	pushq	$__KERNEL_CS	# set correct cs

This adds another relocation to the kernel.  I hope this is safe at this point in the code?
-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

[toc] | [prev] | [next] | [standalone]


#1542174 — Re: [tip:x86/urgent] x86/boot/64: Use 'push' instead of 'call' in start_cpu()

FromJosh Poimboeuf <jpoimboe@redhat.com>
Date2016-12-14 21:20 +0100
SubjectRe: [tip:x86/urgent] x86/boot/64: Use 'push' instead of 'call' in start_cpu()
Message-ID<sOobU-3BZ-7@gated-at.bofh.it>
In reply to#1542140
On Wed, Dec 14, 2016 at 11:24:19AM -0800, hpa@zytor.com wrote:
> On December 14, 2016 12:36:58 AM PST, tip-bot for Josh Poimboeuf <tipbot@zytor.com> wrote:
> >diff --git a/arch/x86/kernel/head_64.S b/arch/x86/kernel/head_64.S
> >index 90de288..1facaf4 100644
> >--- a/arch/x86/kernel/head_64.S
> >+++ b/arch/x86/kernel/head_64.S
> >@@ -298,7 +298,7 @@ ENTRY(start_cpu)
> > 	 *	REX.W + FF /5 JMP m16:64 Jump far, absolute indirect,
> > 	 *		address given in m16:64.
> > 	 */
> >-	call	1f		# put return address on stack for unwinder
> >+	pushq	$1f		# put return address on stack for unwinder
> > 1:	xorq	%rbp, %rbp	# clear frame pointer
> > 	movq	initial_code(%rip), %rax
> > 	pushq	$__KERNEL_CS	# set correct cs
> 
> This adds another relocation to the kernel.  I hope this is safe at this point in the code?

AFAIK, it should be fine.  All relocations were either applied at build
time, or for KASLR, in the compressed boot code which extracts and
copies this code.

Also there are already a bunch of relocations in the rest of the code in
this file, all of which runs before this code does.

(And even if that weren't the case, this address is only used for
displaying stack traces, so pushing a zero or some garbage here wouldn't
really break anything.)

-- 
Josh

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web