Re: KRETPROBES are broken since kernel 5.8

[Masami Hiramatsu]

Hi Adam,

Thank you for reporting and debugging, actually we had investigated this
issue in Aug. Please read this thread.

https://lkml.kernel.org/r/8816bdbbc55c4d2397e0b02aad2825d3@xxxxxxxxxxxxxx

We finally fixed this issue by commit e03b4a084ea6 ("kprobes: Remove NMI
context check") and commit 645f224e7ba2 ("kprobes: Tell lockdep about
kprobe nesting"). Yeah, it will be in the v5.10.

Could you try to test whether these commits can solve the issue?
If it is OK, we should backport those to stable tree.

Thank you,

On Wed, 9 Dec 2020 22:50:01 +0100
Adam Zabrocki <pi3@xxxxxxxxxx> wrote:

> 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
> 


-- 
Masami Hiramatsu <mhiramat@xxxxxxxxxx>