Date:   Wed, 9 Dec 2020 22:50:01 +0100
From:   Adam Zabrocki <pi3@....com.pl>
To:     Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        x86@...nel.org, "H. Peter Anvin" <hpa@...or.com>,
        linux-kernel@...r.kernel.org,
        "Naveen N. Rao" <naveen.n.rao@...ux.ibm.com>,
        Anil S Keshavamurthy <anil.s.keshavamurthy@...el.com>,
        "David S. Miller" <davem@...emloft.net>,
        Masami Hiramatsu <mhiramat@...nel.org>,
        Solar Designer <solar@...nwall.com>
Subject: KRETPROBES are broken since kernel 5.8

Hello,

Starting from kernel 5.8 all non-optimized kretprobes don't work. Until 5.8,
when #DB exception was raised, entry to the NMI was not fully performed. Among
others, the following logic was executed:
https://elixir.bootlin.com/linux/v5.7.19/source/arch/x86/kernel/traps.c#L589

    if (!user_mode(regs)) {
        rcu_nmi_enter();
        preempt_disable();
    }

In some older kernels function ist_enter() was called instead. Inside this
function we can see the following logic:
https://elixir.bootlin.com/linux/v5.7.19/source/arch/x86/kernel/traps.c#L91

    if (user_mode(regs)) {
        RCU_LOCKDEP_WARN(!rcu_is_watching(), "entry code didn't wake RCU");
    } else {
        /*
         * We might have interrupted pretty much anything.  In
         * fact, if we're a machine check, we can even interrupt
         * NMI processing.  We don't want in_nmi() to return true,
         * but we need to notify RCU.
         */
        rcu_nmi_enter();
    }

    preempt_disable();

As the comment says "We don't want in_nmi() to return true, but we need to
notify RCU.". However, since kernel 5.8 the logic of how interrupts are handled
was modified and currently we have this (function "exc_int3"):
https://elixir.bootlin.com/linux/v5.8/source/arch/x86/kernel/traps.c#L630

    /*
     * idtentry_enter_user() uses static_branch_{,un}likely() and therefore
     * can trigger INT3, hence poke_int3_handler() must be done
     * before. If the entry came from kernel mode, then use nmi_enter()
     * because the INT3 could have been hit in any context including
     * NMI.
     */
    if (user_mode(regs)) {
        idtentry_enter_user(regs);
        instrumentation_begin();
        do_int3_user(regs);
        instrumentation_end();
        idtentry_exit_user(regs);
    } else {
        nmi_enter();
        instrumentation_begin();
        trace_hardirqs_off_finish();
        if (!do_int3(regs))
            die("int3", regs, 0);
        if (regs->flags & X86_EFLAGS_IF)
            trace_hardirqs_on_prepare();
        instrumentation_end();
        nmi_exit();
    }

The root of unlucky change comes from this commit:

https://github.com/torvalds/linux/commit/0d00449c7a28a1514595630735df383dec606812#diff-51ce909c2f65ed9cc668bc36cc3c18528541d8a10e84287874cd37a5918abae5

which later was modified by this commit:

https://github.com/torvalds/linux/commit/8edd7e37aed8b9df938a63f0b0259c70569ce3d2

and this is what we currently have in all kernels since 5.8. Essentially,
KRETPROBES are not working since these commits. We have the following logic:

asm_exc_int3() -> exc_int3():
                    |
    ----------------|
    |
    v
...
nmi_enter();
...
if (!do_int3(regs))
       |
  -----|
  |
  v
do_int3() -> kprobe_int3_handler():
                    |
    ----------------|
    |
    v
...
if (!p->pre_handler || !p->pre_handler(p, regs))
                             |
    -------------------------|
    |
    v
...
pre_handler_kretprobe():
...
    if (unlikely(in_nmi())) {
        rp->nmissed++;
        return 0;
    }

Essentially, exc_int3() calls nmi_enter(), and pre_handler_kretprobe() before
invoking any registered kprobe verifies if it is not in NMI via in_nmi() call.

This bug was masked by OPTIMIZER which was turning most of the KPROBES to be
FTRACE so essentially if someone registered KRETPROBE, buggy code was not
invoked (FTRACE code was executed instead). However, the optimizer was changed
and can't optimize as many functions anymore (probably another bug which might
be worth to investigate) and this bug is more visible.

Thanks,
Adam

-- 
pi3 (pi3ki31ny) - pi3 (at) itsec pl
http://pi3.com.pl

Powered by blists - more mailing lists