mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Alexandre Chartre <alexandre.chartre@oracle.com>
To: x86@kernel.org
Cc: linux-kernel@vger.kernel.org, jpoimboe@redhat.com,
	peterz@infradead.org, jthierry@redhat.com, tglx@linutronix.de,
	alexandre.chartre@oracle.com
Subject: [PATCH V3 4/9] objtool: Handle return instruction with intra-function call
Date: Tue, 14 Apr 2020 12:36:13 +0200	[thread overview]
Message-ID: <20200414103618.12657-5-alexandre.chartre@oracle.com> (raw)
In-Reply-To: <20200414103618.12657-1-alexandre.chartre@oracle.com>

Intra-function calls are implemented in objtool like unconditional
jumps. Keep track of intra-functin calls return addresses so that
objtool can make a return instruction continues the flow at the
right location.

Signed-off-by: Alexandre Chartre <alexandre.chartre@oracle.com>
---
 tools/objtool/arch/x86/decode.c |   7 +++
 tools/objtool/check.c           | 104 ++++++++++++++++++++++++++++++--
 tools/objtool/check.h           |   3 +
 3 files changed, 110 insertions(+), 4 deletions(-)

diff --git a/tools/objtool/arch/x86/decode.c b/tools/objtool/arch/x86/decode.c
index f4d70b8835c4..76b593bb2e4f 100644
--- a/tools/objtool/arch/x86/decode.c
+++ b/tools/objtool/arch/x86/decode.c
@@ -427,6 +427,13 @@ int arch_decode_instruction(struct elf *elf, struct section *sec,
 	case 0xc2:
 	case 0xc3:
 		*type = INSN_RETURN;
+		/*
+		 * For the impact on the stack, a ret behaves like
+		 * a pop of the return address.
+		 */
+		op->src.type = OP_SRC_POP;
+		op->dest.type = OP_DEST_REG;
+		op->dest.reg = CFI_RA;
 		break;
 
 	case 0xca: /* retf */
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index ad362c5de281..8b1df659cd68 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -26,9 +26,50 @@ struct alternative {
 	bool skip_orig;
 };
 
+/*
+ * List to keep track of intra-function call return addresses.
+ * The list is a simple static array because we don't expect
+ * to have a lot of nested intra-function calls.
+ */
+#define RADDR_COUNT_MAX		32
+#define RADDR_ALTERED		((void *)-1)
+
+static struct instruction *raddr_list[RADDR_COUNT_MAX];
+static int raddr_count;
+
 const char *objname;
 struct cfi_state initial_func_cfi;
 
+static void raddr_clear(void)
+{
+	raddr_count = 0;
+}
+
+static bool raddr_push(struct instruction *insn)
+{
+	if (raddr_count == RADDR_COUNT_MAX) {
+		WARN_FUNC("return address list is full",
+			  insn->sec, insn->offset);
+		return false;
+	}
+
+	raddr_list[raddr_count++] = insn;
+	return true;
+}
+
+static bool raddr_pop(struct instruction **insn)
+{
+	if (raddr_count == 0)
+		return false;
+
+	*insn = raddr_list[--raddr_count];
+	return true;
+}
+
+static int validate_branch(struct objtool_file *file, struct symbol *func,
+			   struct instruction *from,
+			   struct instruction *first, struct insn_state state);
+
 struct instruction *find_insn(struct objtool_file *file,
 			      struct section *sec, unsigned long offset)
 {
@@ -2039,8 +2080,52 @@ static int validate_sibling_call(struct instruction *insn, struct insn_state *st
 	return validate_call(insn, state);
 }
 
-static int validate_return(struct symbol *func, struct instruction *insn, struct insn_state *state)
+static int validate_return_address(struct objtool_file *file,
+				   struct symbol *func,
+				   struct instruction *insn,
+				   struct insn_state *state)
 {
+	struct instruction *raddr_insn;
+	int ret;
+
+	while (raddr_pop(&raddr_insn)) {
+		/*
+		 * We are branching somewhere and so processing
+		 * a return instruction. So update the stack
+		 * state for this instruction.
+		 */
+		update_insn_state(insn, state);
+
+		/*
+		 * If the return address has no instruction then
+		 * that's the end of the function.
+		 */
+		if (!raddr_insn)
+			break;
+
+		/*
+		 * If we are branching to a defined address then
+		 * just do an unconditional jump there.
+		 */
+		ret = validate_branch(file, func, insn,
+				      raddr_insn, *state);
+		if (ret) {
+			if (backtrace)
+				BT_FUNC("(ret-branch)", insn);
+			return ret;
+		}
+
+		return 0;
+	}
+
+	return -1;
+}
+
+static int validate_return(struct objtool_file *file, struct symbol *func,
+			   struct instruction *insn, struct insn_state *state)
+{
+	int ret;
+
 	if (state->uaccess && !func_uaccess_safe(func)) {
 		WARN_FUNC("return with UACCESS enabled",
 			  insn->sec, insn->offset);
@@ -2059,6 +2144,11 @@ static int validate_return(struct symbol *func, struct instruction *insn, struct
 		return 1;
 	}
 
+	/* check if we have return address to branch to */
+	ret = validate_return_address(file, func, insn, state);
+	if (ret >= 0)
+		return ret;
+
 	if (func && has_modified_stack_frame(state)) {
 		WARN_FUNC("return with modified stack frame",
 			  insn->sec, insn->offset);
@@ -2200,7 +2290,7 @@ static int validate_branch(struct objtool_file *file, struct symbol *func,
 		switch (insn->type) {
 
 		case INSN_RETURN:
-			return validate_return(func, insn, &state);
+			return validate_return(file, func, insn, &state);
 
 		case INSN_CALL:
 		case INSN_CALL_DYNAMIC:
@@ -2223,12 +2313,17 @@ static int validate_branch(struct objtool_file *file, struct symbol *func,
 				/*
 				 * The call instruction can update the stack
 				 * state. Then make the intra-function call
-				 * behaves like and unconditional jump.
+				 * behaves like and unconditional jump. We
+				 * track the return address to handle any
+				 * return instruction.
 				 */
 				ret = update_insn_state(insn, &state);
 				if (ret)
 					return ret;
 
+				if (!raddr_push(next_insn))
+					return 1;
+
 				ret = validate_branch(file, func, insn,
 						      insn->jump_dest, state);
 				if (ret) {
@@ -2383,6 +2478,7 @@ static int validate_unwind_hints(struct objtool_file *file)
 
 	for_each_insn(file, insn) {
 		if (insn->hint && !insn->visited) {
+			raddr_clear();
 			ret = validate_branch(file, insn->func,
 					      NULL, insn, state);
 			if (ret && backtrace)
@@ -2522,7 +2618,7 @@ static int validate_section(struct objtool_file *file, struct section *sec)
 			continue;
 
 		state.uaccess = func->uaccess_safe;
-
+		raddr_clear();
 		ret = validate_branch(file, func, NULL, insn, state);
 		if (ret && backtrace)
 			BT_FUNC("<=== (func)", insn);
diff --git a/tools/objtool/check.h b/tools/objtool/check.h
index 6a80903fc4aa..f7dbecd46bed 100644
--- a/tools/objtool/check.h
+++ b/tools/objtool/check.h
@@ -23,6 +23,7 @@ struct insn_state {
 	unsigned int uaccess_stack;
 	int drap_reg, drap_offset;
 	struct cfi_reg vals[CFI_NUM_REGS];
+	bool stack_altered;
 };
 
 struct instruction {
@@ -39,6 +40,8 @@ struct instruction {
 	bool intra_function_call;
 	bool retpoline_safe;
 	u8 visited;
+	u8 raddr_delete;
+	u8 raddr_alter;
 	struct symbol *call_dest;
 	struct instruction *jump_dest;
 	struct instruction *first_jump_src;
-- 
2.18.2


  parent reply	other threads:[~2020-04-14 10:55 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-14 10:36 [PATCH V3 0/9] objtool changes to check retpoline code Alexandre Chartre
2020-04-14 10:36 ` [PATCH V3 1/9] objtool: is_fentry_call() crashes if call has no destination Alexandre Chartre
2020-05-01 18:22   ` [tip: objtool/core] " tip-bot2 for Alexandre Chartre
2020-04-14 10:36 ` [PATCH V3 2/9] objtool: Allow branches within the same alternative Alexandre Chartre
2020-04-14 10:36 ` [PATCH V3 3/9] objtool: Add support for intra-function calls Alexandre Chartre
2020-04-14 12:07   ` Julien Thierry
2020-04-16 12:12   ` Miroslav Benes
2020-05-01 18:22   ` [tip: objtool/core] " tip-bot2 for Alexandre Chartre
2020-04-14 10:36 ` Alexandre Chartre [this message]
2020-04-14 13:44   ` [PATCH V3 4/9] objtool: Handle return instruction with intra-function call Julien Thierry
2020-04-14 10:36 ` [PATCH V3 5/9] objtool: Add return address unwind hints Alexandre Chartre
2020-04-14 16:16   ` Peter Zijlstra
2020-04-14 16:40     ` Alexandre Chartre
2020-04-14 17:56       ` Peter Zijlstra
2020-04-14 18:31         ` Alexandre Chartre
2020-04-14 18:42           ` Peter Zijlstra
2020-04-14 19:27             ` Alexandre Chartre
2020-04-14 19:48               ` Peter Zijlstra
2020-04-14 10:36 ` [PATCH V3 6/9] objtool: Report inconsistent stack changes in alternative Alexandre Chartre
2020-04-14 15:35   ` Julien Thierry
2020-04-14 22:41   ` kbuild test robot
2020-04-14 23:09   ` kbuild test robot
2020-04-16 14:18   ` Peter Zijlstra
2020-04-16 14:43     ` Alexandre Chartre
2020-04-14 10:36 ` [PATCH V3 7/9] x86/speculation: Change __FILL_RETURN_BUFFER to work with objtool Alexandre Chartre
2020-04-14 10:36 ` [PATCH V3 8/9] x86/speculation: Add return address unwind hints to retpoline and RSB stuffing Alexandre Chartre
2020-04-14 10:36 ` [PATCH V3 9/9] x86/speculation: Annotate intra-function calls Alexandre Chartre

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=20200414103618.12657-5-alexandre.chartre@oracle.com \
    --to=alexandre.chartre@oracle.com \
    --cc=jpoimboe@redhat.com \
    --cc=jthierry@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=peterz@infradead.org \
    --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

Powered by JetHome