All of lore.kernel.org
 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: 29+ 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 22:41     ` kbuild test robot
2020-04-14 23:09   ` 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 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.