All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andy Lutomirski <luto@kernel.org>
To: Andy Lutomirski <luto@kernel.org>
Cc: X86 ML <x86@kernel.org>, LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] x86/entry: Treat BUG/WARN as NMI-like entries
Date: Thu, 11 Jun 2020 21:13:46 -0700	[thread overview]
Message-ID: <CALCETrUE48wbHUoaha=oEZFooM3yrcCwiCYZZ0MnEBDK-JFJAw@mail.gmail.com> (raw)
In-Reply-To: <f8fe40e0088749734b4435b554f73eee53dcf7a8.1591932307.git.luto@kernel.org>

On Thu, Jun 11, 2020 at 8:26 PM Andy Lutomirski <luto@kernel.org> wrote:
>
> If we BUG or WARN in a funny RCU context, we cleverly optimize the
> BUG/WARN using the ud2 hack, which takes us through the
> idtentry_enter...() paths, which might helpfully WARN that the RCU
> context is invalid, which results in infinite recursion.
>
> Split the BUG/WARN handling into an nmi_enter()/nmi_exit() path in
> exc_invalid_op() to increase the chance that we survive the
> experience.
>
> Signed-off-by: Andy Lutomirski <luto@kernel.org>
> ---
>
> This is not as well tested as I would like, but it does cause the splat
> I'm chasing to display a nice warning instead of causing an undebuggable
> stack overflow.
>
> (It would have been debuggable on x86_64, but it's a 32-bit splat, and
> x86_32 doesn't have ORC.)
>
>  arch/x86/kernel/traps.c | 61 +++++++++++++++++++++++------------------
>  arch/x86/mm/extable.c   | 15 ++++++++--
>  2 files changed, 48 insertions(+), 28 deletions(-)
>
> diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
> index cb8c3d26cdf5..6340b12a6616 100644
> --- a/arch/x86/kernel/traps.c
> +++ b/arch/x86/kernel/traps.c
> @@ -98,24 +98,6 @@ int is_valid_bugaddr(unsigned long addr)
>         return ud == INSN_UD0 || ud == INSN_UD2;
>  }
>
> -int fixup_bug(struct pt_regs *regs, int trapnr)
> -{
> -       if (trapnr != X86_TRAP_UD)
> -               return 0;
> -
> -       switch (report_bug(regs->ip, regs)) {
> -       case BUG_TRAP_TYPE_NONE:
> -       case BUG_TRAP_TYPE_BUG:
> -               break;
> -
> -       case BUG_TRAP_TYPE_WARN:
> -               regs->ip += LEN_UD2;
> -               return 1;
> -       }
> -
> -       return 0;
> -}
> -
>  static nokprobe_inline int
>  do_trap_no_signal(struct task_struct *tsk, int trapnr, const char *str,
>                   struct pt_regs *regs, long error_code)
> @@ -191,13 +173,6 @@ static void do_error_trap(struct pt_regs *regs, long error_code, char *str,
>  {
>         RCU_LOCKDEP_WARN(!rcu_is_watching(), "entry code didn't wake RCU");
>
> -       /*
> -        * WARN*()s end up here; fix them up before we call the
> -        * notifier chain.
> -        */
> -       if (!user_mode(regs) && fixup_bug(regs, trapnr))
> -               return;
> -
>         if (notify_die(DIE_TRAP, str, regs, error_code, trapnr, signr) !=
>                         NOTIFY_STOP) {
>                 cond_local_irq_enable(regs);
> @@ -242,9 +217,43 @@ static inline void handle_invalid_op(struct pt_regs *regs)
>                       ILL_ILLOPN, error_get_trap_addr(regs));
>  }
>
> -DEFINE_IDTENTRY(exc_invalid_op)
> +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.
> +        */
> +       if (!user_mode(regs) && is_valid_bugaddr(regs->ip)) {
> +               enum bug_trap_type type;
> +
> +               nmi_enter();
> +               instrumentation_begin();
> +               type = report_bug(regs->ip, regs);
> +               instrumentation_end();
> +               nmi_exit();

Hmm, maybe this should be:

        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();

tglx or peterz, feel free to fix this up and apply it however you like.

  reply	other threads:[~2020-06-12  4:14 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-12  3:26 Andy Lutomirski
2020-06-12  4:13 ` Andy Lutomirski [this message]
2020-06-12 19:50 ` [tip: x86/entry] " tip-bot2 for Andy Lutomirski
2020-06-15 14:50   ` Peter Zijlstra
2020-06-15 17:06     ` Andy Lutomirski
2020-06-15 19:44       ` Peter Zijlstra
2020-06-15 21:08         ` Andy Lutomirski
2020-06-15 22:23           ` Peter Zijlstra
2020-06-15 22:46             ` Andy Lutomirski
2020-06-16 11:14               ` Peter Zijlstra

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CALCETrUE48wbHUoaha=oEZFooM3yrcCwiCYZZ0MnEBDK-JFJAw@mail.gmail.com' \
    --to=luto@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=x86@kernel.org \
    --subject='Re: [PATCH] x86/entry: Treat BUG/WARN as NMI-like entries' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.