* Re: [PATCH] x86/alternatives: Fix optimize_nops() checking
2018-01-10 11:28 ` [PATCH] x86/alternatives: Fix optimize_nops() checking Borislav Petkov
@ 2018-01-10 11:36 ` David Woodhouse
2018-01-10 11:45 ` Borislav Petkov
2018-01-10 17:33 ` [tip:x86/pti] " tip-bot for Borislav Petkov
` (3 subsequent siblings)
4 siblings, 1 reply; 44+ messages in thread
From: David Woodhouse @ 2018-01-10 11:36 UTC (permalink / raw)
To: Borislav Petkov
Cc: Linus Torvalds, Andi Kleen, Thomas Gleixner,
the arch/x86 maintainers, Linux Kernel Mailing List, Paul Turner,
Andrew Lutomirski, Peter Zijlstra, Tom Lendacky, Tim Chen,
Greg Kroah-Hartman, Dave Hansen, Jiri Kosina, Andi Kleen
[-- Attachment #1: Type: text/plain, Size: 683 bytes --]
On Wed, 2018-01-10 at 12:28 +0100, Borislav Petkov wrote:
> From: Borislav Petkov <bp@suse.de>
> Date: Wed, 10 Jan 2018 12:14:07 +0100
>
> We check only the first byte whether it is a NOP but if David Woodhouse
> wants to do some crazy experiments with slapping NOPs in front of the
> payload and having the actual instructions after it, this "optimized"
> test breaks. :-)
:)
Thank you.
Tested-by: David Woodhouse <dwmw@amazon.co.uk>
That fixed and understood, I shall remove the offending NOPs anyway,
because aligning instructions in the *altinstr* section is entirely
pointless as they might still not be aligned when they get copied into
place anyway.
[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5213 bytes --]
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH] x86/alternatives: Fix optimize_nops() checking
2018-01-10 11:36 ` David Woodhouse
@ 2018-01-10 11:45 ` Borislav Petkov
2018-01-10 11:49 ` David Woodhouse
0 siblings, 1 reply; 44+ messages in thread
From: Borislav Petkov @ 2018-01-10 11:45 UTC (permalink / raw)
To: David Woodhouse
Cc: Linus Torvalds, Andi Kleen, Thomas Gleixner,
the arch/x86 maintainers, Linux Kernel Mailing List, Paul Turner,
Andrew Lutomirski, Peter Zijlstra, Tom Lendacky, Tim Chen,
Greg Kroah-Hartman, Dave Hansen, Jiri Kosina, Andi Kleen
On Wed, Jan 10, 2018 at 11:36:41AM +0000, David Woodhouse wrote:
> That fixed and understood, I shall remove the offending NOPs anyway,
> because aligning instructions in the *altinstr* section is entirely
> pointless as they might still not be aligned when they get copied into
> place anyway.
Yap. It was still a good experiment because we found this shortcoming!
:-)
Thx.
--
Regards/Gruss,
Boris.
Good mailing practices for 400: avoid top-posting and trim the reply.
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH] x86/alternatives: Fix optimize_nops() checking
2018-01-10 11:45 ` Borislav Petkov
@ 2018-01-10 11:49 ` David Woodhouse
2018-01-10 11:57 ` Borislav Petkov
0 siblings, 1 reply; 44+ messages in thread
From: David Woodhouse @ 2018-01-10 11:49 UTC (permalink / raw)
To: Borislav Petkov
Cc: Linus Torvalds, Andi Kleen, Thomas Gleixner,
the arch/x86 maintainers, Linux Kernel Mailing List, Paul Turner,
Andrew Lutomirski, Peter Zijlstra, Tom Lendacky, Tim Chen,
Greg Kroah-Hartman, Dave Hansen, Jiri Kosina, Andi Kleen
[-- Attachment #1: Type: text/plain, Size: 743 bytes --]
On Wed, 2018-01-10 at 12:45 +0100, Borislav Petkov wrote:
> On Wed, Jan 10, 2018 at 11:36:41AM +0000, David Woodhouse wrote:
> >
> > That fixed and understood, I shall remove the offending NOPs anyway,
> > because aligning instructions in the *altinstr* section is entirely
> > pointless as they might still not be aligned when they get copied into
> > place anyway.
>
> Yap. It was still a good experiment because we found this shortcoming!
Don't suppose you want to make the alignment actually *work*? :)
Paul, how much of a win was it really? And not just in a microbenchmark
of the retpoline itself, saying "look Ma! This goes faster if I make it
take three times as many icache lines!", but overall system benchmarks?
:)
[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5213 bytes --]
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH] x86/alternatives: Fix optimize_nops() checking
2018-01-10 11:49 ` David Woodhouse
@ 2018-01-10 11:57 ` Borislav Petkov
2018-01-10 11:58 ` David Woodhouse
0 siblings, 1 reply; 44+ messages in thread
From: Borislav Petkov @ 2018-01-10 11:57 UTC (permalink / raw)
To: David Woodhouse
Cc: Linus Torvalds, Andi Kleen, Thomas Gleixner,
the arch/x86 maintainers, Linux Kernel Mailing List, Paul Turner,
Andrew Lutomirski, Peter Zijlstra, Tom Lendacky, Tim Chen,
Greg Kroah-Hartman, Dave Hansen, Jiri Kosina, Andi Kleen
On Wed, Jan 10, 2018 at 11:49:55AM +0000, David Woodhouse wrote:
> Don't suppose you want to make the alignment actually *work*? :)
I can try but only if it is really worth it. If we don't see it in
measurements, then it is wasted time and energy and added needless code
complexity.
--
Regards/Gruss,
Boris.
Good mailing practices for 400: avoid top-posting and trim the reply.
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH] x86/alternatives: Fix optimize_nops() checking
2018-01-10 11:57 ` Borislav Petkov
@ 2018-01-10 11:58 ` David Woodhouse
0 siblings, 0 replies; 44+ messages in thread
From: David Woodhouse @ 2018-01-10 11:58 UTC (permalink / raw)
To: Borislav Petkov
Cc: Linus Torvalds, Andi Kleen, Thomas Gleixner,
the arch/x86 maintainers, Linux Kernel Mailing List, Paul Turner,
Andrew Lutomirski, Peter Zijlstra, Tom Lendacky, Tim Chen,
Greg Kroah-Hartman, Dave Hansen, Jiri Kosina, Andi Kleen
[-- Attachment #1: Type: text/plain, Size: 398 bytes --]
On Wed, 2018-01-10 at 12:57 +0100, Borislav Petkov wrote:
> On Wed, Jan 10, 2018 at 11:49:55AM +0000, David Woodhouse wrote:
> > Don't suppose you want to make the alignment actually *work*? :)
>
> I can try but only if it is really worth it. If we don't see it in
> measurements, then it is wasted time and energy and added needless
> code complexity.
Agreed. Hence the question to Paul.
[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5213 bytes --]
^ permalink raw reply [flat|nested] 44+ messages in thread
* [tip:x86/pti] x86/alternatives: Fix optimize_nops() checking
2018-01-10 11:28 ` [PATCH] x86/alternatives: Fix optimize_nops() checking Borislav Petkov
2018-01-10 11:36 ` David Woodhouse
@ 2018-01-10 17:33 ` tip-bot for Borislav Petkov
2018-01-10 18:12 ` tip-bot for Borislav Petkov
` (2 subsequent siblings)
4 siblings, 0 replies; 44+ messages in thread
From: tip-bot for Borislav Petkov @ 2018-01-10 17:33 UTC (permalink / raw)
To: linux-tip-commits
Cc: peterz, ak, jikos, hpa, thomas.lendacky, bp, pjt, mingo,
linux-kernel, andi, dave.hansen, tglx, luto, dwmw2, tim.c.chen,
gregkh, torvalds
Commit-ID: d1cb4348f683d132ef2d468d4e9ad421486163f9
Gitweb: https://git.kernel.org/tip/d1cb4348f683d132ef2d468d4e9ad421486163f9
Author: Borislav Petkov <bp@suse.de>
AuthorDate: Wed, 10 Jan 2018 12:28:16 +0100
Committer: Thomas Gleixner <tglx@linutronix.de>
CommitDate: Wed, 10 Jan 2018 18:28:21 +0100
x86/alternatives: Fix optimize_nops() checking
The alternatives code checks only the first byte whether it is a NOP, but
with NOPs in front of the payload and having actual instructions after it
breaks the "optimized' test.
Make sure to scan all bytes before deciding to optimize the NOPs in there.
Reported-by: David Woodhouse <dwmw2@infradead.org>
Signed-off-by: Borislav Petkov <bp@suse.de>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Tim Chen <tim.c.chen@linux.intel.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Jiri Kosina <jikos@kernel.org>
Cc: Dave Hansen <dave.hansen@intel.com>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: Andrew Lutomirski <luto@kernel.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Greg Kroah-Hartman <gregkh@linux-foundation.org>
Cc: Paul Turner <pjt@google.com>
Link: https://lkml.kernel.org/r/20180110112815.mgciyf5acwacphkq@pd.tnic
---
arch/x86/kernel/alternative.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index 3344d33..e0b97e4 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -344,9 +344,12 @@ done:
static void __init_or_module noinline optimize_nops(struct alt_instr *a, u8 *instr)
{
unsigned long flags;
+ int i;
- if (instr[0] != 0x90)
- return;
+ for (i = 0; i < a->padlen; i++) {
+ if (instr[i] != 0x90)
+ return;
+ }
local_irq_save(flags);
add_nops(instr + (a->instrlen - a->padlen), a->padlen);
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [tip:x86/pti] x86/alternatives: Fix optimize_nops() checking
2018-01-10 11:28 ` [PATCH] x86/alternatives: Fix optimize_nops() checking Borislav Petkov
2018-01-10 11:36 ` David Woodhouse
2018-01-10 17:33 ` [tip:x86/pti] " tip-bot for Borislav Petkov
@ 2018-01-10 18:12 ` tip-bot for Borislav Petkov
2018-01-10 18:39 ` tip-bot for Borislav Petkov
2018-01-10 19:38 ` [PATCH] " Linus Torvalds
4 siblings, 0 replies; 44+ messages in thread
From: tip-bot for Borislav Petkov @ 2018-01-10 18:12 UTC (permalink / raw)
To: linux-tip-commits
Cc: thomas.lendacky, gregkh, ak, jikos, tim.c.chen, mingo,
dave.hansen, torvalds, peterz, dwmw2, luto, pjt, bp, tglx, hpa,
linux-kernel, andi
Commit-ID: 0795e94c2eacd888c88e2ad2321368b6b0fcb20a
Gitweb: https://git.kernel.org/tip/0795e94c2eacd888c88e2ad2321368b6b0fcb20a
Author: Borislav Petkov <bp@suse.de>
AuthorDate: Wed, 10 Jan 2018 12:28:16 +0100
Committer: Thomas Gleixner <tglx@linutronix.de>
CommitDate: Wed, 10 Jan 2018 19:09:08 +0100
x86/alternatives: Fix optimize_nops() checking
The alternatives code checks only the first byte whether it is a NOP, but
with NOPs in front of the payload and having actual instructions after it
breaks the "optimized' test.
Make sure to scan all bytes before deciding to optimize the NOPs in there.
Reported-by: David Woodhouse <dwmw2@infradead.org>
Signed-off-by: Borislav Petkov <bp@suse.de>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Tim Chen <tim.c.chen@linux.intel.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Jiri Kosina <jikos@kernel.org>
Cc: Dave Hansen <dave.hansen@intel.com>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: Andrew Lutomirski <luto@kernel.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Greg Kroah-Hartman <gregkh@linux-foundation.org>
Cc: Paul Turner <pjt@google.com>
Link: https://lkml.kernel.org/r/20180110112815.mgciyf5acwacphkq@pd.tnic
---
arch/x86/kernel/alternative.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index 3344d33..e0b97e4 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -344,9 +344,12 @@ done:
static void __init_or_module noinline optimize_nops(struct alt_instr *a, u8 *instr)
{
unsigned long flags;
+ int i;
- if (instr[0] != 0x90)
- return;
+ for (i = 0; i < a->padlen; i++) {
+ if (instr[i] != 0x90)
+ return;
+ }
local_irq_save(flags);
add_nops(instr + (a->instrlen - a->padlen), a->padlen);
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [tip:x86/pti] x86/alternatives: Fix optimize_nops() checking
2018-01-10 11:28 ` [PATCH] x86/alternatives: Fix optimize_nops() checking Borislav Petkov
` (2 preceding siblings ...)
2018-01-10 18:12 ` tip-bot for Borislav Petkov
@ 2018-01-10 18:39 ` tip-bot for Borislav Petkov
2018-01-10 19:38 ` [PATCH] " Linus Torvalds
4 siblings, 0 replies; 44+ messages in thread
From: tip-bot for Borislav Petkov @ 2018-01-10 18:39 UTC (permalink / raw)
To: linux-tip-commits
Cc: ak, tglx, dave.hansen, andi, dwmw2, hpa, peterz, gregkh,
tim.c.chen, linux-kernel, jikos, mingo, bp, pjt, torvalds,
thomas.lendacky, luto
Commit-ID: 612e8e9350fd19cae6900cf36ea0c6892d1a0dca
Gitweb: https://git.kernel.org/tip/612e8e9350fd19cae6900cf36ea0c6892d1a0dca
Author: Borislav Petkov <bp@suse.de>
AuthorDate: Wed, 10 Jan 2018 12:28:16 +0100
Committer: Thomas Gleixner <tglx@linutronix.de>
CommitDate: Wed, 10 Jan 2018 19:36:22 +0100
x86/alternatives: Fix optimize_nops() checking
The alternatives code checks only the first byte whether it is a NOP, but
with NOPs in front of the payload and having actual instructions after it
breaks the "optimized' test.
Make sure to scan all bytes before deciding to optimize the NOPs in there.
Reported-by: David Woodhouse <dwmw2@infradead.org>
Signed-off-by: Borislav Petkov <bp@suse.de>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Tim Chen <tim.c.chen@linux.intel.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Jiri Kosina <jikos@kernel.org>
Cc: Dave Hansen <dave.hansen@intel.com>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: Andrew Lutomirski <luto@kernel.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Greg Kroah-Hartman <gregkh@linux-foundation.org>
Cc: Paul Turner <pjt@google.com>
Link: https://lkml.kernel.org/r/20180110112815.mgciyf5acwacphkq@pd.tnic
---
arch/x86/kernel/alternative.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index 3344d33..e0b97e4 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -344,9 +344,12 @@ done:
static void __init_or_module noinline optimize_nops(struct alt_instr *a, u8 *instr)
{
unsigned long flags;
+ int i;
- if (instr[0] != 0x90)
- return;
+ for (i = 0; i < a->padlen; i++) {
+ if (instr[i] != 0x90)
+ return;
+ }
local_irq_save(flags);
add_nops(instr + (a->instrlen - a->padlen), a->padlen);
^ permalink raw reply related [flat|nested] 44+ messages in thread
* Re: [PATCH] x86/alternatives: Fix optimize_nops() checking
2018-01-10 11:28 ` [PATCH] x86/alternatives: Fix optimize_nops() checking Borislav Petkov
` (3 preceding siblings ...)
2018-01-10 18:39 ` tip-bot for Borislav Petkov
@ 2018-01-10 19:38 ` Linus Torvalds
2018-01-10 19:55 ` Thomas Gleixner
2018-01-10 20:05 ` Borislav Petkov
4 siblings, 2 replies; 44+ messages in thread
From: Linus Torvalds @ 2018-01-10 19:38 UTC (permalink / raw)
To: Borislav Petkov
Cc: David Woodhouse, Andi Kleen, Thomas Gleixner,
the arch/x86 maintainers, Linux Kernel Mailing List, Paul Turner,
Andrew Lutomirski, Peter Zijlstra, Tom Lendacky, Tim Chen,
Greg Kroah-Hartman, Dave Hansen, Jiri Kosina, Andi Kleen
On Wed, Jan 10, 2018 at 3:28 AM, Borislav Petkov <bp@alien8.de> wrote:
>
> Make sure we scan all bytes before we decide to optimize the NOPs in
> there.
Can we also add compile-time checking (presumably in objtool, but who
knows) that there are no relocations in the alternative section?
Because that was the other "oops, this really doesn't work with
altinstructions" issue, wasn't it?
Linus
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH] x86/alternatives: Fix optimize_nops() checking
2018-01-10 19:38 ` [PATCH] " Linus Torvalds
@ 2018-01-10 19:55 ` Thomas Gleixner
2018-01-10 20:15 ` Josh Poimboeuf
2018-01-10 20:05 ` Borislav Petkov
1 sibling, 1 reply; 44+ messages in thread
From: Thomas Gleixner @ 2018-01-10 19:55 UTC (permalink / raw)
To: Linus Torvalds
Cc: Borislav Petkov, David Woodhouse, Andi Kleen,
the arch/x86 maintainers, Linux Kernel Mailing List, Paul Turner,
Andrew Lutomirski, Peter Zijlstra, Tom Lendacky, Tim Chen,
Greg Kroah-Hartman, Dave Hansen, Jiri Kosina, Andi Kleen,
Josh Poimboeuf
On Wed, 10 Jan 2018, Linus Torvalds wrote:
> On Wed, Jan 10, 2018 at 3:28 AM, Borislav Petkov <bp@alien8.de> wrote:
> >
> > Make sure we scan all bytes before we decide to optimize the NOPs in
> > there.
>
> Can we also add compile-time checking (presumably in objtool, but who
> knows) that there are no relocations in the alternative section?
Cc'ing the overlor^Haded objtool wizard
> Because that was the other "oops, this really doesn't work with
> altinstructions" issue, wasn't it?
Yes.
Thanks,
tglx
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH] x86/alternatives: Fix optimize_nops() checking
2018-01-10 19:55 ` Thomas Gleixner
@ 2018-01-10 20:15 ` Josh Poimboeuf
2018-01-10 20:19 ` David Woodhouse
2018-01-10 20:26 ` Linus Torvalds
0 siblings, 2 replies; 44+ messages in thread
From: Josh Poimboeuf @ 2018-01-10 20:15 UTC (permalink / raw)
To: Thomas Gleixner
Cc: Linus Torvalds, Borislav Petkov, David Woodhouse, Andi Kleen,
the arch/x86 maintainers, Linux Kernel Mailing List, Paul Turner,
Andrew Lutomirski, Peter Zijlstra, Tom Lendacky, Tim Chen,
Greg Kroah-Hartman, Dave Hansen, Jiri Kosina, Andi Kleen
On Wed, Jan 10, 2018 at 08:55:40PM +0100, Thomas Gleixner wrote:
> On Wed, 10 Jan 2018, Linus Torvalds wrote:
>
> > On Wed, Jan 10, 2018 at 3:28 AM, Borislav Petkov <bp@alien8.de> wrote:
> > >
> > > Make sure we scan all bytes before we decide to optimize the NOPs in
> > > there.
> >
> > Can we also add compile-time checking (presumably in objtool, but who
> > knows) that there are no relocations in the alternative section?
>
> Cc'ing the overlor^Haded objtool wizard
>
> > Because that was the other "oops, this really doesn't work with
> > altinstructions" issue, wasn't it?
I think .altinstruction relocations *do* work if they're for the first
instruction, and it's a jump or a call. There's some alternatives code
which adjusts the jump/call offset in that case, and there are some
users of alternatives who rely on that.
I think Boris had a patch floating around to add an instruction decoder
to alternatives, so you can do a call/jmp anywhere.
--
Josh
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH] x86/alternatives: Fix optimize_nops() checking
2018-01-10 20:15 ` Josh Poimboeuf
@ 2018-01-10 20:19 ` David Woodhouse
2018-01-10 20:26 ` Linus Torvalds
1 sibling, 0 replies; 44+ messages in thread
From: David Woodhouse @ 2018-01-10 20:19 UTC (permalink / raw)
To: Josh Poimboeuf, Thomas Gleixner
Cc: Linus Torvalds, Borislav Petkov, Andi Kleen,
the arch/x86 maintainers, Linux Kernel Mailing List, Paul Turner,
Andrew Lutomirski, Peter Zijlstra, Tom Lendacky, Tim Chen,
Greg Kroah-Hartman, Dave Hansen, Jiri Kosina, Andi Kleen
[-- Attachment #1: Type: text/plain, Size: 1629 bytes --]
On Wed, 2018-01-10 at 14:15 -0600, Josh Poimboeuf wrote:
> On Wed, Jan 10, 2018 at 08:55:40PM +0100, Thomas Gleixner wrote:
> > On Wed, 10 Jan 2018, Linus Torvalds wrote:
> >
> > > On Wed, Jan 10, 2018 at 3:28 AM, Borislav Petkov <bp@alien8.de> wrote:
> > > >
> > > > Make sure we scan all bytes before we decide to optimize the NOPs in
> > > > there.
> > >
> > > Can we also add compile-time checking (presumably in objtool, but who
> > > knows) that there are no relocations in the alternative section?
> >
> > Cc'ing the overlor^Haded objtool wizard
> >
> > > Because that was the other "oops, this really doesn't work with
> > > altinstructions" issue, wasn't it?
>
> I think .altinstruction relocations *do* work if they're for the first
> instruction, and it's a jump or a call. There's some alternatives code
> which adjusts the jump/call offset in that case, and there are some
> users of alternatives who rely on that.
>
> I think Boris had a patch floating around to add an instruction decoder
> to alternatives, so you can do a call/jmp anywhere.
Strictly speaking, it's not about the ELF relocs. Those get processed
in advance, and the altinstructions already have them applied.
What Borislav had was a patch to process the actual branch
instructions, then frob their targets by {&altinstr - &oldinstr} to
make them work again... except it only worked by chance for targets
*within* the altinstr.
On the whole I think we're better off not touching that right now for
fear that it will introduce new bugs. But yes, it *should* be fixed....
just not this week please ;)
[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5213 bytes --]
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH] x86/alternatives: Fix optimize_nops() checking
2018-01-10 20:15 ` Josh Poimboeuf
2018-01-10 20:19 ` David Woodhouse
@ 2018-01-10 20:26 ` Linus Torvalds
2018-01-10 20:33 ` Peter Zijlstra
` (2 more replies)
1 sibling, 3 replies; 44+ messages in thread
From: Linus Torvalds @ 2018-01-10 20:26 UTC (permalink / raw)
To: Josh Poimboeuf
Cc: Thomas Gleixner, Borislav Petkov, David Woodhouse, Andi Kleen,
the arch/x86 maintainers, Linux Kernel Mailing List, Paul Turner,
Andrew Lutomirski, Peter Zijlstra, Tom Lendacky, Tim Chen,
Greg Kroah-Hartman, Dave Hansen, Jiri Kosina, Andi Kleen
On Wed, Jan 10, 2018 at 12:15 PM, Josh Poimboeuf <jpoimboe@redhat.com> wrote:
>
> I think .altinstruction relocations *do* work if they're for the first
> instruction, and it's a jump or a call.
Yes - for the alternative that is in-line - not in the "altinstruction" section.
Because then the alternative is in the right spot at link-time already.
But the "altinstruction" section definitely should not have
relocations. I guess you could hack them up by hand by explicitly
trying to take the difference between the non-altinstruction and the
altinstruction into account, but it would be error-prone and fragile
as hell.
> I think Boris had a patch floating around to add an instruction decoder
> to alternatives, so you can do a call/jmp anywhere.
.. and no, we're not doing that. Christ.
People, we need to try to be *robust* here. That's doubly (triply!)
true of things like altinstructions where people - very much by design
- won't even *test* the alternatives very much, because very much by
design the altinstructions are only used on certain architectures or
in certain situations.
And we almost certainly don't actuially _need_ relocations. But we
need to protect against the "oops, I didn't realize" issue, exactly
because testing won't actually catch the odd cases.
Because we don't want to be in the situation where some random poor
user hits it because they have an old CPU that no developer has, and
then the relocation will basically do completely random things.
Imagine just how crazy that would be to debug. You'd be basically
executing insane code, and looking at the sources - or even the
binaries - it would _look_ completely sane.
Linus
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH] x86/alternatives: Fix optimize_nops() checking
2018-01-10 20:26 ` Linus Torvalds
@ 2018-01-10 20:33 ` Peter Zijlstra
2018-01-10 20:36 ` David Woodhouse
2018-01-10 20:49 ` Linus Torvalds
2018-01-10 20:55 ` Borislav Petkov
2018-01-10 20:55 ` Josh Poimboeuf
2 siblings, 2 replies; 44+ messages in thread
From: Peter Zijlstra @ 2018-01-10 20:33 UTC (permalink / raw)
To: Linus Torvalds
Cc: Josh Poimboeuf, Thomas Gleixner, Borislav Petkov,
David Woodhouse, Andi Kleen, the arch/x86 maintainers,
Linux Kernel Mailing List, Paul Turner, Andrew Lutomirski,
Tom Lendacky, Tim Chen, Greg Kroah-Hartman, Dave Hansen,
Jiri Kosina, Andi Kleen
On Wed, Jan 10, 2018 at 12:26:25PM -0800, Linus Torvalds wrote:
> Imagine just how crazy that would be to debug. You'd be basically
> executing insane code, and looking at the sources - or even the
> binaries - it would _look_ completely sane.
Been there done that.. we have too much self modifying code for that
not to have been needed.
Use gdb on /proc/kcore and disassemble self to see the _real_ code.
But yes, tricky stuff. Not arguing we need relocations in alternatives,
just saying debugging them (and static keys and others) is great fun.
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH] x86/alternatives: Fix optimize_nops() checking
2018-01-10 20:33 ` Peter Zijlstra
@ 2018-01-10 20:36 ` David Woodhouse
2018-01-10 20:49 ` Linus Torvalds
1 sibling, 0 replies; 44+ messages in thread
From: David Woodhouse @ 2018-01-10 20:36 UTC (permalink / raw)
To: Peter Zijlstra, Linus Torvalds
Cc: Josh Poimboeuf, Thomas Gleixner, Borislav Petkov, Andi Kleen,
the arch/x86 maintainers, Linux Kernel Mailing List, Paul Turner,
Andrew Lutomirski, Tom Lendacky, Tim Chen, Greg Kroah-Hartman,
Dave Hansen, Jiri Kosina, Andi Kleen
[-- Attachment #1: Type: text/plain, Size: 573 bytes --]
On Wed, 2018-01-10 at 21:33 +0100, Peter Zijlstra wrote:
> On Wed, Jan 10, 2018 at 12:26:25PM -0800, Linus Torvalds wrote:
> > Imagine just how crazy that would be to debug. You'd be basically
> > executing insane code, and looking at the sources - or even the
> > binaries - it would _look_ completely sane.
>
> Been there done that.. we have too much self modifying code for that
> not to have been needed.
>
> Use gdb on /proc/kcore and disassemble self to see the _real_ code.
Or qemu -d in_asm to see what the code *was* at the time the CPU hit
it... :)
[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5213 bytes --]
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH] x86/alternatives: Fix optimize_nops() checking
2018-01-10 20:33 ` Peter Zijlstra
2018-01-10 20:36 ` David Woodhouse
@ 2018-01-10 20:49 ` Linus Torvalds
1 sibling, 0 replies; 44+ messages in thread
From: Linus Torvalds @ 2018-01-10 20:49 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Josh Poimboeuf, Thomas Gleixner, Borislav Petkov,
David Woodhouse, Andi Kleen, the arch/x86 maintainers,
Linux Kernel Mailing List, Paul Turner, Andrew Lutomirski,
Tom Lendacky, Tim Chen, Greg Kroah-Hartman, Dave Hansen,
Jiri Kosina, Andi Kleen
On Wed, Jan 10, 2018 at 12:33 PM, Peter Zijlstra <peterz@infradead.org> wrote:
>
> Use gdb on /proc/kcore and disassemble self to see the _real_ code.
Yes, because that works so well when you get these random reports of
crazy behavior that simply doesn't ever trigger on any machine you
have.
> But yes, tricky stuff. Not arguing we need relocations in alternatives,
> just saying debugging them (and static keys and others) is great fun.
Yes. It's definitely a good challenge, and can be _very_ satisfying
when you then finally figure it out.
I'm not sure it's worth the pain before that point, though.
So I think we should strive to make our alternatives code just catch
some of the more subtle errors early instead.
Linus
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH] x86/alternatives: Fix optimize_nops() checking
2018-01-10 20:26 ` Linus Torvalds
2018-01-10 20:33 ` Peter Zijlstra
@ 2018-01-10 20:55 ` Borislav Petkov
2018-01-10 21:05 ` Linus Torvalds
2018-01-10 20:55 ` Josh Poimboeuf
2 siblings, 1 reply; 44+ messages in thread
From: Borislav Petkov @ 2018-01-10 20:55 UTC (permalink / raw)
To: Linus Torvalds
Cc: Josh Poimboeuf, Thomas Gleixner, David Woodhouse, Andi Kleen,
the arch/x86 maintainers, Linux Kernel Mailing List, Paul Turner,
Andrew Lutomirski, Peter Zijlstra, Tom Lendacky, Tim Chen,
Greg Kroah-Hartman, Dave Hansen, Jiri Kosina, Andi Kleen
On Wed, Jan 10, 2018 at 12:26:25PM -0800, Linus Torvalds wrote:
> > I think Boris had a patch floating around to add an instruction decoder
> > to alternatives, so you can do a call/jmp anywhere.
>
> .. and no, we're not doing that. Christ.
>
> People, we need to try to be *robust* here. That's doubly (triply!)
> true of things like altinstructions where people - very much by design
> - won't even *test* the alternatives very much, because very much by
> design the altinstructions are only used on certain architectures or
> in certain situations.
Ok, so the problem was: how to fixup jumps which are not the first
instruction which is being replaced but a following one in the
instruction bytes with which we replace.
I used the insn decoder to get insn boundaries so that I can know
whether bytes 0xeb or 0xe9 are the actual JMP opcode and not some other
bytes from the stream.
So how do you suggest I do that without the decoder? I still need some
sort of parsing to find out where the boundaries are...
Thx.
--
Regards/Gruss,
Boris.
Good mailing practices for 400: avoid top-posting and trim the reply.
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH] x86/alternatives: Fix optimize_nops() checking
2018-01-10 20:55 ` Borislav Petkov
@ 2018-01-10 21:05 ` Linus Torvalds
2018-01-10 21:08 ` David Woodhouse
2018-01-10 21:10 ` Borislav Petkov
0 siblings, 2 replies; 44+ messages in thread
From: Linus Torvalds @ 2018-01-10 21:05 UTC (permalink / raw)
To: Borislav Petkov
Cc: Josh Poimboeuf, Thomas Gleixner, David Woodhouse, Andi Kleen,
the arch/x86 maintainers, Linux Kernel Mailing List, Paul Turner,
Andrew Lutomirski, Peter Zijlstra, Tom Lendacky, Tim Chen,
Greg Kroah-Hartman, Dave Hansen, Jiri Kosina, Andi Kleen
On Wed, Jan 10, 2018 at 12:55 PM, Borislav Petkov <bp@alien8.de> wrote:
>
> Ok, so the problem was: how to fixup jumps which are not the first
> instruction which is being replaced but a following one in the
> instruction bytes with which we replace.
What jumps do you have that need to be fixed up?
I really think we should avoid having things like that.
Any jumps *within* the alternatives should have been handled by the
assembler already.
And jumps between the alternatives and other places? Why do they exist?
Linus
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH] x86/alternatives: Fix optimize_nops() checking
2018-01-10 21:05 ` Linus Torvalds
@ 2018-01-10 21:08 ` David Woodhouse
2018-01-10 21:11 ` Linus Torvalds
2018-01-10 21:10 ` Borislav Petkov
1 sibling, 1 reply; 44+ messages in thread
From: David Woodhouse @ 2018-01-10 21:08 UTC (permalink / raw)
To: Linus Torvalds, Borislav Petkov
Cc: Josh Poimboeuf, Thomas Gleixner, Andi Kleen,
the arch/x86 maintainers, Linux Kernel Mailing List, Paul Turner,
Andrew Lutomirski, Peter Zijlstra, Tom Lendacky, Tim Chen,
Greg Kroah-Hartman, Dave Hansen, Jiri Kosina, Andi Kleen
[-- Attachment #1: Type: text/plain, Size: 1100 bytes --]
On Wed, 2018-01-10 at 13:05 -0800, Linus Torvalds wrote:
> On Wed, Jan 10, 2018 at 12:55 PM, Borislav Petkov <bp@alien8.de>
> wrote:
> >
> > Ok, so the problem was: how to fixup jumps which are not the first
> > instruction which is being replaced but a following one in the
> > instruction bytes with which we replace.
>
> What jumps do you have that need to be fixed up?
>
> I really think we should avoid having things like that.
>
> Any jumps *within* the alternatives should have been handled by the
> assembler already.
>
> And jumps between the alternatives and other places? Why do they
> exist?
There are a few of the form 'call *somefunc'.
The existing code handles them not by virtue of the relocs, as I said,
but by a simple delta of the old and new location of the instruction.
But it only does so for the *first* instruction of the altinstr, if it
happens to be a (4-byte?) branch.
Right now for retpoline I am just studiously avoiding doing anything
that the alternatives mechanism isn't going to get right, or might
change in future. I think ;)
[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5213 bytes --]
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH] x86/alternatives: Fix optimize_nops() checking
2018-01-10 21:08 ` David Woodhouse
@ 2018-01-10 21:11 ` Linus Torvalds
2018-01-10 21:16 ` Josh Poimboeuf
2018-01-10 21:17 ` Linus Torvalds
0 siblings, 2 replies; 44+ messages in thread
From: Linus Torvalds @ 2018-01-10 21:11 UTC (permalink / raw)
To: David Woodhouse
Cc: Borislav Petkov, Josh Poimboeuf, Thomas Gleixner, Andi Kleen,
the arch/x86 maintainers, Linux Kernel Mailing List, Paul Turner,
Andrew Lutomirski, Peter Zijlstra, Tom Lendacky, Tim Chen,
Greg Kroah-Hartman, Dave Hansen, Jiri Kosina, Andi Kleen
On Wed, Jan 10, 2018 at 1:08 PM, David Woodhouse <dwmw2@infradead.org> wrote:
>
> There are a few of the form 'call *somefunc'.
>
> The existing code handles them not by virtue of the relocs, as I said,
> but by a simple delta of the old and new location of the instruction.
>
> But it only does so for the *first* instruction of the altinstr, if it
> happens to be a (4-byte?) branch.
Ugh. That's nasty.
Wouldn't it be much better to simply do it as part of relocation instead?
Linus
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH] x86/alternatives: Fix optimize_nops() checking
2018-01-10 21:11 ` Linus Torvalds
@ 2018-01-10 21:16 ` Josh Poimboeuf
2018-01-10 21:17 ` Linus Torvalds
1 sibling, 0 replies; 44+ messages in thread
From: Josh Poimboeuf @ 2018-01-10 21:16 UTC (permalink / raw)
To: Linus Torvalds
Cc: David Woodhouse, Borislav Petkov, Thomas Gleixner, Andi Kleen,
the arch/x86 maintainers, Linux Kernel Mailing List, Paul Turner,
Andrew Lutomirski, Peter Zijlstra, Tom Lendacky, Tim Chen,
Greg Kroah-Hartman, Dave Hansen, Jiri Kosina, Andi Kleen
On Wed, Jan 10, 2018 at 01:11:48PM -0800, Linus Torvalds wrote:
> On Wed, Jan 10, 2018 at 1:08 PM, David Woodhouse <dwmw2@infradead.org> wrote:
> >
> > There are a few of the form 'call *somefunc'.
> >
> > The existing code handles them not by virtue of the relocs, as I said,
> > but by a simple delta of the old and new location of the instruction.
> >
> > But it only does so for the *first* instruction of the altinstr, if it
> > happens to be a (4-byte?) branch.
>
> Ugh. That's nasty.
>
> Wouldn't it be much better to simply do it as part of relocation instead?
Not possible, the relocations are applied during vmlinux linking.
--
Josh
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH] x86/alternatives: Fix optimize_nops() checking
2018-01-10 21:11 ` Linus Torvalds
2018-01-10 21:16 ` Josh Poimboeuf
@ 2018-01-10 21:17 ` Linus Torvalds
2018-01-10 21:27 ` Josh Poimboeuf
1 sibling, 1 reply; 44+ messages in thread
From: Linus Torvalds @ 2018-01-10 21:17 UTC (permalink / raw)
To: David Woodhouse
Cc: Borislav Petkov, Josh Poimboeuf, Thomas Gleixner, Andi Kleen,
the arch/x86 maintainers, Linux Kernel Mailing List, Paul Turner,
Andrew Lutomirski, Peter Zijlstra, Tom Lendacky, Tim Chen,
Greg Kroah-Hartman, Dave Hansen, Jiri Kosina, Andi Kleen
On Wed, Jan 10, 2018 at 1:11 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> Wouldn't it be much better to simply do it as part of relocation instead?
.. except we only do real relocation for modules, and depend on the
linker doing everything for us (handle_relocations() at load-time) I
think.
So it's somewhat more involved surgery. Which explains the hack.
Nasty.
Linus
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH] x86/alternatives: Fix optimize_nops() checking
2018-01-10 21:17 ` Linus Torvalds
@ 2018-01-10 21:27 ` Josh Poimboeuf
0 siblings, 0 replies; 44+ messages in thread
From: Josh Poimboeuf @ 2018-01-10 21:27 UTC (permalink / raw)
To: Linus Torvalds
Cc: David Woodhouse, Borislav Petkov, Thomas Gleixner, Andi Kleen,
the arch/x86 maintainers, Linux Kernel Mailing List, Paul Turner,
Andrew Lutomirski, Peter Zijlstra, Tom Lendacky, Tim Chen,
Greg Kroah-Hartman, Dave Hansen, Jiri Kosina, Andi Kleen
On Wed, Jan 10, 2018 at 01:17:45PM -0800, Linus Torvalds wrote:
> On Wed, Jan 10, 2018 at 1:11 PM, Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
> >
> > Wouldn't it be much better to simply do it as part of relocation instead?
>
> .. except we only do real relocation for modules, and depend on the
> linker doing everything for us (handle_relocations() at load-time) I
> think.
>
> So it's somewhat more involved surgery. Which explains the hack.
>
> Nasty.
Right. With KASLR, the relocations seem to be resolved by
handle_relocations(), but without KASLR, they're resolved in vmlinux
linking.
--
Josh
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH] x86/alternatives: Fix optimize_nops() checking
2018-01-10 21:05 ` Linus Torvalds
2018-01-10 21:08 ` David Woodhouse
@ 2018-01-10 21:10 ` Borislav Petkov
1 sibling, 0 replies; 44+ messages in thread
From: Borislav Petkov @ 2018-01-10 21:10 UTC (permalink / raw)
To: Linus Torvalds
Cc: Josh Poimboeuf, Thomas Gleixner, David Woodhouse, Andi Kleen,
the arch/x86 maintainers, Linux Kernel Mailing List, Paul Turner,
Andrew Lutomirski, Peter Zijlstra, Tom Lendacky, Tim Chen,
Greg Kroah-Hartman, Dave Hansen, Jiri Kosina, Andi Kleen
On Wed, Jan 10, 2018 at 01:05:08PM -0800, Linus Torvalds wrote:
> Any jumps *within* the alternatives should have been handled by the
> assembler already.
>
> And jumps between the alternatives and other places? Why do they exist?
Yap, I say so too, the alternatives handle everything just fine but then
Woodhouse appeared... :-)
--
Regards/Gruss,
Boris.
Good mailing practices for 400: avoid top-posting and trim the reply.
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH] x86/alternatives: Fix optimize_nops() checking
2018-01-10 20:26 ` Linus Torvalds
2018-01-10 20:33 ` Peter Zijlstra
2018-01-10 20:55 ` Borislav Petkov
@ 2018-01-10 20:55 ` Josh Poimboeuf
2 siblings, 0 replies; 44+ messages in thread
From: Josh Poimboeuf @ 2018-01-10 20:55 UTC (permalink / raw)
To: Linus Torvalds
Cc: Thomas Gleixner, Borislav Petkov, David Woodhouse, Andi Kleen,
the arch/x86 maintainers, Linux Kernel Mailing List, Paul Turner,
Andrew Lutomirski, Peter Zijlstra, Tom Lendacky, Tim Chen,
Greg Kroah-Hartman, Dave Hansen, Jiri Kosina, Andi Kleen
On Wed, Jan 10, 2018 at 12:26:25PM -0800, Linus Torvalds wrote:
> On Wed, Jan 10, 2018 at 12:15 PM, Josh Poimboeuf <jpoimboe@redhat.com> wrote:
> >
> > I think .altinstruction relocations *do* work if they're for the first
> > instruction, and it's a jump or a call.
>
> Yes - for the alternative that is in-line - not in the "altinstruction" section.
>
> Because then the alternative is in the right spot at link-time already.
>
> But the "altinstruction" section definitely should not have
> relocations.
I misspoke, it's really .altinstr_replacement which has the replacement
instructions. And it has a bunch of relocations:
Relocation section [ 8] '.rela.altinstr_replacement' for section [ 7] '.altinstr_replacement' at offset 0x14439710 contains 355 entries:
> I guess you could hack them up by hand by explicitly
> trying to take the difference between the non-altinstruction and the
> altinstruction into account, but it would be error-prone and fragile
> as hell.
apply_alternatives() already does that today. It actually seems pretty
solid, except for the whole "only works on the first instruction" thing.
> > I think Boris had a patch floating around to add an instruction decoder
> > to alternatives, so you can do a call/jmp anywhere.
>
> .. and no, we're not doing that. Christ.
>
> People, we need to try to be *robust* here. That's doubly (triply!)
> true of things like altinstructions where people - very much by design
> - won't even *test* the alternatives very much, because very much by
> design the altinstructions are only used on certain architectures or
> in certain situations.
>
> And we almost certainly don't actuially _need_ relocations. But we
> need to protect against the "oops, I didn't realize" issue, exactly
> because testing won't actually catch the odd cases.
If we need objtool to detect them, it's certainly possible. But maybe I
missed the previous discussion -- what's the, um, alternative to
relocations, when we have calls and jumps being patched in?
> Because we don't want to be in the situation where some random poor
> user hits it because they have an old CPU that no developer has, and
> then the relocation will basically do completely random things.
>
> Imagine just how crazy that would be to debug. You'd be basically
> executing insane code, and looking at the sources - or even the
> binaries - it would _look_ completely sane.
Well, I think we already made that deal with the devil when we added
alternatives/paravirt/smp_locks/jump_labels/kprobes/ftrace/bpf, etc.
--
Josh
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH] x86/alternatives: Fix optimize_nops() checking
2018-01-10 19:38 ` [PATCH] " Linus Torvalds
2018-01-10 19:55 ` Thomas Gleixner
@ 2018-01-10 20:05 ` Borislav Petkov
2018-01-10 20:20 ` Linus Torvalds
1 sibling, 1 reply; 44+ messages in thread
From: Borislav Petkov @ 2018-01-10 20:05 UTC (permalink / raw)
To: Linus Torvalds
Cc: David Woodhouse, Andi Kleen, Thomas Gleixner,
the arch/x86 maintainers, Linux Kernel Mailing List, Paul Turner,
Andrew Lutomirski, Peter Zijlstra, Tom Lendacky, Tim Chen,
Greg Kroah-Hartman, Dave Hansen, Jiri Kosina, Andi Kleen
On Wed, Jan 10, 2018 at 11:38:36AM -0800, Linus Torvalds wrote:
> Can we also add compile-time checking (presumably in objtool, but who
> knows) that there are no relocations in the alternative section?
Well, so we do fail the build if the jmp's offset doesn't fit in s32:
Makefile:996: recipe for target 'vmlinux' failed
arch/x86/kernel/cpu/amd.o:(.altinstr_replacement+0x4): relocation truncated to fit: R_X86_64_32 against `.altinstr_replacement'
make: *** [vmlinux] Error 1
I did this:
alternative("", "xor %%rdi, %%rdi; .byte 0xe9; .long 2f; 2: jmp startup_64", X86_FEATURE_K8);
And I was thinking whether it would make sense to beef up
recompute_jump() to be smart enough to massage the JMPs into jumping to
the right place.
Because in the example above, I'm basically jumping to the label 2: but
since the .altinstr_replacement is so far away, it fails the build due
to the reloc.
Even though it shouldn't have because when we patch, that label is
immediately following the JMP. And probably it wouldn't need a reloc
either. Whether that sequence makes sense is another story...
Anyway, I asked a gcc person and we'll see what we could do.
Thx.
--
Regards/Gruss,
Boris.
Good mailing practices for 400: avoid top-posting and trim the reply.
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH] x86/alternatives: Fix optimize_nops() checking
2018-01-10 20:05 ` Borislav Petkov
@ 2018-01-10 20:20 ` Linus Torvalds
2018-01-10 20:50 ` Borislav Petkov
0 siblings, 1 reply; 44+ messages in thread
From: Linus Torvalds @ 2018-01-10 20:20 UTC (permalink / raw)
To: Borislav Petkov
Cc: David Woodhouse, Andi Kleen, Thomas Gleixner,
the arch/x86 maintainers, Linux Kernel Mailing List, Paul Turner,
Andrew Lutomirski, Peter Zijlstra, Tom Lendacky, Tim Chen,
Greg Kroah-Hartman, Dave Hansen, Jiri Kosina, Andi Kleen
On Wed, Jan 10, 2018 at 12:05 PM, Borislav Petkov <bp@alien8.de> wrote:
>
> I did this:
>
> alternative("", "xor %%rdi, %%rdi; .byte 0xe9; .long 2f; 2: jmp startup_64", X86_FEATURE_K8);
No, that's not valid. That could never work anyway. The ".long 2f"
would be the absolute address in the alternative section, but opcode
E9 takes a relative 32-bit offset.
So the error you get isn't because relocations wouldn't work in
alternatives, it's because you tried to fit an absolute 64-biy value
in a 32-bit relocation, and it was wrong _regardless_ of which section
you were in.
Linus
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH] x86/alternatives: Fix optimize_nops() checking
2018-01-10 20:20 ` Linus Torvalds
@ 2018-01-10 20:50 ` Borislav Petkov
0 siblings, 0 replies; 44+ messages in thread
From: Borislav Petkov @ 2018-01-10 20:50 UTC (permalink / raw)
To: Linus Torvalds
Cc: David Woodhouse, Andi Kleen, Thomas Gleixner,
the arch/x86 maintainers, Linux Kernel Mailing List, Paul Turner,
Andrew Lutomirski, Peter Zijlstra, Tom Lendacky, Tim Chen,
Greg Kroah-Hartman, Dave Hansen, Jiri Kosina, Andi Kleen
On Wed, Jan 10, 2018 at 12:20:40PM -0800, Linus Torvalds wrote:
> No, that's not valid. That could never work anyway. The ".long 2f"
> would be the absolute address in the alternative section, but opcode
> E9 takes a relative 32-bit offset.
Ah, right, doh. I remember now. We used to do those jmps by computing the
relative offset:
From: 090a3f615524c3f75d09fdb37f15ea1868d79f7e
- .section .altinstr_replacement,"ax"
-1: .byte 0xeb /* jmp <disp8> */
- .byte (copy_page_rep - copy_page) - (2f - 1b) /* offset */
--
Regards/Gruss,
Boris.
Good mailing practices for 400: avoid top-posting and trim the reply.
^ permalink raw reply [flat|nested] 44+ messages in thread