On Mon, 24 Aug 2020 12:02:58 +0000
"eddy...@trendmicro.com" <eddy...@trendmicro.com> wrote:

> Greetings!
> 
> Starting from kernel 5.8 (x86_64), kretprobe handler will always missed if 
> corresponding kprobe on function entry is not optimized (using break point 
> instead).

Oops, good catch. I always enabled ftrace hook for kretprobe, I didn't noticed 
that.

> Step to reproduce this:
> 1) Build the kretprobe example module (CONFIG_SAMPLE_KRETPROBES=m)
> 2) Disable jump optimization (`sysctl debug.kprobes-optimization=0` or 
> register any kprobe.post_handler at same location)
> 3) Insert the kretprobe_example module
> 4) Launch some process to trigger _do_fork
> 5) Remove kretprobe_example module
> 6) dmesg shows that all probing instances are missed
> 
> Example output:
> # sysctl debug.kprobes-optimization=0
> debug.kprobes-optimization = 0
> # insmod samples/kprobes/kretprobe_example.ko
> # ls > /dev/null
> # rmmod kretprobe_example
> # dmesg
> [48555.067295] Planted return probe at _do_fork: 0000000038ae0211
> [48560.229459] kretprobe at 0000000038ae0211 unregistered
> [48560.229460] Missed probing 3 instances of _do_fork
> 
> After bisecting, I found this behavior seems to introduce by this commit: 
> (5.8-rc1)
> 0d00449c7a28a1514595630735df383dec606812 x86: Replace ist_enter() with 
> nmi_enter()
> This make kprobe_int3_handler() effectively running as NMI context, which 
> pre_handler_kretprobe() explicitly checked to prevent recursion.

Thanks for the bisecting! 

> 
> (in_nmi() check appears from v3.17)
> f96f56780ca584930bb3a2769d73fd9a101bcbbe kprobes: Skip kretprobe hit in NMI 
> context to avoid deadlock
> 
> To make kretprobe work again with int3 breakpoint, I think we can replace the 
> in_nmi() check with in_nmi() == (1 << NMI_SHIFT) at kprobe_int3_handler() and 
> skip kretprobe if nested NMI.

Ah, I see. Now int3 is a kind of NMI, so in the handler in_nmi() always returns 
!0.

> Did a quick test on 5.9-rc2 and it seems to be working.
> I'm not sure if it is the best way to do since it may also require change to 
> other architecture as well, any thought?

Hmm, this behavior is arch-dependent. So I think we need an weak function like 
this.

@kernel/kprobes.c

bool __weak arch_kprobe_in_nmi(void)
{
        return in_nmi()
}

@arch/x86/kernel/kprobes/core.c

bool arch_kprobe_in_nmi(void)
{
       /*
        * Since the int3 is one of NMI, we have to check in_nmi() is
        * bigger than 1 << NMI_SHIFT instead of !0.
        */
       return in_nmi() > (1 << NMI_SHIFT);
}

And use arch_kprobe_in_nmi() instead of in_nmi() in kprobes.c.

Thanks,

-- 
Masami Hiramatsu <mhira...@kernel.org>

Reply via email to