Skip to content

Commit 54262aa

Browse files
Peter ZijlstraIngo Molnar
authored andcommitted
objtool: Fix sibling call detection
It turned out that we failed to detect some sibling calls; specifically those without relocation records; like: $ ./objdump-func.sh defconfig-build/mm/kasan/generic.o __asan_loadN 0000 0000000000000840 <__asan_loadN>: 0000 840: 48 8b 0c 24 mov (%rsp),%rcx 0004 844: 31 d2 xor %edx,%edx 0006 846: e9 45 fe ff ff jmpq 690 <check_memory_region> So extend the cross-function jump to also consider those that are not between known (or newly detected) parent/child functions, as sibling-cals when they jump to the start of the function. The second part of that condition is to deal with random jumps to the middle of other function, as can be found in arch/x86/lib/copy_user_64.S for example. This then (with later patches applied) makes the above recognise the sibling call: mm/kasan/generic.o: warning: objtool: __asan_loadN()+0x6: call to check_memory_region() with UACCESS enabled Also make sure to set insn->call_dest for sibling calls so we can know who we're calling. This is useful information when printing validation warnings later. Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> Acked-by: Josh Poimboeuf <jpoimboe@redhat.com> Cc: Borislav Petkov <bp@alien8.de> Cc: Linus Torvalds <torvalds@linux-foundation.org> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Thomas Gleixner <tglx@linutronix.de> Signed-off-by: Ingo Molnar <mingo@kernel.org>
1 parent 764eef4 commit 54262aa

File tree

1 file changed

+55
-31
lines changed

1 file changed

+55
-31
lines changed

tools/objtool/check.c

Lines changed: 55 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -515,7 +515,8 @@ static int add_jump_destinations(struct objtool_file *file)
515515
continue;
516516
} else {
517517
/* sibling call */
518-
insn->jump_dest = 0;
518+
insn->call_dest = rela->sym;
519+
insn->jump_dest = NULL;
519520
continue;
520521
}
521522

@@ -537,25 +538,38 @@ static int add_jump_destinations(struct objtool_file *file)
537538
}
538539

539540
/*
540-
* For GCC 8+, create parent/child links for any cold
541-
* subfunctions. This is _mostly_ redundant with a similar
542-
* initialization in read_symbols().
543-
*
544-
* If a function has aliases, we want the *first* such function
545-
* in the symbol table to be the subfunction's parent. In that
546-
* case we overwrite the initialization done in read_symbols().
547-
*
548-
* However this code can't completely replace the
549-
* read_symbols() code because this doesn't detect the case
550-
* where the parent function's only reference to a subfunction
551-
* is through a switch table.
541+
* Cross-function jump.
552542
*/
553543
if (insn->func && insn->jump_dest->func &&
554-
insn->func != insn->jump_dest->func &&
555-
!strstr(insn->func->name, ".cold.") &&
556-
strstr(insn->jump_dest->func->name, ".cold.")) {
557-
insn->func->cfunc = insn->jump_dest->func;
558-
insn->jump_dest->func->pfunc = insn->func;
544+
insn->func != insn->jump_dest->func) {
545+
546+
/*
547+
* For GCC 8+, create parent/child links for any cold
548+
* subfunctions. This is _mostly_ redundant with a
549+
* similar initialization in read_symbols().
550+
*
551+
* If a function has aliases, we want the *first* such
552+
* function in the symbol table to be the subfunction's
553+
* parent. In that case we overwrite the
554+
* initialization done in read_symbols().
555+
*
556+
* However this code can't completely replace the
557+
* read_symbols() code because this doesn't detect the
558+
* case where the parent function's only reference to a
559+
* subfunction is through a switch table.
560+
*/
561+
if (!strstr(insn->func->name, ".cold.") &&
562+
strstr(insn->jump_dest->func->name, ".cold.")) {
563+
insn->func->cfunc = insn->jump_dest->func;
564+
insn->jump_dest->func->pfunc = insn->func;
565+
566+
} else if (insn->jump_dest->func->pfunc != insn->func->pfunc &&
567+
insn->jump_dest->offset == insn->jump_dest->func->offset) {
568+
569+
/* sibling class */
570+
insn->call_dest = insn->jump_dest->func;
571+
insn->jump_dest = NULL;
572+
}
559573
}
560574
}
561575

@@ -1785,6 +1799,17 @@ static bool insn_state_match(struct instruction *insn, struct insn_state *state)
17851799
return false;
17861800
}
17871801

1802+
static int validate_sibling_call(struct instruction *insn, struct insn_state *state)
1803+
{
1804+
if (has_modified_stack_frame(state)) {
1805+
WARN_FUNC("sibling call from callable instruction with modified stack frame",
1806+
insn->sec, insn->offset);
1807+
return 1;
1808+
}
1809+
1810+
return 0;
1811+
}
1812+
17881813
/*
17891814
* Follow the branch starting at the given instruction, and recursively follow
17901815
* any other branches (jumps). Meanwhile, track the frame pointer state at
@@ -1935,21 +1960,21 @@ static int validate_branch(struct objtool_file *file, struct instruction *first,
19351960

19361961
case INSN_JUMP_CONDITIONAL:
19371962
case INSN_JUMP_UNCONDITIONAL:
1938-
if (insn->jump_dest &&
1939-
(!func || !insn->jump_dest->func ||
1940-
insn->jump_dest->func->pfunc == func)) {
1963+
if (func && !insn->jump_dest) {
1964+
ret = validate_sibling_call(insn, &state);
1965+
if (ret)
1966+
return ret;
1967+
1968+
} else if (insn->jump_dest &&
1969+
(!func || !insn->jump_dest->func ||
1970+
insn->jump_dest->func->pfunc == func)) {
19411971
ret = validate_branch(file, insn->jump_dest,
19421972
state);
19431973
if (ret) {
19441974
if (backtrace)
19451975
BT_FUNC("(branch)", insn);
19461976
return ret;
19471977
}
1948-
1949-
} else if (func && has_modified_stack_frame(&state)) {
1950-
WARN_FUNC("sibling call from callable instruction with modified stack frame",
1951-
sec, insn->offset);
1952-
return 1;
19531978
}
19541979

19551980
if (insn->type == INSN_JUMP_UNCONDITIONAL)
@@ -1958,11 +1983,10 @@ static int validate_branch(struct objtool_file *file, struct instruction *first,
19581983
break;
19591984

19601985
case INSN_JUMP_DYNAMIC:
1961-
if (func && list_empty(&insn->alts) &&
1962-
has_modified_stack_frame(&state)) {
1963-
WARN_FUNC("sibling call from callable instruction with modified stack frame",
1964-
sec, insn->offset);
1965-
return 1;
1986+
if (func && list_empty(&insn->alts)) {
1987+
ret = validate_sibling_call(insn, &state);
1988+
if (ret)
1989+
return ret;
19661990
}
19671991

19681992
return 0;

0 commit comments

Comments
 (0)