* [PATCH 0/2] x86/boot/64: a couple of start_cpu() cleanups @ 2016-12-14 3:25 Josh Poimboeuf 2016-12-14 3:25 ` [PATCH 1/2] x86/boot/64: use 'push' instead of 'call' in start_cpu() Josh Poimboeuf 2016-12-14 3:25 ` [PATCH 2/2] x86/boot/64: push correct start_cpu() return address Josh Poimboeuf 0 siblings, 2 replies; 7+ messages in thread From: Josh Poimboeuf @ 2016-12-14 3:25 UTC (permalink / raw) To: x86; +Cc: linux-kernel, Borislav Petkov, Andy Lutomirski Josh Poimboeuf (2): x86/boot/64: use 'push' instead of 'call' in start_cpu() x86/boot/64: push correct start_cpu() return address arch/x86/kernel/head_64.S | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) -- 2.7.4 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2] x86/boot/64: use 'push' instead of 'call' in start_cpu() 2016-12-14 3:25 [PATCH 0/2] x86/boot/64: a couple of start_cpu() cleanups Josh Poimboeuf @ 2016-12-14 3:25 ` Josh Poimboeuf 2016-12-14 8:36 ` [tip:x86/urgent] x86/boot/64: Use " tip-bot for Josh Poimboeuf 2016-12-14 3:25 ` [PATCH 2/2] x86/boot/64: push correct start_cpu() return address Josh Poimboeuf 1 sibling, 1 reply; 7+ messages in thread From: Josh Poimboeuf @ 2016-12-14 3:25 UTC (permalink / raw) To: x86; +Cc: linux-kernel, Borislav Petkov, Andy Lutomirski 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 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [tip:x86/urgent] x86/boot/64: Use 'push' instead of 'call' in start_cpu() 2016-12-14 3:25 ` [PATCH 1/2] x86/boot/64: use 'push' instead of 'call' in start_cpu() Josh Poimboeuf @ 2016-12-14 8:36 ` tip-bot for Josh Poimboeuf 2016-12-14 19:24 ` hpa 0 siblings, 1 reply; 7+ messages in thread From: tip-bot for Josh Poimboeuf @ 2016-12-14 8:36 UTC (permalink / raw) To: linux-tip-commits Cc: linux-kernel, hpa, bp, tglx, torvalds, peterz, luto, jpoimboe, mingo, brgerst, dvlasenk 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 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [tip:x86/urgent] x86/boot/64: Use 'push' instead of 'call' in start_cpu() 2016-12-14 8:36 ` [tip:x86/urgent] x86/boot/64: Use " tip-bot for Josh Poimboeuf @ 2016-12-14 19:24 ` hpa 2016-12-14 20:13 ` Josh Poimboeuf 0 siblings, 1 reply; 7+ messages in thread From: hpa @ 2016-12-14 19:24 UTC (permalink / raw) To: linux-tip-commits, tip-bot for Josh Poimboeuf Cc: linux-kernel, bp, tglx, torvalds, peterz, luto, jpoimboe, mingo, brgerst, dvlasenk 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. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [tip:x86/urgent] x86/boot/64: Use 'push' instead of 'call' in start_cpu() 2016-12-14 19:24 ` hpa @ 2016-12-14 20:13 ` Josh Poimboeuf 0 siblings, 0 replies; 7+ messages in thread From: Josh Poimboeuf @ 2016-12-14 20:13 UTC (permalink / raw) To: hpa Cc: linux-tip-commits, tip-bot for Josh Poimboeuf, linux-kernel, bp, tglx, torvalds, peterz, luto, mingo, brgerst, dvlasenk 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/2] x86/boot/64: push correct start_cpu() return address 2016-12-14 3:25 [PATCH 0/2] x86/boot/64: a couple of start_cpu() cleanups Josh Poimboeuf 2016-12-14 3:25 ` [PATCH 1/2] x86/boot/64: use 'push' instead of 'call' in start_cpu() Josh Poimboeuf @ 2016-12-14 3:25 ` Josh Poimboeuf 2016-12-14 8:37 ` [tip:x86/urgent] x86/boot/64: Push " tip-bot for Josh Poimboeuf 1 sibling, 1 reply; 7+ messages in thread From: Josh Poimboeuf @ 2016-12-14 3:25 UTC (permalink / raw) To: x86; +Cc: linux-kernel, Borislav Petkov, Andy Lutomirski 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 currently shows the wrong function offset. It's more correct to show the address immediately after the 'lretq' instruction. Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com> --- arch/x86/kernel/head_64.S | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/arch/x86/kernel/head_64.S b/arch/x86/kernel/head_64.S index 1facaf4..b467b14 100644 --- a/arch/x86/kernel/head_64.S +++ b/arch/x86/kernel/head_64.S @@ -298,12 +298,13 @@ ENTRY(start_cpu) * REX.W + FF /5 JMP m16:64 Jump far, absolute indirect, * address given in m16:64. */ - pushq $1f # put return address on stack for unwinder -1: xorq %rbp, %rbp # clear frame pointer + pushq $.Lafter_lret # put return address on stack for unwinder + xorq %rbp, %rbp # clear frame pointer movq initial_code(%rip), %rax pushq $__KERNEL_CS # set correct cs pushq %rax # target address in negative space lretq +.Lafter_lret: ENDPROC(start_cpu) #include "verify_cpu.S" -- 2.7.4 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [tip:x86/urgent] x86/boot/64: Push correct start_cpu() return address 2016-12-14 3:25 ` [PATCH 2/2] x86/boot/64: push correct start_cpu() return address Josh Poimboeuf @ 2016-12-14 8:37 ` tip-bot for Josh Poimboeuf 0 siblings, 0 replies; 7+ messages in thread From: tip-bot for Josh Poimboeuf @ 2016-12-14 8:37 UTC (permalink / raw) To: linux-tip-commits Cc: jpoimboe, dvlasenk, linux-kernel, hpa, brgerst, peterz, tglx, mingo, luto, bp, torvalds Commit-ID: 31dcfec11f827e9a5d8720fe4728f1305894884f Gitweb: http://git.kernel.org/tip/31dcfec11f827e9a5d8720fe4728f1305894884f Author: Josh Poimboeuf <jpoimboe@redhat.com> AuthorDate: Tue, 13 Dec 2016 21:25:36 -0600 Committer: Ingo Molnar <mingo@kernel.org> CommitDate: Wed, 14 Dec 2016 08:48:05 +0100 x86/boot/64: Push correct start_cpu() return address 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 currently shows the wrong function offset. It's more correct to show the address immediately after the 'lretq' instruction. Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com> Cc: Andy Lutomirski <luto@kernel.org> Cc: Borislav Petkov <bp@alien8.de> 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/2cadd9f16c77da7ee7957bfc5e1c67928c23ca48.1481685203.git.jpoimboe@redhat.com Signed-off-by: Ingo Molnar <mingo@kernel.org> --- arch/x86/kernel/head_64.S | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/arch/x86/kernel/head_64.S b/arch/x86/kernel/head_64.S index 1facaf4..b467b14 100644 --- a/arch/x86/kernel/head_64.S +++ b/arch/x86/kernel/head_64.S @@ -298,12 +298,13 @@ ENTRY(start_cpu) * REX.W + FF /5 JMP m16:64 Jump far, absolute indirect, * address given in m16:64. */ - pushq $1f # put return address on stack for unwinder -1: xorq %rbp, %rbp # clear frame pointer + pushq $.Lafter_lret # put return address on stack for unwinder + xorq %rbp, %rbp # clear frame pointer movq initial_code(%rip), %rax pushq $__KERNEL_CS # set correct cs pushq %rax # target address in negative space lretq +.Lafter_lret: ENDPROC(start_cpu) #include "verify_cpu.S" ^ permalink raw reply related [flat|nested] 7+ messages in thread
end of thread, other threads:[~2016-12-14 20:13 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-12-14 3:25 [PATCH 0/2] x86/boot/64: a couple of start_cpu() cleanups Josh Poimboeuf 2016-12-14 3:25 ` [PATCH 1/2] x86/boot/64: use 'push' instead of 'call' in start_cpu() Josh Poimboeuf 2016-12-14 8:36 ` [tip:x86/urgent] x86/boot/64: Use " tip-bot for Josh Poimboeuf 2016-12-14 19:24 ` hpa 2016-12-14 20:13 ` Josh Poimboeuf 2016-12-14 3:25 ` [PATCH 2/2] x86/boot/64: push correct start_cpu() return address Josh Poimboeuf 2016-12-14 8:37 ` [tip:x86/urgent] x86/boot/64: Push " tip-bot for Josh Poimboeuf
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).