Skip to content

Commit 8085fcd

Browse files
jpoimboePeter Zijlstra
authored andcommitted
x86/traps: Make exc_double_fault() consistently noreturn
The CONFIG_X86_ESPFIX64 version of exc_double_fault() can return to its caller, but the !CONFIG_X86_ESPFIX64 version never does. In the latter case the compiler and/or objtool may consider it to be implicitly noreturn. However, due to the currently inflexible way objtool detects noreturns, a function's noreturn status needs to be consistent across configs. The current workaround for this issue is to suppress unreachable warnings for exc_double_fault()'s callers. Unfortunately that can result in ORC coverage gaps and potentially worse issues like inert static calls and silently disabled CPU mitigations. Instead, prevent exc_double_fault() from ever being implicitly marked noreturn by forcing a return behind a never-taken conditional. Until a more integrated noreturn detection method exists, this is likely the least objectionable workaround. Fixes: 55eeab2 ("objtool: Ignore exc_double_fault() __noreturn warnings") Signed-off-by: Josh Poimboeuf <[email protected]> Signed-off-by: Peter Zijlstra (Intel) <[email protected]> Reviewed-by: Brendan Jackman <[email protected]> Link: https://lore.kernel.org/r/d1f4026f8dc35d0de6cc61f2684e0cb6484009d1.1741975349.git.jpoimboe@kernel.org
1 parent e20ab7d commit 8085fcd

File tree

2 files changed

+18
-31
lines changed

2 files changed

+18
-31
lines changed

arch/x86/kernel/traps.c

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -379,6 +379,21 @@ __visible void __noreturn handle_stack_overflow(struct pt_regs *regs,
379379
}
380380
#endif
381381

382+
/*
383+
* Prevent the compiler and/or objtool from marking the !CONFIG_X86_ESPFIX64
384+
* version of exc_double_fault() as noreturn. Otherwise the noreturn mismatch
385+
* between configs triggers objtool warnings.
386+
*
387+
* This is a temporary hack until we have compiler or plugin support for
388+
* annotating noreturns.
389+
*/
390+
#ifdef CONFIG_X86_ESPFIX64
391+
#define always_true() true
392+
#else
393+
bool always_true(void);
394+
bool __weak always_true(void) { return true; }
395+
#endif
396+
382397
/*
383398
* Runs on an IST stack for x86_64 and on a special task stack for x86_32.
384399
*
@@ -514,7 +529,8 @@ DEFINE_IDTENTRY_DF(exc_double_fault)
514529

515530
pr_emerg("PANIC: double fault, error_code: 0x%lx\n", error_code);
516531
die("double fault", regs, error_code);
517-
panic("Machine halted.");
532+
if (always_true())
533+
panic("Machine halted.");
518534
instrumentation_end();
519535
}
520536

tools/objtool/check.c

Lines changed: 1 addition & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -4460,35 +4460,6 @@ static int validate_sls(struct objtool_file *file)
44604460
return warnings;
44614461
}
44624462

4463-
static bool ignore_noreturn_call(struct instruction *insn)
4464-
{
4465-
struct symbol *call_dest = insn_call_dest(insn);
4466-
4467-
/*
4468-
* FIXME: hack, we need a real noreturn solution
4469-
*
4470-
* Problem is, exc_double_fault() may or may not return, depending on
4471-
* whether CONFIG_X86_ESPFIX64 is set. But objtool has no visibility
4472-
* to the kernel config.
4473-
*
4474-
* Other potential ways to fix it:
4475-
*
4476-
* - have compiler communicate __noreturn functions somehow
4477-
* - remove CONFIG_X86_ESPFIX64
4478-
* - read the .config file
4479-
* - add a cmdline option
4480-
* - create a generic objtool annotation format (vs a bunch of custom
4481-
* formats) and annotate it
4482-
*/
4483-
if (!strcmp(call_dest->name, "exc_double_fault")) {
4484-
/* prevent further unreachable warnings for the caller */
4485-
insn->sym->warned = 1;
4486-
return true;
4487-
}
4488-
4489-
return false;
4490-
}
4491-
44924463
static int validate_reachable_instructions(struct objtool_file *file)
44934464
{
44944465
struct instruction *insn, *prev_insn;
@@ -4505,7 +4476,7 @@ static int validate_reachable_instructions(struct objtool_file *file)
45054476
prev_insn = prev_insn_same_sec(file, insn);
45064477
if (prev_insn && prev_insn->dead_end) {
45074478
call_dest = insn_call_dest(prev_insn);
4508-
if (call_dest && !ignore_noreturn_call(prev_insn)) {
4479+
if (call_dest) {
45094480
WARN_INSN(insn, "%s() is missing a __noreturn annotation",
45104481
call_dest->name);
45114482
warnings++;

0 commit comments

Comments
 (0)