All of lore.kernel.org
 help / color / mirror / Atom feed
From: tip-bot for Jann Horn <tipbot@zytor.com>
To: linux-tip-commits@vger.kernel.org
Cc: ndesaulniers@google.com, linux-kernel@vger.kernel.org,
	mingo@kernel.org, tglx@linutronix.de, arnd@arndb.de,
	peterz@infradead.org, rdunlap@infradead.org, hpa@zytor.com,
	jpoimboe@redhat.com, jannh@google.com
Subject: [tip:core/urgent] objtool: Support repeated uses of the same C jump table
Date: Thu, 18 Jul 2019 12:20:48 -0700	[thread overview]
Message-ID: <tip-bd98c81346468fc2f86aeeb44d4d0d6f763a62b7@git.kernel.org> (raw)
In-Reply-To: <e995befaada9d4d8b2cf788ff3f566ba900d2b4d.1563413318.git.jpoimboe@redhat.com>

Commit-ID:  bd98c81346468fc2f86aeeb44d4d0d6f763a62b7
Gitweb:     https://git.kernel.org/tip/bd98c81346468fc2f86aeeb44d4d0d6f763a62b7
Author:     Jann Horn <jannh@google.com>
AuthorDate: Wed, 17 Jul 2019 20:36:54 -0500
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Thu, 18 Jul 2019 21:01:09 +0200

objtool: Support repeated uses of the same C jump table

This fixes objtool for both a GCC issue and a Clang issue:

1) GCC issue:

   kernel/bpf/core.o: warning: objtool: ___bpf_prog_run()+0x8d5: sibling call from callable instruction with modified stack frame

   With CONFIG_RETPOLINE=n, GCC is doing the following optimization in
   ___bpf_prog_run().

   Before:

           select_insn:
                   jmp *jumptable(,%rax,8)
                   ...
           ALU64_ADD_X:
                   ...
                   jmp select_insn
           ALU_ADD_X:
                   ...
                   jmp select_insn

   After:

           select_insn:
                   jmp *jumptable(, %rax, 8)
                   ...
           ALU64_ADD_X:
                   ...
                   jmp *jumptable(, %rax, 8)
           ALU_ADD_X:
                   ...
                   jmp *jumptable(, %rax, 8)

   This confuses objtool.  It has never seen multiple indirect jump
   sites which use the same jump table.

   For GCC switch tables, the only way of detecting the size of a table
   is by continuing to scan for more tables.  The size of the previous
   table can only be determined after another switch table is found, or
   when the scan reaches the end of the function.

   That logic was reused for C jump tables, and was based on the
   assumption that each jump table only has a single jump site.  The
   above optimization breaks that assumption.

2) Clang issue:

   drivers/usb/misc/sisusbvga/sisusb.o: warning: objtool: sisusb_write_mem_bulk()+0x588: can't find switch jump table

   With clang 9, code can be generated where a function contains two
   indirect jump instructions which use the same switch table.

The fix is the same for both issues: split the jump table parsing into
two passes.

In the first pass, locate the heads of all switch tables for the
function and mark their locations.

In the second pass, parse the switch tables and add them.

Fixes: e55a73251da3 ("bpf: Fix ORC unwinding in non-JIT BPF code")
Reported-by: Randy Dunlap <rdunlap@infradead.org>
Reported-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Jann Horn <jannh@google.com>
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Tested-by: Nick Desaulniers <ndesaulniers@google.com>
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Link: https://lkml.kernel.org/r/e995befaada9d4d8b2cf788ff3f566ba900d2b4d.1563413318.git.jpoimboe@redhat.com

Co-developed-by: Josh Poimboeuf <jpoimboe@redhat.com>
---
 tools/objtool/check.c | 53 +++++++++++++++++++++++++++------------------------
 tools/objtool/check.h |  1 +
 tools/objtool/elf.h   |  1 +
 3 files changed, 30 insertions(+), 25 deletions(-)

diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 4525cf677a1b..66f7c01385a4 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -900,7 +900,7 @@ out:
 }
 
 static int add_jump_table(struct objtool_file *file, struct instruction *insn,
-			    struct rela *table, struct rela *next_table)
+			    struct rela *table)
 {
 	struct rela *rela = table;
 	struct instruction *dest_insn;
@@ -913,7 +913,9 @@ static int add_jump_table(struct objtool_file *file, struct instruction *insn,
 	 * instruction.
 	 */
 	list_for_each_entry_from(rela, &table->sec->rela_list, list) {
-		if (rela == next_table)
+
+		/* Check for the end of the table: */
+		if (rela != table && rela->jump_table_start)
 			break;
 
 		/* Make sure the table entries are consecutive: */
@@ -1072,13 +1074,15 @@ static struct rela *find_jump_table(struct objtool_file *file,
 	return NULL;
 }
 
-
-static int add_func_jump_tables(struct objtool_file *file,
-				  struct symbol *func)
+/*
+ * First pass: Mark the head of each jump table so that in the next pass,
+ * we know when a given jump table ends and the next one starts.
+ */
+static void mark_func_jump_tables(struct objtool_file *file,
+				    struct symbol *func)
 {
-	struct instruction *insn, *last = NULL, *prev_jump = NULL;
-	struct rela *rela, *prev_rela = NULL;
-	int ret;
+	struct instruction *insn, *last = NULL;
+	struct rela *rela;
 
 	func_for_each_insn_all(file, func, insn) {
 		if (!last)
@@ -1102,26 +1106,24 @@ static int add_func_jump_tables(struct objtool_file *file,
 			continue;
 
 		rela = find_jump_table(file, func, insn);
-		if (!rela)
-			continue;
-
-		/*
-		 * We found a jump table, but we don't know yet how big it
-		 * is.  Don't add it until we reach the end of the function or
-		 * the beginning of another jump table in the same function.
-		 */
-		if (prev_jump) {
-			ret = add_jump_table(file, prev_jump, prev_rela, rela);
-			if (ret)
-				return ret;
+		if (rela) {
+			rela->jump_table_start = true;
+			insn->jump_table = rela;
 		}
-
-		prev_jump = insn;
-		prev_rela = rela;
 	}
+}
+
+static int add_func_jump_tables(struct objtool_file *file,
+				  struct symbol *func)
+{
+	struct instruction *insn;
+	int ret;
+
+	func_for_each_insn_all(file, func, insn) {
+		if (!insn->jump_table)
+			continue;
 
-	if (prev_jump) {
-		ret = add_jump_table(file, prev_jump, prev_rela, NULL);
+		ret = add_jump_table(file, insn, insn->jump_table);
 		if (ret)
 			return ret;
 	}
@@ -1148,6 +1150,7 @@ static int add_jump_table_alts(struct objtool_file *file)
 			if (func->type != STT_FUNC)
 				continue;
 
+			mark_func_jump_tables(file, func);
 			ret = add_func_jump_tables(file, func);
 			if (ret)
 				return ret;
diff --git a/tools/objtool/check.h b/tools/objtool/check.h
index cb60b9acf5cf..afa6a79e0715 100644
--- a/tools/objtool/check.h
+++ b/tools/objtool/check.h
@@ -38,6 +38,7 @@ struct instruction {
 	struct symbol *call_dest;
 	struct instruction *jump_dest;
 	struct instruction *first_jump_src;
+	struct rela *jump_table;
 	struct list_head alts;
 	struct symbol *func;
 	struct stack_op stack_op;
diff --git a/tools/objtool/elf.h b/tools/objtool/elf.h
index d4d3e0528d4a..44150204db4d 100644
--- a/tools/objtool/elf.h
+++ b/tools/objtool/elf.h
@@ -62,6 +62,7 @@ struct rela {
 	unsigned int type;
 	unsigned long offset;
 	int addend;
+	bool jump_table_start;
 };
 
 struct elf {

  reply	other threads:[~2019-07-18 19:21 UTC|newest]

Thread overview: 60+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-07-18  1:36 [PATCH v2 00/22] x86, objtool: several fixes/improvements Josh Poimboeuf
2019-07-18  1:36 ` [PATCH v2 01/22] x86/paravirt: Fix callee-saved function ELF sizes Josh Poimboeuf
2019-07-18 19:07   ` [tip:core/urgent] " tip-bot for Josh Poimboeuf
2019-07-18  1:36 ` [PATCH v2 02/22] x86/kvm: Fix fastop function ELF metadata Josh Poimboeuf
2019-07-18 19:08   ` [tip:core/urgent] " tip-bot for Josh Poimboeuf
2019-07-18  1:36 ` [PATCH v2 03/22] x86/kvm: Replace vmx_vmenter()'s call to kvm_spurious_fault() with UD2 Josh Poimboeuf
2019-07-18  8:17   ` Paolo Bonzini
2019-07-18 19:08   ` [tip:core/urgent] " tip-bot for Josh Poimboeuf
2019-07-18  1:36 ` [PATCH v2 04/22] x86/kvm: Don't call kvm_spurious_fault() from .fixup Josh Poimboeuf
2019-07-18  8:22   ` Paolo Bonzini
2019-07-18 13:16     ` Sean Christopherson
2019-07-18 13:18       ` Paolo Bonzini
2019-07-18 14:12         ` Josh Poimboeuf
2019-07-18 14:13           ` Paolo Bonzini
2019-07-18 14:03       ` Josh Poimboeuf
2019-07-18 19:09   ` [tip:core/urgent] " tip-bot for Josh Poimboeuf
2019-07-18  1:36 ` [PATCH v2 05/22] x86/entry: Fix thunk function ELF sizes Josh Poimboeuf
2019-07-18 19:10   ` [tip:core/urgent] " tip-bot for Josh Poimboeuf
2019-07-18  1:36 ` [PATCH v2 06/22] x86/head/64: Annotate start_cpu0() as non-callable Josh Poimboeuf
2019-07-18 19:11   ` [tip:core/urgent] " tip-bot for Josh Poimboeuf
2019-07-18  1:36 ` [PATCH v2 07/22] x86/uaccess: Remove ELF function annotation from copy_user_handle_tail() Josh Poimboeuf
2019-07-18 19:11   ` [tip:core/urgent] " tip-bot for Josh Poimboeuf
2019-07-18  1:36 ` [PATCH v2 08/22] x86/uaccess: Don't leak AC flag into fentry from mcsafe_handle_tail() Josh Poimboeuf
2019-07-18 19:12   ` [tip:core/urgent] " tip-bot for Josh Poimboeuf
2019-07-18  1:36 ` [PATCH v2 09/22] x86/uaccess: Remove redundant CLACs in getuser/putuser error paths Josh Poimboeuf
2019-07-18 19:13   ` [tip:core/urgent] " tip-bot for Josh Poimboeuf
2019-07-18  1:36 ` [PATCH v2 10/22] bpf: Disable GCC -fgcse optimization for ___bpf_prog_run() Josh Poimboeuf
2019-07-18 19:14   ` [tip:core/urgent] " tip-bot for Josh Poimboeuf
2020-04-29 21:51     ` BPF vs objtool again Josh Poimboeuf
2020-04-29 22:01       ` Arvind Sankar
2020-04-29 23:41       ` Alexei Starovoitov
2020-04-30  0:13         ` Josh Poimboeuf
2020-04-30  2:10           ` Alexei Starovoitov
2020-04-30  3:53             ` Josh Poimboeuf
2020-04-30  4:24               ` Alexei Starovoitov
2020-04-30  4:43                 ` Josh Poimboeuf
2019-07-18  1:36 ` [PATCH v2 11/22] objtool: Add mcsafe_handle_tail() to the uaccess safe list Josh Poimboeuf
2019-07-18 19:14   ` [tip:core/urgent] " tip-bot for Josh Poimboeuf
2019-07-18  1:36 ` [PATCH v2 12/22] objtool: Track original function across branches Josh Poimboeuf
2019-07-18 19:15   ` [tip:core/urgent] " tip-bot for Josh Poimboeuf
2019-07-18  1:36 ` [PATCH v2 13/22] objtool: Refactor function alias logic Josh Poimboeuf
2019-07-18 19:16   ` [tip:core/urgent] " tip-bot for Josh Poimboeuf
2019-07-18  1:36 ` [PATCH v2 14/22] objtool: Warn on zero-length functions Josh Poimboeuf
2019-07-18 19:17   ` [tip:core/urgent] " tip-bot for Josh Poimboeuf
2019-07-18  1:36 ` [PATCH v2 15/22] objtool: Change dead_end_function() to return boolean Josh Poimboeuf
2019-07-18 19:17   ` [tip:core/urgent] " tip-bot for Josh Poimboeuf
2019-07-18  1:36 ` [PATCH v2 16/22] objtool: Do frame pointer check before dead end check Josh Poimboeuf
2019-07-18 19:18   ` [tip:core/urgent] " tip-bot for Josh Poimboeuf
2019-07-18  1:36 ` [PATCH v2 17/22] objtool: Refactor sibling call detection logic Josh Poimboeuf
2019-07-18 19:19   ` [tip:core/urgent] " tip-bot for Josh Poimboeuf
2019-07-18  1:36 ` [PATCH v2 18/22] objtool: Refactor jump table code Josh Poimboeuf
2019-07-18 19:20   ` [tip:core/urgent] " tip-bot for Josh Poimboeuf
2019-07-18  1:36 ` [PATCH v2 19/22] objtool: Support repeated uses of the same C jump table Josh Poimboeuf
2019-07-18 19:20   ` tip-bot for Jann Horn [this message]
2019-07-18  1:36 ` [PATCH v2 20/22] objtool: Fix seg fault on bad switch table entry Josh Poimboeuf
2019-07-18 19:21   ` [tip:core/urgent] " tip-bot for Josh Poimboeuf
2019-07-18  1:36 ` [PATCH v2 21/22] objtool: convert insn type to enum Josh Poimboeuf
2019-07-18 19:22   ` [tip:core/urgent] objtool: Convert " tip-bot for Josh Poimboeuf
2019-07-18  1:36 ` [PATCH v2 22/22] objtool: Support conditional retpolines Josh Poimboeuf
2019-07-18 19:23   ` [tip:core/urgent] " tip-bot for Josh Poimboeuf

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=tip-bd98c81346468fc2f86aeeb44d4d0d6f763a62b7@git.kernel.org \
    --to=tipbot@zytor.com \
    --cc=arnd@arndb.de \
    --cc=hpa@zytor.com \
    --cc=jannh@google.com \
    --cc=jpoimboe@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-tip-commits@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=ndesaulniers@google.com \
    --cc=peterz@infradead.org \
    --cc=rdunlap@infradead.org \
    --cc=tglx@linutronix.de \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.