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

Peter Zijlstra
2020-03-12T22:23:34Z

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 {