Re: [PATCH 1/7] x86/entry: Fix #UD vs WARN more

Andy Lutomirski
2020-06-18T14:58:19Z

> On Jun 18, 2020, at 7:50 AM, Peter Zijlstra <peterz@infradead.org> wrote:
>
> ﻿vmlinux.o: warning: objtool: exc_invalid_op()+0x47: call to probe_kernel_read() leaves .noinstr.text section
>
> Since we use UD2 as a short-cut for 'CALL __WARN', treat it as such.
> Have the bare exception handler do the report_bug() thing.

I think you should consider inlining or noinstr-ifying report_bug() too if you want to make this more bulletproof. I admit the scenario where someone instruments it and it goes wrong is farfetched.

>
> Fixes: 15a416e8aaa7 ("x86/entry: Treat BUG/WARN as NMI-like entries")
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> ---
> ---
> arch/x86/kernel/traps.c | 50 +++++++++++++++++++++---------------------------
> 1 file changed, 22 insertions(+), 28 deletions(-)
>
> --- a/arch/x86/kernel/traps.c
> +++ b/arch/x86/kernel/traps.c
> @@ -216,40 +216,34 @@ static inline void handle_invalid_op(str
> ILL_ILLOPN, error_get_trap_addr(regs));
> }
>
> +static noinstr bool handle_bug(struct pt_regs *regs)
> +{
> + bool handled = false;
> +
> + /*
> + * All lies, just get the WARN/BUG out.
> + */
> + instrumentation_begin();
> + if (report_bug(regs->ip, regs) == BUG_TRAP_TYPE_WARN) {
> + regs->ip += LEN_UD2;
> + handled = true;
> + }
> + instrumentation_end();
> +
> + return handled;
> +}
> +
> DEFINE_IDTENTRY_RAW(exc_invalid_op)
> {
> bool rcu_exit;
>
> /*
> - * Handle BUG/WARN like NMIs instead of like normal idtentries:
> - * if we bugged/warned in a bad RCU context, for example, the last
> - * thing we want is to BUG/WARN again in the idtentry code, ad
> - * infinitum.
> + * We use UD2 as a short encoding for 'CALL __WARN', as such
> + * handle it before exception entry to avoid recursive WARN
> + * in case exception entry is the one triggering WARNs.
> */
> - if (!user_mode(regs) && is_valid_bugaddr(regs->ip)) {
> - enum bug_trap_type type;
> -
> - nmi_enter();
> - instrumentation_begin();
> - trace_hardirqs_off_finish();
> - type = report_bug(regs->ip, regs);
> - if (regs->flags & X86_EFLAGS_IF)
> - trace_hardirqs_on_prepare();
> - instrumentation_end();
> - nmi_exit();
> -
> - if (type == BUG_TRAP_TYPE_WARN) {
> - /* Skip the ud2. */
> - regs->ip += LEN_UD2;
> - return;
> - }
> -
> - /*
> - * Else, if this was a BUG and report_bug returns or if this
> - * was just a normal #UD, we want to continue onward and
> - * crash.
> - */
> - }
> + if (!user_mode(regs) && handle_bug(regs))
> + return;
>
> rcu_exit = idtentry_enter_cond_rcu(regs);
> instrumentation_begin();
>
>