Re: [RFC][PATCH 00/16] objtool: vmlinux.o and noinstr validation

[Peter Zijlstra <peterz@infradead.org>]

On Thu, Mar 12, 2020 at 05:23:37PM +0100, Peter Zijlstra wrote:

> So one of the problem i've ran into while playing with this and Thomas'
> patches is that it is 'difficult' to deal with indirect function calls.
> 
> objtool basically gives up instantly.
> 
> I know smatch has passes were it looks for function pointer assignments
> and carries that forward into it's callchain generation. Doing something
> like that for objtool is going to be 'fun'...
> 
> For now I've got limited success dodging a few instances with
> __always_inline (which then results in the compiler resolving the
> indirection).

Here's a little something that at least detects 'immediate' function
pointers crossing the boundary.

It's slow though; it almost tripples the runtime. But I'm too tired to
make it fast, maybe tomorrow.

---

--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -247,6 +247,9 @@ static int decode_instructions(struct ob
 		    strncmp(sec->name, ".discard.", 9))
 			sec->text = true;

+		if (!strcmp(sec->name, ".noinstr.text"))
+			sec->noinstr = true;
+
 		for (offset = 0; offset < sec->len; offset += insn->len) {
 			insn = malloc(sizeof(*insn));
 			if (!insn) {
@@ -2040,6 +2043,28 @@ static int validate_return(struct symbol
 	return 0;
 }

+static int validate_rela(struct instruction *insn, struct insn_state *state)
+{
+	struct section *sec;
+	struct rela *rela;
+
+	if (!(state->noinstr && state->instr <= 0))
+		return 0;
+
+	rela = find_rela_by_dest_range(insn->sec, insn->offset, insn->len);
+	if (!rela || !rela->sym || !rela->sym->sec)
+		return 0;
+
+	sec = rela->sym->sec;
+	if (sec->text && !sec->noinstr) {
+		WARN_FUNC("loading non-noinstr function pointer\n",
+			  insn->sec, insn->offset);
+		return 1;
+	}
+
+	return 0;
+}
+
 /*
  * Follow the branch starting at the given instruction, and recursively follow
  * any other branches (jumps).  Meanwhile, track the frame pointer state at
@@ -2222,6 +2247,10 @@ static int validate_branch(struct objtoo
 			return 0;

 		case INSN_STACK:
+			ret = validate_rela(insn, &state);
+			if (ret)
+				return ret;
+
 			if (update_insn_state(insn, &state))
 				return 1;

@@ -2285,6 +2314,10 @@ static int validate_branch(struct objtoo
 			break;

 		default:
+			ret = validate_rela(insn, &state);
+			if (ret)
+				return ret;
+
 			break;
 		}

@@ -2442,8 +2475,8 @@ static int validate_sec_functions(struct
 	 * not correctly determine insn->call_dest->sec (external symbols do
 	 * not have a section).
 	 */
-	if (vmlinux && !strcmp(sec->name, ".noinstr.text"))
-		state.noinstr = true;
+	if (vmlinux)
+		state.noinstr = sec->noinstr;

 	list_for_each_entry(func, &sec->symbol_list, list) {
 		if (func->type != STT_FUNC)
--- a/tools/objtool/elf.h
+++ b/tools/objtool/elf.h
@@ -43,7 +43,7 @@ struct section {
 	char *name;
 	int idx;
 	unsigned int len;
-	bool changed, text, rodata;
+	bool changed, text, rodata, noinstr;
 };

 struct symbol {