linux-kernel - [PATCH 4/7] objtool: Add support for return trampoline call

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] [thread-next>] [day] [month] [year] [list]
Date:   Thu,  2 Apr 2020 10:22:17 +0200
From:   Alexandre Chartre <alexandre.chartre@...cle.com>
To:     x86@...nel.org
Cc:     linux-kernel@...r.kernel.org, jpoimboe@...hat.com,
        peterz@...radead.org, jthierry@...hat.com, tglx@...utronix.de,
        alexandre.chartre@...cle.com
Subject: [PATCH 4/7] objtool: Add support for return trampoline call

With retpoline, the return instruction is used to branch to an address
stored on the stack. So, unlike a regular return instruction, when a
retpoline return instruction is reached the stack has been modified
compared to what we have when the function was entered.

Provide the mechanism to explicitly call-out such return instruction
so that objtool can correctly handle them.

Signed-off-by: Alexandre Chartre <alexandre.chartre@...cle.com>
---
 tools/objtool/check.c | 78 +++++++++++++++++++++++++++++++++++++++++--
 tools/objtool/check.h |  1 +
 2 files changed, 76 insertions(+), 3 deletions(-)

diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 0cec91291d46..ed8e3ea1d8da 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -1344,6 +1344,48 @@ static int read_intra_function_call(struct objtool_file *file)
 	return 0;
 }
 
+static int read_retpoline_ret(struct objtool_file *file)
+{
+	struct section *sec;
+	struct instruction *insn;
+	struct rela *rela;
+
+	sec = find_section_by_name(file->elf, ".rela.discard.retpoline_ret");
+	if (!sec)
+		return 0;
+
+	list_for_each_entry(rela, &sec->rela_list, list) {
+		if (rela->sym->type != STT_SECTION) {
+			WARN("unexpected relocation symbol type in %s",
+			     sec->name);
+			return -1;
+		}
+
+		insn = find_insn(file, rela->sym->sec, rela->addend);
+		if (!insn) {
+			WARN("bad .discard.retpoline_ret entry");
+			return -1;
+		}
+
+		if (insn->type != INSN_RETURN) {
+			WARN_FUNC("retpoline_ret not a return",
+				  insn->sec, insn->offset);
+			return -1;
+		}
+
+		insn->retpoline_ret = true;
+		/*
+		 * For the impact on the stack, make a return trampoline
+		 * behaves like a pop of the return address.
+		 */
+		insn->stack_op.src.type = OP_SRC_POP;
+		insn->stack_op.dest.type = OP_DEST_REG;
+		insn->stack_op.dest.reg = CFI_RA;
+	}
+
+	return 0;
+}
+
 static void mark_rodata(struct objtool_file *file)
 {
 	struct section *sec;
@@ -1403,6 +1445,10 @@ static int decode_sections(struct objtool_file *file)
 	if (ret)
 		return ret;
 
+	ret = read_retpoline_ret(file);
+	if (ret)
+		return ret;
+
 	ret = add_call_destinations(file);
 	if (ret)
 		return ret;
@@ -1432,7 +1478,8 @@ static bool is_fentry_call(struct instruction *insn)
 	return false;
 }
 
-static bool has_modified_stack_frame(struct insn_state *state)
+static bool has_modified_stack_frame(struct insn_state *state,
+				     bool check_registers)
 {
 	int i;
 
@@ -1442,6 +1489,9 @@ static bool has_modified_stack_frame(struct insn_state *state)
 	    state->drap)
 		return true;
 
+	if (!check_registers)
+		return false;
+
 	for (i = 0; i < CFI_NUM_REGS; i++)
 		if (state->regs[i].base != initial_func_cfi.regs[i].base ||
 		    state->regs[i].offset != initial_func_cfi.regs[i].offset)
@@ -1987,7 +2037,7 @@ static int validate_call(struct instruction *insn, struct insn_state *state)
 
 static int validate_sibling_call(struct instruction *insn, struct insn_state *state)
 {
-	if (has_modified_stack_frame(state)) {
+	if (has_modified_stack_frame(state, true)) {
 		WARN_FUNC("sibling call from callable instruction with modified stack frame",
 				insn->sec, insn->offset);
 		return 1;
@@ -2009,6 +2059,7 @@ static int validate_branch(struct objtool_file *file, struct symbol *func,
 	struct alternative *alt;
 	struct instruction *insn, *next_insn;
 	struct section *sec;
+	bool check_registers;
 	u8 visited;
 	int ret;
 
@@ -2130,7 +2181,28 @@ static int validate_branch(struct objtool_file *file, struct symbol *func,
 				return 1;
 			}
 
-			if (func && has_modified_stack_frame(&state)) {
+			/*
+			 * With retpoline, the return instruction is used
+			 * to branch to an address stored on the stack.
+			 * So when we reach the ret instruction, the stack
+			 * frame has been modified with the address to
+			 * branch to and we need update the stack state.
+			 *
+			 * The retpoline address to branch to is typically
+			 * pushed on the stack from a register, but this
+			 * confuses the logic which checks callee saved
+			 * registers. So we don't check if registers have
+			 * been modified if we have a return trampoline.
+			 */
+			if (insn->retpoline_ret) {
+				update_insn_state(insn, &state);
+				check_registers = false;
+			} else {
+				check_registers = true;
+			}
+
+			if (func && has_modified_stack_frame(&state,
+							     check_registers)) {
 				WARN_FUNC("return with modified stack frame",
 					  sec, insn->offset);
 				return 1;
diff --git a/tools/objtool/check.h b/tools/objtool/check.h
index 2bd6d2f46baa..5ecd16ad71a8 100644
--- a/tools/objtool/check.h
+++ b/tools/objtool/check.h
@@ -37,6 +37,7 @@ struct instruction {
 	bool dead_end, ignore, hint, save, restore, ignore_alts;
 	bool intra_function_call;
 	bool retpoline_safe;
+	bool retpoline_ret;
 	u8 visited;
 	struct symbol *call_dest;
 	struct instruction *jump_dest;
-- 
2.18.2