Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753142AbbDGJlC (ORCPT ); Tue, 7 Apr 2015 05:41:02 -0400 Received: from terminus.zytor.com ([198.137.202.10]:48194 "EHLO terminus.zytor.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750993AbbDGJk7 (ORCPT ); Tue, 7 Apr 2015 05:40:59 -0400 Date: Tue, 7 Apr 2015 02:40:12 -0700 From: tip-bot for Borislav Petkov Message-ID: Cc: quentin.casasnovas@oracle.com, dvlasenk@redhat.com, bp@alien8.de, bp@suse.de, torvalds@linux-foundation.org, luto@amacapital.net, mingo@kernel.org, hpa@zytor.com, linux-kernel@vger.kernel.org, oleg@redhat.com, brgerst@gmail.com, tglx@linutronix.de Reply-To: hpa@zytor.com, tglx@linutronix.de, oleg@redhat.com, brgerst@gmail.com, linux-kernel@vger.kernel.org, bp@suse.de, dvlasenk@redhat.com, bp@alien8.de, quentin.casasnovas@oracle.com, mingo@kernel.org, torvalds@linux-foundation.org, luto@amacapital.net In-Reply-To: <20150404133443.GE21152@pd.tnic> References: <20150404133443.GE21152@pd.tnic> To: linux-tip-commits@vger.kernel.org Subject: [tip:x86/asm] x86/alternatives: Fix ALTERNATIVE_2 padding generation properly Git-Commit-ID: dbe4058a6a44af4ca5d146aebe01b0a1f9b7fd2a X-Mailer: tip-git-log-daemon Robot-ID: Robot-Unsubscribe: Contact to get blacklisted from these emails MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Content-Type: text/plain; charset=UTF-8 Content-Disposition: inline Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5763 Lines: 150 Commit-ID: dbe4058a6a44af4ca5d146aebe01b0a1f9b7fd2a Gitweb: http://git.kernel.org/tip/dbe4058a6a44af4ca5d146aebe01b0a1f9b7fd2a Author: Borislav Petkov AuthorDate: Sat, 4 Apr 2015 15:34:43 +0200 Committer: Ingo Molnar CommitDate: Sat, 4 Apr 2015 15:58:23 +0200 x86/alternatives: Fix ALTERNATIVE_2 padding generation properly 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! Reported-by: Quentin Casasnovas Signed-off-by: Borislav Petkov Cc: Andy Lutomirski Cc: Borislav Petkov Cc: Brian Gerst Cc: Denys Vlasenko Cc: H. Peter Anvin Cc: Linus Torvalds Cc: Oleg Nesterov Cc: Thomas Gleixner Link: http://lkml.kernel.org/r/20150404133443.GE21152@pd.tnic Signed-off-by: Ingo Molnar --- 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 524bddc..bdf02ee 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 5aef6a9..ba32af0 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 5c993c9..7c4ad00 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); -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/