| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <tip-6d01f28bc08814abc7b8f8973cc71195537f4c80@git.kernel.org>
Date: Fri, 29 Jul 2016 08:14:23 -0700
From: tip-bot for Josh Poimboeuf <tipbot@...or.com>
To: linux-tip-commits@...r.kernel.org
Cc: jpoimboe@...hat.com, brgerst@...il.com, mingo@...nel.org,
hpa@...or.com, dvlasenk@...hat.com, bp@...en8.de,
linux-kernel@...r.kernel.org, torvalds@...ux-foundation.org,
peterz@...radead.org, luto@...nel.org, tglx@...utronix.de
Subject: [tip:core/urgent] objtool: Support new GCC 6 switch jump table
pattern
Commit-ID: 6d01f28bc08814abc7b8f8973cc71195537f4c80
Gitweb: http://git.kernel.org/tip/6d01f28bc08814abc7b8f8973cc71195537f4c80
Author: Josh Poimboeuf <jpoimboe@...hat.com>
AuthorDate: Thu, 28 Jul 2016 19:14:58 -0500
Committer: Ingo Molnar <mingo@...nel.org>
CommitDate: Fri, 29 Jul 2016 16:56:21 +0200
objtool: Support new GCC 6 switch jump table pattern
This fixes some false positive objtool warnings seen with gcc 6.1.1:
kernel/trace/ring_buffer.o: warning: objtool: ring_buffer_read_page()+0x36c: sibling call from callable instruction with changed frame pointer
arch/x86/kernel/reboot.o: warning: objtool: native_machine_emergency_restart()+0x139: sibling call from callable instruction with changed frame pointer
lib/xz/xz_dec_stream.o: warning: objtool: xz_dec_run()+0xc2: sibling call from callable instruction with changed frame pointer
With GCC 6, a new code pattern is sometimes used to access a switch
statement jump table in .rodata, which objtool doesn't yet recognize:
mov [rodata addr],%reg1
... some instructions ...
jmpq *(%reg1,%reg2,8)
Add support for detecting that pattern. The detection code is rather
crude, but it's still effective at weeding out false positives and
catching real warnings. It can be refined later once objtool starts
reading DWARF CFI.
Reported-by: Linus Torvalds <torvalds@...ux-foundation.org>
Signed-off-by: Josh Poimboeuf <jpoimboe@...hat.com>
Cc: Andy Lutomirski <luto@...nel.org>
Cc: Borislav Petkov <bp@...en8.de>
Cc: Brian Gerst <brgerst@...il.com>
Cc: Denys Vlasenko <dvlasenk@...hat.com>
Cc: H. Peter Anvin <hpa@...or.com>
Cc: Peter Zijlstra <peterz@...radead.org>
Cc: Thomas Gleixner <tglx@...utronix.de>
Link: http://lkml.kernel.org/r/b8c9503b4ad8c8a827cc5400db4c1b40a3ea07bc.1469751119.git.jpoimboe@redhat.com
Signed-off-by: Ingo Molnar <mingo@...nel.org>
---
tools/objtool/builtin-check.c | 140 ++++++++++++++++++++++++++----------------
1 file changed, 88 insertions(+), 52 deletions(-)
diff --git a/tools/objtool/builtin-check.c b/tools/objtool/builtin-check.c
index 17fa7fc..bd09d0e 100644
--- a/tools/objtool/builtin-check.c
+++ b/tools/objtool/builtin-check.c
@@ -107,6 +107,12 @@ static struct instruction *next_insn_same_sec(struct objtool_file *file,
insn->offset < func->offset + func->len; \
insn = list_next_entry(insn, list))
+#define func_for_each_insn_continue_reverse(file, func, insn) \
+ for (insn = list_prev_entry(insn, list); \
+ &insn->list != &file->insn_list && \
+ insn->sec == func->sec && insn->offset >= func->offset; \
+ insn = list_prev_entry(insn, list))
+
#define sec_for_each_insn_from(file, insn) \
for (; insn; insn = next_insn_same_sec(file, insn))
@@ -664,65 +670,95 @@ static int add_switch_table(struct objtool_file *file, struct symbol *func,
return 0;
}
-static int add_func_switch_tables(struct objtool_file *file,
- struct symbol *func)
+/*
+ * find_switch_table() - Given a dynamic jump, find the switch jump table in
+ * .rodata associated with it.
+ *
+ * There are 3 basic patterns:
+ *
+ * 1. jmpq *[rodata addr](,%reg,8)
+ *
+ * This is the most common case by far. It jumps to an address in a simple
+ * jump table which is stored in .rodata.
+ *
+ * 2. jmpq *[rodata addr](%rip)
+ *
+ * This is caused by a rare GCC quirk, currently only seen in three driver
+ * functions in the kernel, only with certain obscure non-distro configs.
+ *
+ * As part of an optimization, GCC makes a copy of an existing switch jump
+ * table, modifies it, and then hard-codes the jump (albeit with an indirect
+ * jump) to use a single entry in the table. The rest of the jump table and
+ * some of its jump targets remain as dead code.
+ *
+ * In such a case we can just crudely ignore all unreachable instruction
+ * warnings for the entire object file. Ideally we would just ignore them
+ * for the function, but that would require redesigning the code quite a
+ * bit. And honestly that's just not worth doing: unreachable instruction
+ * warnings are of questionable value anyway, and this is such a rare issue.
+ *
+ * 3. mov [rodata addr],%reg1
+ * ... some instructions ...
+ * jmpq *(%reg1,%reg2,8)
+ *
+ * This is a fairly uncommon pattern which is new for GCC 6. As of this
+ * writing, there are 11 occurrences of it in the allmodconfig kernel.
+ *
+ * TODO: Once we have DWARF CFI and smarter instruction decoding logic,
+ * ensure the same register is used in the mov and jump instructions.
+ */
+static struct rela *find_switch_table(struct objtool_file *file,
+ struct symbol *func,
+ struct instruction *insn)
{
- struct instruction *insn, *prev_jump;
- struct rela *text_rela, *rodata_rela, *prev_rela = NULL;
- int ret;
+ struct rela *text_rela, *rodata_rela;
- prev_jump = NULL;
+ text_rela = find_rela_by_dest_range(insn->sec, insn->offset, insn->len);
+ if (text_rela && text_rela->sym == file->rodata->sym) {
+ /* case 1 */
+ rodata_rela = find_rela_by_dest(file->rodata,
+ text_rela->addend);
+ if (rodata_rela)
+ return rodata_rela;
- func_for_each_insn(file, func, insn) {
- if (insn->type != INSN_JUMP_DYNAMIC)
- continue;
+ /* case 2 */
+ rodata_rela = find_rela_by_dest(file->rodata,
+ text_rela->addend + 4);
+ if (!rodata_rela)
+ return NULL;
+ file->ignore_unreachables = true;
+ return rodata_rela;
+ }
+
+ /* case 3 */
+ func_for_each_insn_continue_reverse(file, func, insn) {
+ if (insn->type == INSN_JUMP_UNCONDITIONAL ||
+ insn->type == INSN_JUMP_DYNAMIC)
+ break;
text_rela = find_rela_by_dest_range(insn->sec, insn->offset,
insn->len);
- if (!text_rela || text_rela->sym != file->rodata->sym)
- continue;
+ if (text_rela && text_rela->sym == file->rodata->sym)
+ return find_rela_by_dest(file->rodata,
+ text_rela->addend);
+ }
- /* common case: jmpq *[addr](,%rax,8) */
- rodata_rela = find_rela_by_dest(file->rodata,
- text_rela->addend);
+ return NULL;
+}
- /*
- * rare case: jmpq *[addr](%rip)
- *
- * This check is for a rare gcc quirk, currently only seen in
- * three driver functions in the kernel, only with certain
- * obscure non-distro configs.
- *
- * As part of an optimization, gcc makes a copy of an existing
- * switch jump table, modifies it, and then hard-codes the jump
- * (albeit with an indirect jump) to use a single entry in the
- * table. The rest of the jump table and some of its jump
- * targets remain as dead code.
- *
- * In such a case we can just crudely ignore all unreachable
- * instruction warnings for the entire object file. Ideally we
- * would just ignore them for the function, but that would
- * require redesigning the code quite a bit. And honestly
- * that's just not worth doing: unreachable instruction
- * warnings are of questionable value anyway, and this is such
- * a rare issue.
- *
- * kbuild reports:
- * - https://lkml.kernel.org/r/201603231906.LWcVUpxm%25fengguang.wu@intel.com
- * - https://lkml.kernel.org/r/201603271114.K9i45biy%25fengguang.wu@intel.com
- * - https://lkml.kernel.org/r/201603291058.zuJ6ben1%25fengguang.wu@intel.com
- *
- * gcc bug:
- * - https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70604
- */
- if (!rodata_rela) {
- rodata_rela = find_rela_by_dest(file->rodata,
- text_rela->addend + 4);
- if (rodata_rela)
- file->ignore_unreachables = true;
- }
+static int add_func_switch_tables(struct objtool_file *file,
+ struct symbol *func)
+{
+ struct instruction *insn, *prev_jump = NULL;
+ struct rela *rela, *prev_rela = NULL;
+ int ret;
- if (!rodata_rela)
+ func_for_each_insn(file, func, insn) {
+ if (insn->type != INSN_JUMP_DYNAMIC)
+ continue;
+
+ rela = find_switch_table(file, func, insn);
+ if (!rela)
continue;
/*
@@ -732,13 +768,13 @@ static int add_func_switch_tables(struct objtool_file *file,
*/
if (prev_jump) {
ret = add_switch_table(file, func, prev_jump, prev_rela,
- rodata_rela);
+ rela);
if (ret)
return ret;
}
prev_jump = insn;
- prev_rela = rodata_rela;
+ prev_rela = rela;
}
if (prev_jump) {
Powered by blists - more mailing lists