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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <tip-7e578441a4a3bba2a79426ca0f709c801210d08e@git.kernel.org>
Date:	Fri, 15 Apr 2016 02:45:36 -0700
From:	tip-bot for Josh Poimboeuf <tipbot@...or.com>
To:	linux-tip-commits@...r.kernel.org
Cc:	hpa@...or.com, peterz@...radead.org, tglx@...utronix.de,
	mingo@...nel.org, linux-kernel@...r.kernel.org,
	fengguang.wu@...el.com, torvalds@...ux-foundation.org,
	jpoimboe@...hat.com
Subject: [tip:core/urgent] objtool: Add workaround for GCC switch jump table
 bug

Commit-ID:  7e578441a4a3bba2a79426ca0f709c801210d08e
Gitweb:     http://git.kernel.org/tip/7e578441a4a3bba2a79426ca0f709c801210d08e
Author:     Josh Poimboeuf <jpoimboe@...hat.com>
AuthorDate: Thu, 14 Apr 2016 14:52:24 -0500
Committer:  Ingo Molnar <mingo@...nel.org>
CommitDate: Fri, 15 Apr 2016 11:42:13 +0200

objtool: Add workaround for GCC switch jump table bug

GCC has a rare quirk, currently only seen in three driver functions in
the kernel, and only with certain obscure non-distro configs, which can
cause objtool to produce "unreachable instruction" false positive
warnings.

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 a very 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

Reported-by: kbuild test robot <fengguang.wu@...el.com>
Signed-off-by: Josh Poimboeuf <jpoimboe@...hat.com>
Cc: Linus Torvalds <torvalds@...ux-foundation.org>
Cc: Peter Zijlstra <peterz@...radead.org>
Cc: Thomas Gleixner <tglx@...utronix.de>
Link: http://lkml.kernel.org/r/700fa029bbb0feff34f03ffc69d666a3c3b57a61.1460663532.git.jpoimboe@redhat.com
Signed-off-by: Ingo Molnar <mingo@...nel.org>
---
 tools/objtool/builtin-check.c | 53 +++++++++++++++++++++++++++++++++----------
 1 file changed, 41 insertions(+), 12 deletions(-)

diff --git a/tools/objtool/builtin-check.c b/tools/objtool/builtin-check.c
index 7515cb2..157a0f9 100644
--- a/tools/objtool/builtin-check.c
+++ b/tools/objtool/builtin-check.c
@@ -66,6 +66,7 @@ struct objtool_file {
 	struct list_head insn_list;
 	DECLARE_HASHTABLE(insn_hash, 16);
 	struct section *rodata, *whitelist;
+	bool ignore_unreachables;
 };
 
 const char *objname;
@@ -664,13 +665,40 @@ static int add_func_switch_tables(struct objtool_file *file,
 						text_rela->addend);
 
 		/*
-		 * TODO: Document where this is needed, or get rid of it.
-		 *
 		 * 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)
+		if (!rodata_rela) {
 			rodata_rela = find_rela_by_dest(file->rodata,
 							text_rela->addend + 4);
+			if (rodata_rela)
+				file->ignore_unreachables = true;
+		}
 
 		if (!rodata_rela)
 			continue;
@@ -732,9 +760,6 @@ static int decode_sections(struct objtool_file *file)
 {
 	int ret;
 
-	file->whitelist = find_section_by_name(file->elf, "__func_stack_frame_non_standard");
-	file->rodata = find_section_by_name(file->elf, ".rodata");
-
 	ret = decode_instructions(file);
 	if (ret)
 		return ret;
@@ -1056,13 +1081,14 @@ static int validate_functions(struct objtool_file *file)
 				if (insn->visited)
 					continue;
 
-				if (!ignore_unreachable_insn(func, insn) &&
-				    !warnings) {
-					WARN_FUNC("function has unreachable instruction", insn->sec, insn->offset);
-					warnings++;
-				}
-
 				insn->visited = true;
+
+				if (file->ignore_unreachables || warnings ||
+				    ignore_unreachable_insn(func, insn))
+					continue;
+
+				WARN_FUNC("function has unreachable instruction", insn->sec, insn->offset);
+				warnings++;
 			}
 		}
 	}
@@ -1133,6 +1159,9 @@ int cmd_check(int argc, const char **argv)
 
 	INIT_LIST_HEAD(&file.insn_list);
 	hash_init(file.insn_hash);
+	file.whitelist = find_section_by_name(file.elf, "__func_stack_frame_non_standard");
+	file.rodata = find_section_by_name(file.elf, ".rodata");
+	file.ignore_unreachables = false;
 
 	ret = decode_sections(&file);
 	if (ret < 0)

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ