From: Borislav Petkov <bp@alien8.de>
To: Ingo Molnar <mingo@kernel.org>
Cc: Quentin Casasnovas <quentin.casasnovas@oracle.com>,
X86 ML <x86@kernel.org>, LKML <linux-kernel@vger.kernel.org>,
"H. Peter Anvin" <hpa@zytor.com>, Ingo Molnar <mingo@kernel.org>,
Thomas Gleixner <tglx@linutronix.de>,
Oleg Nesterov <oleg@redhat.com>,
Andy Lutomirski <luto@amacapital.net>
Subject: [PATCH] x86/alternatives: Fix ALTERNATIVE_2 padding generation properly
Date: Sat, 4 Apr 2015 15:34:43 +0200 [thread overview]
Message-ID: <20150404133443.GE21152@pd.tnic> (raw)
In-Reply-To: <20150404133243.GD21152@pd.tnic>
From: Borislav Petkov <bp@suse.de>
Quentin caught a corner case with the generation of instruction padding
in the ALTERNATIVE_2 macro: if len(orig_insn) < len(alt1) < len(alt2),
then not enough padding gets added and that is not good(tm) as we could
overwrite the beginning of the next instruction.
Luckily, at the time of this writing, we don't have ALTERNATIVE_2()
invocations which have that problem and even if we did, a simple fix
would be to prepend the instructions with enough prefixes so that that
corner case doesn't happen.
However, best it would be if we fixed it properly. See below for a
simple, abstracted example of what we're doing.
So what we ended up doing is, we compute the
max(len(alt1), len(alt2)) - len(orig_insn)
and feed that value to the .skip gas directive. The max() cannot have
conditionals due to gas limitations, thus the fancy integer math.
With this patch, all ALTERNATIVE_2 sites get padded correctly; generating
obscure test cases pass too:
#define alt_max_short(a, b) ((a) ^ (((a) ^ (b)) & -(-((a) < (b)))))
#define gen_skip(orig, alt1, alt2, marker) \
.skip -((alt_max_short(alt1, alt2) - (orig)) > 0) * \
(alt_max_short(alt1, alt2) - (orig)),marker
.pushsection .text, "ax"
.globl main
main:
gen_skip(1, 2, 4, 0x09)
gen_skip(4, 1, 2, 0x10)
...
.popsection
Thanks to Quentin for catching it and double-checking the fix!
Signed-off-by: Borislav Petkov <bp@suse.de>
Reported-by: Quentin Casasnovas <quentin.casasnovas@oracle.com>
---
arch/x86/include/asm/alternative-asm.h | 14 ++++++++++++--
arch/x86/include/asm/alternative.h | 16 ++++++++++++----
arch/x86/kernel/alternative.c | 4 ++--
3 files changed, 26 insertions(+), 8 deletions(-)
diff --git a/arch/x86/include/asm/alternative-asm.h b/arch/x86/include/asm/alternative-asm.h
index 524bddce0b76..bdf02eeee765 100644
--- a/arch/x86/include/asm/alternative-asm.h
+++ b/arch/x86/include/asm/alternative-asm.h
@@ -45,12 +45,22 @@
.popsection
.endm
+#define old_len 141b-140b
+#define new_len1 144f-143f
+#define new_len2 145f-144f
+
+/*
+ * max without conditionals. Idea adapted from:
+ * http://graphics.stanford.edu/~seander/bithacks.html#IntegerMinOrMax
+ */
+#define alt_max_short(a, b) ((a) ^ (((a) ^ (b)) & -(-((a) < (b)))))
+
.macro ALTERNATIVE_2 oldinstr, newinstr1, feature1, newinstr2, feature2
140:
\oldinstr
141:
- .skip -(((144f-143f)-(141b-140b)) > 0) * ((144f-143f)-(141b-140b)),0x90
- .skip -(((145f-144f)-(144f-143f)-(141b-140b)) > 0) * ((145f-144f)-(144f-143f)-(141b-140b)),0x90
+ .skip -((alt_max_short(new_len1, new_len2) - (old_len)) > 0) * \
+ (alt_max_short(new_len1, new_len2) - (old_len)),0x90
142:
.pushsection .altinstructions,"a"
diff --git a/arch/x86/include/asm/alternative.h b/arch/x86/include/asm/alternative.h
index 5aef6a97d80e..ba32af062f61 100644
--- a/arch/x86/include/asm/alternative.h
+++ b/arch/x86/include/asm/alternative.h
@@ -96,13 +96,21 @@ static inline int alternatives_text_reserved(void *start, void *end)
alt_end_marker ":\n"
/*
+ * max without conditionals. Idea adapted from:
+ * http://graphics.stanford.edu/~seander/bithacks.html#IntegerMinOrMax
+ *
+ * The additional "-" is needed because gas works with s32s.
+ */
+#define alt_max_short(a, b) "((" a ") ^ (((" a ") ^ (" b ")) & -(-((" a ") - (" b ")))))"
+
+/*
* Pad the second replacement alternative with additional NOPs if it is
* additionally longer than the first replacement alternative.
*/
-#define OLDINSTR_2(oldinstr, num1, num2) \
- __OLDINSTR(oldinstr, num1) \
- ".skip -(((" alt_rlen(num2) ")-(" alt_rlen(num1) ")-(662b-661b)) > 0) * " \
- "((" alt_rlen(num2) ")-(" alt_rlen(num1) ")-(662b-661b)),0x90\n" \
+#define OLDINSTR_2(oldinstr, num1, num2) \
+ "661:\n\t" oldinstr "\n662:\n" \
+ ".skip -((" alt_max_short(alt_rlen(num1), alt_rlen(num2)) " - (" alt_slen ")) > 0) * " \
+ "(" alt_max_short(alt_rlen(num1), alt_rlen(num2)) " - (" alt_slen ")), 0x90\n" \
alt_end_marker ":\n"
#define ALTINSTR_ENTRY(feature, num) \
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index 5c993c94255e..7c4ad005d7a0 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -369,11 +369,11 @@ void __init_or_module apply_alternatives(struct alt_instr *start,
continue;
}
- DPRINTK("feat: %d*32+%d, old: (%p, len: %d), repl: (%p, len: %d)",
+ DPRINTK("feat: %d*32+%d, old: (%p, len: %d), repl: (%p, len: %d), pad: %d",
a->cpuid >> 5,
a->cpuid & 0x1f,
instr, a->instrlen,
- replacement, a->replacementlen);
+ replacement, a->replacementlen, a->padlen);
DUMP_BYTES(instr, a->instrlen, "%p: old_insn: ", instr);
DUMP_BYTES(replacement, a->replacementlen, "%p: rpl_insn: ", replacement);
--
2.3.3
--
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
--
next prev parent reply other threads:[~2015-04-04 13:36 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-04-02 13:11 [PATCH] x86/xsave: Robustify and merge macros Borislav Petkov
2015-04-02 15:52 ` Quentin Casasnovas
2015-04-02 16:12 ` Borislav Petkov
2015-04-02 16:33 ` Quentin Casasnovas
2015-04-02 16:45 ` Borislav Petkov
2015-04-03 14:06 ` Quentin Casasnovas
2015-04-03 14:14 ` Quentin Casasnovas
2015-04-03 15:23 ` Borislav Petkov
2015-04-03 15:40 ` Quentin Casasnovas
2015-04-03 17:06 ` Borislav Petkov
2015-04-03 17:33 ` Quentin Casasnovas
2015-04-03 17:48 ` Borislav Petkov
2015-04-03 20:42 ` Quentin Casasnovas
2015-04-04 7:34 ` Borislav Petkov
2015-04-04 8:36 ` Quentin Casasnovas
2015-04-04 9:25 ` Borislav Petkov
2015-04-04 10:11 ` Quentin Casasnovas
2015-04-04 10:29 ` Borislav Petkov
2015-04-04 13:32 ` Borislav Petkov
2015-04-04 13:34 ` Borislav Petkov [this message]
2015-04-07 9:27 ` [PATCH] x86/alternatives: Fix ALTERNATIVE_2 padding generation properly Quentin Casasnovas
2015-04-07 9:40 ` [tip:x86/asm] " tip-bot for Borislav Petkov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20150404133443.GE21152@pd.tnic \
--to=bp@alien8.de \
--cc=hpa@zytor.com \
--cc=linux-kernel@vger.kernel.org \
--cc=luto@amacapital.net \
--cc=mingo@kernel.org \
--cc=oleg@redhat.com \
--cc=quentin.casasnovas@oracle.com \
--cc=tglx@linutronix.de \
--cc=x86@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).