[PATCH 01/13] x86/traps: Make exc_double_fault() consistently noreturn

Josh Poimboeuf posted 13 patches 9 months, 1 week ago
[PATCH 01/13] x86/traps: Make exc_double_fault() consistently noreturn
Posted by Josh Poimboeuf 9 months, 1 week ago
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: 55eeab2a8a11 ("objtool: Ignore exc_double_fault() __noreturn warnings")
Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
---
 arch/x86/kernel/traps.c | 18 +++++++++++++++++-
 tools/objtool/check.c   | 31 +------------------------------
 2 files changed, 18 insertions(+), 31 deletions(-)

diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
index f4263cb3d21e..9f88b8a78e50 100644
--- a/arch/x86/kernel/traps.c
+++ b/arch/x86/kernel/traps.c
@@ -461,6 +461,21 @@ __visible void __noreturn handle_stack_overflow(struct pt_regs *regs,
 }
 #endif
 
+/*
+ * Prevent the compiler and/or objtool from marking the !CONFIG_X86_ESPFIX64
+ * version of exc_double_fault() as noreturn.  Otherwise the noreturn mismatch
+ * between configs triggers objtool warnings.
+ *
+ * This is a temporary hack until we have compiler or plugin support for
+ * annotating noreturns.
+ */
+#ifdef CONFIG_X86_ESPFIX64
+#define always_true() true
+#else
+bool always_true(void);
+bool __weak always_true(void) { return true; }
+#endif
+
 /*
  * Runs on an IST stack for x86_64 and on a special task stack for x86_32.
  *
@@ -596,7 +611,8 @@ DEFINE_IDTENTRY_DF(exc_double_fault)
 
 	pr_emerg("PANIC: double fault, error_code: 0x%lx\n", error_code);
 	die("double fault", regs, error_code);
-	panic("Machine halted.");
+	if (always_true())
+		panic("Machine halted.");
 	instrumentation_end();
 }
 
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 814d40724069..3a3a1a905199 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -4441,35 +4441,6 @@ static int validate_sls(struct objtool_file *file)
 	return warnings;
 }
 
-static bool ignore_noreturn_call(struct instruction *insn)
-{
-	struct symbol *call_dest = insn_call_dest(insn);
-
-	/*
-	 * FIXME: hack, we need a real noreturn solution
-	 *
-	 * Problem is, exc_double_fault() may or may not return, depending on
-	 * whether CONFIG_X86_ESPFIX64 is set.  But objtool has no visibility
-	 * to the kernel config.
-	 *
-	 * Other potential ways to fix it:
-	 *
-	 *   - have compiler communicate __noreturn functions somehow
-	 *   - remove CONFIG_X86_ESPFIX64
-	 *   - read the .config file
-	 *   - add a cmdline option
-	 *   - create a generic objtool annotation format (vs a bunch of custom
-	 *     formats) and annotate it
-	 */
-	if (!strcmp(call_dest->name, "exc_double_fault")) {
-		/* prevent further unreachable warnings for the caller */
-		insn->sym->warned = 1;
-		return true;
-	}
-
-	return false;
-}
-
 static int validate_reachable_instructions(struct objtool_file *file)
 {
 	struct instruction *insn, *prev_insn;
@@ -4486,7 +4457,7 @@ static int validate_reachable_instructions(struct objtool_file *file)
 		prev_insn = prev_insn_same_sec(file, insn);
 		if (prev_insn && prev_insn->dead_end) {
 			call_dest = insn_call_dest(prev_insn);
-			if (call_dest && !ignore_noreturn_call(prev_insn)) {
+			if (call_dest) {
 				WARN_INSN(insn, "%s() is missing a __noreturn annotation",
 					  call_dest->name);
 				warnings++;
-- 
2.48.1
Re: [PATCH 01/13] x86/traps: Make exc_double_fault() consistently noreturn
Posted by Miroslav Benes 9 months ago
On Fri, 14 Mar 2025, Josh Poimboeuf wrote:

> 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: 55eeab2a8a11 ("objtool: Ignore exc_double_fault() __noreturn warnings")
> Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>

Reviewed-by: Miroslav Benes <mbenes@suse.cz>

M
Re: [PATCH 01/13] x86/traps: Make exc_double_fault() consistently noreturn
Posted by Brendan Jackman 9 months ago
On Fri, Mar 14, 2025 at 12:28:59PM -0700, Josh Poimboeuf wrote:
> 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.

Yeah it's pretty nasty, but I don't have a better idea. And we do
really want objtool to work on this code. (I am taking your word for
it on the objtool side issues, I have not researched that).

Reviewed-by: Brendan Jackman <jackmanb@google.com>