[PATCH] x86/traps: move kmsan check after instrumentation_begin

[PATCH] x86/traps: move kmsan check after instrumentation_begin

[Sabyrzhan Tasbolatov]

During x86_64 kernel build with CONFIG_KMSAN, the objtool warns
following:

  AR      built-in.a
  AR      vmlinux.a
  LD      vmlinux.o
vmlinux.o: warning: objtool: handle_bug+0x4: call to
    kmsan_unpoison_entry_regs() leaves .noinstr.text section
  OBJCOPY modules.builtin.modinfo
  GEN     modules.builtin
  MODPOST Module.symvers
  CC      .vmlinux.export.o

Moving kmsan_unpoison_entry_regs() _after_ instrumentation_begin() fixes
the warning.

There is decode_bug(regs->ip, &imm) is left before KMSAN unpoisoining,
but it has the return condition and if we include it after
instrumentation_begin() it results the warning
"return with instrumentation enabled", hence, I'm concerned that regs
will not be KMSAN unpoisoned if `ud_type == BUG_NONE` is true.

Fixes: ba54d194f8da ("x86/traps: avoid KMSAN bugs originating from handle_bug()")
Signed-off-by: Sabyrzhan Tasbolatov <snovitoll@gmail.com>
---
 arch/x86/kernel/traps.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
index d05392db5d0..2dbadf347b5 100644
--- a/arch/x86/kernel/traps.c
+++ b/arch/x86/kernel/traps.c
@@ -261,12 +261,6 @@ static noinstr bool handle_bug(struct pt_regs *regs)
 	int ud_type;
 	u32 imm;
 
-	/*
-	 * Normally @regs are unpoisoned by irqentry_enter(), but handle_bug()
-	 * is a rare case that uses @regs without passing them to
-	 * irqentry_enter().
-	 */
-	kmsan_unpoison_entry_regs(regs);
 	ud_type = decode_bug(regs->ip, &imm);
 	if (ud_type == BUG_NONE)
 		return handled;
@@ -275,6 +269,12 @@ static noinstr bool handle_bug(struct pt_regs *regs)
 	 * All lies, just get the WARN/BUG out.
 	 */
 	instrumentation_begin();
+	/*
+	 * Normally @regs are unpoisoned by irqentry_enter(), but handle_bug()
+	 * is a rare case that uses @regs without passing them to
+	 * irqentry_enter().
+	 */
+	kmsan_unpoison_entry_regs(regs);
 	/*
 	 * Since we're emulating a CALL with exceptions, restore the interrupt
 	 * state to what it was at the exception site.
-- 
2.34.1

Re: [PATCH] x86/traps: move kmsan check after instrumentation_begin

[Alexander Potapenko]

On Wed, Oct 16, 2024 at 5:23 PM Sabyrzhan Tasbolatov
<snovitoll@gmail.com> wrote:
>
> During x86_64 kernel build with CONFIG_KMSAN, the objtool warns
> following:
>
>   AR      built-in.a
>   AR      vmlinux.a
>   LD      vmlinux.o
> vmlinux.o: warning: objtool: handle_bug+0x4: call to
>     kmsan_unpoison_entry_regs() leaves .noinstr.text section
>   OBJCOPY modules.builtin.modinfo
>   GEN     modules.builtin
>   MODPOST Module.symvers
>   CC      .vmlinux.export.o
>
> Moving kmsan_unpoison_entry_regs() _after_ instrumentation_begin() fixes
> the warning.

Thanks for taking care of this!

> There is decode_bug(regs->ip, &imm) is left before KMSAN unpoisoining,

(side note: decode_bug itself is inlined into handle_bug(), so it is
not instrumented, and no bugs are reported in it)

> but it has the return condition and if we include it after
> instrumentation_begin() it results the warning
> "return with instrumentation enabled", hence, I'm concerned that regs
> will not be KMSAN unpoisoned if `ud_type == BUG_NONE` is true.

So far the only caller of handle_bug() passes regs to
irqentry_enter(), which unpoisons them anyway.
I think this is fine, adding an extra instrumentation region around
kmsan_unpoison_entry_regs() in handle_bug() would be an overkill.

>
> Fixes: ba54d194f8da ("x86/traps: avoid KMSAN bugs originating from handle_bug()")
> Signed-off-by: Sabyrzhan Tasbolatov <snovitoll@gmail.com>
Reviewed-by: Alexander Potapenko <glider@google.com>