RE: x86/kprobes: kretprobe fails to triggered if kprobe at function entry is not optimized (trigger by int3 breakpoint)

[eddy...@trendmicro.com]

> -Original Message-
> From: pet...@infradead.org 
> Sent: Tuesday, August 25, 2020 8:09 PM
> To: Masami Hiramatsu 
> Cc: Eddy Wu (RD-TW) ; linux-kernel@vger.kernel.org; 
> x...@kernel.org; David S. Miller
> 
> Subject: Re: x86/kprobes: kretprobe fails to triggered if kprobe at function 
> entry is not optimized (trigger by int3 breakpoint)
>
> Surely we can do a lockless list for this. We have llist_add() and
> llist_del_first() to make a lockless LIFO/stack.
>

llist operations require atomic cmpxchg, for some arch doesn't have 
CONFIG_ARCH_HAVE_NMI_SAFE_CMPXCHG, in_nmi() check might still needed.
(HAVE_KRETPROBES && !CONFIG_ARCH_HAVE_NMI_SAFE_CMPXCHG): arc, arm, csky, mips

TREND MICRO EMAIL NOTICE

The information contained in this email and any attachments is confidential and 
may be subject to copyright or other intellectual property protection. If you 
are not the intended recipient, you are not authorized to use or disclose this 
information, and we request that you notify us by reply mail or telephone and 
delete the original message from your mail system.

For details about what personal information we collect and why, please see our 
Privacy Notice on our website at: Read privacy 
policy

RE: x86/kprobes: kretprobe fails to triggered if kprobe at function entry is not optimized (trigger by int3 breakpoint)

[eddy...@trendmicro.com]

> -Original Message-
> From: pet...@infradead.org 
> Sent: Wednesday, August 26, 2020 6:26 PM
> To: Masami Hiramatsu 
> Cc: Eddy Wu (RD-TW) ; linux-kernel@vger.kernel.org; 
> x...@kernel.org
> Subject: Re: x86/kprobes: kretprobe fails to triggered if kprobe at function 
> entry is not optimized (trigger by int3 breakpoint)
>
> On Wed, Aug 26, 2020 at 07:00:41PM +0900, Masami Hiramatsu wrote:
> > Of course, this doesn't solve the llist_del_first() contention in the
> > pre_kretprobe_handler(). So anyway we need a lock for per-probe llist
> > (if I understand llist.h comment correctly.)
>
> Bah, lemme think about that. Kprobes really shouldn't be using locks :/

Maybe we can have per-cpu free list for retprobe_instance?
This ensure we only have one user requesting free instance at a time, given 
that pre_kretprobe_handler() wont recursive.

We may be wasting memory if target function perfer some cpu though.


TREND MICRO EMAIL NOTICE

The information contained in this email and any attachments is confidential and 
may be subject to copyright or other intellectual property protection. If you 
are not the intended recipient, you are not authorized to use or disclose this 
information, and we request that you notify us by reply mail or telephone and 
delete the original message from your mail system.

For details about what personal information we collect and why, please see our 
Privacy Notice on our website at: Read privacy 
policy

RE: [RFC][PATCH 3/7] kprobes: Remove kretprobe hash

[eddy...@trendmicro.com]

> -Original Message-
> From: Peter Zijlstra 
> Sent: Friday, August 28, 2020 12:13 AM
> To: linux-kernel@vger.kernel.org; mhira...@kernel.org
> Cc: Eddy Wu (RD-TW) ; x...@kernel.org; 
> da...@davemloft.net; rost...@goodmis.org;
> naveen.n@linux.ibm.com; anil.s.keshavamur...@intel.com; 
> linux-a...@vger.kernel.org; came...@moodycamel.com;
> o...@redhat.com; w...@kernel.org; paul...@kernel.org; pet...@infradead.org
> Subject: [RFC][PATCH 3/7] kprobes: Remove kretprobe hash
>
> @@ -1935,71 +1932,45 @@ unsigned long __kretprobe_trampoline_han
> unsigned long trampoline_address,
> void *frame_pointer)
>  {
> // ... removed
> // NULL here
> +   first = node = current->kretprobe_instances.first;
> +   while (node) {
> +   ri = container_of(node, struct kretprobe_instance, llist);
>
> -   orig_ret_address = (unsigned long)ri->ret_addr;
> -   if (skipped)
> -   pr_warn("%ps must be blacklisted because of incorrect 
> kretprobe order\n",
> -   ri->rp->kp.addr);
> +   BUG_ON(ri->fp != frame_pointer);
>
> -   if (orig_ret_address != trampoline_address)
> +   orig_ret_address = (unsigned long)ri->ret_addr;
> +   if (orig_ret_address != trampoline_address) {
> /*
>  * This is the real return address. Any other
>  * instances associated with this task are for
>  * other calls deeper on the call stack
>  */
> break;
> +   }
> +
> +   node = node->next;
> }
>

Hi, I found a NULL pointer dereference here, where 
current->kretprobe_instances.first == NULL in these two scenario:

1) In task "rs:main Q:Reg"
# insmod samples/kprobes/kretprobe_example.ko func=schedule
# pkill sddm-greeter

2) In task "llvmpipe-10"
# insmod samples/kprobes/kretprobe_example.ko func=schedule
login plasmashell session from sddm graphical interface

based on Masami's v2 + Peter's lockless patch, I'll try the new branch once I 
can compile kernel

Stacktrace may not be really useful here:
[  402.008630] BUG: kernel NULL pointer dereference, address: 0018
[  402.008633] #PF: supervisor read access in kernel mode
[  402.008642] #PF: error_code(0x) - not-present page
[  402.008644] PGD 0 P4D 0
[  402.008646] Oops:  [#1] PREEMPT SMP PTI
[  402.008649] CPU: 7 PID: 1505 Comm: llvmpipe-10 Kdump: loaded Not tainted 
5.9.0-rc2-00111-g72091ec08f03-dirty #45
[  402.008650] Hardware name: VMware, Inc. VMware Virtual Platform/440BX 
Desktop Reference Platform, BIOS 6.00 07/29/2019
[  402.008653] RIP: 0010:__kretprobe_trampoline_handler+0xb8/0x17f
[  402.008655] Code: 65 4c 8b 34 25 80 6d 01 00 4c 89 e2 48 c7 c7 91 6b 85 91 
49 8d b6 38 07 00 00 e8 d1 1a f9 ff 48 85 db 74 06 48 3b 5d d0 75 16 <49> 8b 75 
18 48 c7 c7 a0 6c 85 91 48
 8b 56 28 e8 b2 1a f9 ff 0f 0b
[  402.008655] RSP: 0018:ab408147bde0 EFLAGS: 00010246
[  402.008656] RAX: 0021 RBX:  RCX: 0002
[  402.008657] RDX: 8002 RSI: 9189757d RDI: 
[  402.008658] RBP: ab408147be20 R08: 0001 R09: 955c
[  402.008658] R10: 0004 R11:  R12: 
[  402.008659] R13:  R14: 90736d305f40 R15: 
[  402.008661] FS:  7f20f6ffd700() GS:9073781c() 
knlGS:
[  402.008675] CS:  0010 DS:  ES:  CR0: 80050033
[  402.008678] CR2: 0018 CR3: 0001ed256006 CR4: 003706e0
[  402.008684] Call Trace:
[  402.008689]  ? elfcorehdr_read+0x40/0x40
[  402.008690]  ? elfcorehdr_read+0x40/0x40
[  402.008691]  trampoline_handler+0x42/0x60
[  402.008692]  kretprobe_trampoline+0x2a/0x50
[  402.008693] RIP: 0010:kretprobe_trampoline+0x0/0x50

TREND MICRO EMAIL NOTICE

The information contained in this email and any attachments is confidential and 
may be subject to copyright or other intellectual property protection. If you 
are not the intended recipient, you are not authorized to use or disclose this 
information, and we request that you notify us by reply mail or telephone and 
delete the original message from your mail system.

For details about what personal information we collect and why, please see our 
Privacy Notice on our website at: Read privacy 
policy

RE: [RFC][PATCH 3/7] kprobes: Remove kretprobe hash

[eddy...@trendmicro.com]

> From: Masami Hiramatsu 
>
> OK, schedule function will be the key. I guess the senario is..
>
> 1) kretporbe replace the return address with kretprobe_trampoline on task1's 
> kernel stack
> 2) the task1 forks task2 before returning to the kretprobe_trampoline
> 3) while copying the process with the kernel stack, 
> task2->kretprobe_instances.first = NULL

I think new process created by fork/clone uses a brand new kernel stack? I 
thought only user stack are copied.
Otherwise any process launch should crash in the same way

By the way, I can reproduce this on the latest branch(v4)
TREND MICRO EMAIL NOTICE

The information contained in this email and any attachments is confidential and 
may be subject to copyright or other intellectual property protection. If you 
are not the intended recipient, you are not authorized to use or disclose this 
information, and we request that you notify us by reply mail or telephone and 
delete the original message from your mail system.

For details about what personal information we collect and why, please see our 
Privacy Notice on our website at: Read privacy 
policy

RE: [PATCH v4 19/23] kprobes: Remove kretprobe hash

[eddy...@trendmicro.com]

> -Original Message-
> From: Masami Hiramatsu 
>
> @@ -1311,24 +1257,23 @@ void kprobe_busy_end(void)
>  void kprobe_flush_task(struct task_struct *tk)
>  {
> struct kretprobe_instance *ri;
> -   struct hlist_head *head;
> -   struct hlist_node *tmp;
> -   unsigned long hash, flags = 0;
> +   struct llist_node *node;
>
> +   /* Early boot, not yet initialized. */
> if (unlikely(!kprobes_initialized))
> -   /* Early boot.  kretprobe_table_locks not yet initialized. */
> return;
>
> kprobe_busy_begin();
>
> -   hash = hash_ptr(tk, KPROBE_HASH_BITS);
> -   head = &kretprobe_inst_table[hash];
> -   kretprobe_table_lock(hash, &flags);
> -   hlist_for_each_entry_safe(ri, tmp, head, hlist) {
> -   if (ri->task == tk)
> -   recycle_rp_inst(ri);
> +   node = current->kretprobe_instances.first;
> +   current->kretprobe_instances.first = NULL;

I think we are flushing tk instead of current here.
After fixing this to tk, the NULL pointer deference is gone!

> +
> +   while (node) {
> +   ri = container_of(node, struct kretprobe_instance, llist);
> +   node = node->next;
> +
> +   recycle_rp_inst(ri);
> }
> -   kretprobe_table_unlock(hash, &flags);
>
> kprobe_busy_end();
>  }

TREND MICRO EMAIL NOTICE

The information contained in this email and any attachments is confidential and 
may be subject to copyright or other intellectual property protection. If you 
are not the intended recipient, you are not authorized to use or disclose this 
information, and we request that you notify us by reply mail or telephone and 
delete the original message from your mail system.

For details about what personal information we collect and why, please see our 
Privacy Notice on our website at: Read privacy 
policy

x86/kprobes: kretprobe fails to triggered if kprobe at function entry is not optimized (trigger by int3 breakpoint)

[eddy...@trendmicro.com]

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).
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: 38ae0211
[48560.229459] kretprobe at 38ae0211 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.

(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.
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?


TREND MICRO EMAIL NOTICE

The information contained in this email and any attachments is confidential and 
may be subject to copyright or other intellectual property protection. If you 
are not the intended recipient, you are not authorized to use or disclose this 
information, and we request that you notify us by reply mail or telephone and 
delete the original message from your mail system.

For details about what personal information we collect and why, please see our 
Privacy Notice on our website at: Read privacy 
policy

RE: x86/kprobes: kretprobe fails to triggered if kprobe at function entry is not optimized (trigger by int3 breakpoint)

[eddy...@trendmicro.com]

> -Original Message-
> From: Peter Zijlstra 
> Sent: Monday, August 24, 2020 10:14 PM
> To: Eddy Wu 
> Cc: Masami Hiramatsu ; linux-kernel@vger.kernel.org; 
> x...@kernel.org; David S. Miller 
> Subject: Re: x86/kprobes: kretprobe fails to triggered if kprobe at function 
> entry is not optimized (trigger by int3 breakpoint)
>
> On Mon, Aug 24, 2020 at 12:02:58PM +, eddy...@trendmicro.com wrote:
> > 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.
> >
> > (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.  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?
>
> Masami, would it be possible to have a kretprobe specific recursion
> count here?
>
> I did the below, but i'm not at all sure that isn't horrible broken. I
> can't really find many rp->lock sites and this might break things by
> limiting contention.
>
> ---
>
> diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h
> index 9be1bff4f586..0bff314cc800 100644
> --- a/include/linux/kprobes.h
> +++ b/include/linux/kprobes.h
> @@ -153,6 +153,7 @@ struct kretprobe {
> size_t data_size;
> struct hlist_head free_instances;
> raw_spinlock_t lock;
> +   atomic_t recursion;
>  };
>
>  struct kretprobe_instance {
> diff --git a/kernel/kprobes.c b/kernel/kprobes.c
> index 287b263c9cb9..27fd096bcb9a 100644
> --- a/kernel/kprobes.c
> +++ b/kernel/kprobes.c
> @@ -1934,22 +1934,17 @@ unsigned long __weak arch_deref_entry_point(void 
> *entry)
>  static int pre_handler_kretprobe(struct kprobe *p, struct pt_regs *regs)
>  {
> struct kretprobe *rp = container_of(p, struct kretprobe, kp);
> -   unsigned long hash, flags = 0;
> struct kretprobe_instance *ri;
> -
> -   /*
> -* To avoid deadlocks, prohibit return probing in NMI contexts,
> -* just skip the probe and increase the (inexact) 'nmissed'
> -* statistical counter, so that the user is informed that
> -* something happened:
> -*/
> -   if (unlikely(in_nmi())) {
> -   rp->nmissed++;
> -   return 0;
> -   }
> +   unsigned long hash, flags;
> +   int rec;
>
> /* TODO: consider to only swap the RA after the last pre_handler 
> fired */
> hash = hash_ptr(current, KPROBE_HASH_BITS);
> +   rec = atomic_fetch_inc_acquire(&rp->recursion);
> +   if (rec) {
> +   rp->nmissed++;
> +   goto out;
> +   }
> raw_spin_lock_irqsave(&rp->lock, flags);
> if (!hlist_empty(&rp->free_instances)) {
> ri = hlist_entry(rp->free_instances.first,
> @@ -1964,7 +1959,7 @@ static int pre_handler_kretprobe(struct kprobe *p, 
> struct pt_regs *regs)
> raw_spin_lock_irqsave(&rp->lock, flags);
> hlist_add_head(&ri->hlist, &rp->free_instances);
> raw_spin_unlock_irqrestore(&rp->lock, flags);
> -   return 0;
> +   goto out;
> }
>
> arch_prepare_kretprobe(ri, regs);
> @@ -1978,6 +1973,8 @@ static int pre_handler_kretprobe(struct kprobe *p, 
> struct pt_regs *regs)
> rp->nmissed++;
> raw_spin_unlock_irqrestore(&rp->lock, flags);
> }
> +out:
> +   atomic_dec(&rp->recursion);
> return 0;
>  }
>  NOKPROBE_SYMBOL(pre_handler_kretprobe);
>
I think kprobe_int3_handler() already prevented pre_handler_kretprobe() from 
recursing, we need to protect critical section in recycle_rp_inst() that might 
be interrupt by NMI.
There is another kretprobe_table_lock has other call site maybe be interrupt by 
NMI too
TREND MICRO EMAIL NOTICE

The information contained in this email and any attachments is confidential and 
may be subject to copyright or other intellectual property protection. If you 
are not the intended recipient, you are not authorized to use or disclose this 
information, and we request that you notify us by reply mail or telephone and 
delete the original message from your mail system.

For details about what personal information we collect and why, please see our 
Privacy Notice on our website at: Read privacy 
policy<http://www.trendmicro.com/privacy>

RE: x86/kprobes: kretprobe fails to triggered if kprobe at function entry is not optimized (trigger by int3 breakpoint)

[eddy...@trendmicro.com]

> -Original Message-
> From: Masami Hiramatsu 
> Sent: Monday, August 24, 2020 11:54 PM
> To: Eddy Wu (RD-TW) 
> Cc: Peter Zijlstra ; linux-kernel@vger.kernel.org; 
> x...@kernel.org; David S. Miller 
> Subject: Re: x86/kprobes: kretprobe fails to triggered if kprobe at function 
> entry is not optimized (trigger by int3 breakpoint)
>
>
> This message was sent from outside of Trend Micro. Please do not click links 
> or open attachments unless you recognise the source of this
> email and know the content is safe.
>
>
> On Mon, 24 Aug 2020 12:02:58 +
> "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: 38ae0211
> > [48560.229459] kretprobe at 38ae0211 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 

Kretprobe might still trigger from NMI with nmi counter == 1 (if entry kprobe 
is jump-optimized).
The arch- dependent weak function looks cleaner than doing this in 
kprobe_int3_handler() under x86/, but I don't know if there is a way to check 
if called by specific int3 handler or not.

My original patch below, need to change all architecture support kretprobe 
though

Thanks

---
 arch/x86/kernel/kprobes/core.c |  6 ++
 include/linux/kprobes.h|  1 +
 kernel/kprobes.c   | 13 +
 3 files changed, 8 insertions(+), 12 deletions(-)

diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c
index fdadc37d72af..1b785aef85ef 100644
--- a/arch/x86/kernel/kprobes/core.c
+++ b/arch/x86/kernel/kprobes/core.c
@@ -699,6 +699,12 @@ int kprobe_int3_handler(struct pt_regs *regs)
 set_current_kprobe(p, regs, kcb);
 kcb->kprobe_status = KPROBE_HIT_ACTIVE;

+if (p->pre_handler == pre_handler_kretprobe && in_nmi() != (1 << NMI_SHIFT)) {
+struct kretprobe *rp = container_of(p, struct kretprobe, kp);
+rp->nmissed++;
+setup_singlestep(p, regs, kcb, 0);
+return 1;
+}
 /*
  * If we have no pre-handler or it returned 0, we
  * continue with normal processing.  

RE: x86/kprobes: kretprobe fails to triggered if kprobe at function entry is not optimized (trigger by int3 breakpoint)

[eddy...@trendmicro.com]

> -Original Message-
> From: Masami Hiramatsu 
> Sent: Tuesday, August 25, 2020 2:16 PM
> To: Eddy Wu (RD-TW) 
> Cc: Peter Zijlstra ; linux-kernel@vger.kernel.org; 
> x...@kernel.org; David S. Miller 
> Subject: Re: x86/kprobes: kretprobe fails to triggered if kprobe at function 
> entry is not optimized (trigger by int3 breakpoint)
>
>
> This message was sent from outside of Trend Micro. Please do not click links 
> or open attachments unless you recognise the source of this
> email and know the content is safe.
>
>
> Hi Eddy,
>
> On Mon, 24 Aug 2020 16:41:58 +
> "eddy...@trendmicro.com"  wrote:
>
> > > -Original Message-
> > > From: Masami Hiramatsu 
> > > Sent: Monday, August 24, 2020 11:54 PM
> > > To: Eddy Wu (RD-TW) 
> > > Cc: Peter Zijlstra ; linux-kernel@vger.kernel.org; 
> > > x...@kernel.org; David S. Miller 
> > > Subject: Re: x86/kprobes: kretprobe fails to triggered if kprobe at 
> > > function entry is not optimized (trigger by int3 breakpoint)
> > >
> > >
> > > This message was sent from outside of Trend Micro. Please do not click 
> > > links or open attachments unless you recognise the source of
> this
> > > email and know the content is safe.
> > >
> > >
> > > On Mon, 24 Aug 2020 12:02:58 +
> > > "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: 38ae0211
> > > > [48560.229459] kretprobe at 38ae0211 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)
> > > {
> > >/*
> > &g

RE: [PATCH v5 00/21] kprobes: Unify kretprobe trampoline handlers and make kretprobe lockless

[eddy...@trendmicro.com]

> From: pet...@infradead.org 
>
> I'm now trying and failing to reproduce I can't seem to make it use
> int3 today. It seems to want to use ftrace or refuses everything. I'm
> probably doing it wrong.
>

You can turn off CONFIG_KPROBES_ON_FTRACE (and also sysctl 
debug.kprobes-optimization) to make it use int3 handler


TREND MICRO EMAIL NOTICE

The information contained in this email and any attachments is confidential and 
may be subject to copyright or other intellectual property protection. If you 
are not the intended recipient, you are not authorized to use or disclose this 
information, and we request that you notify us by reply mail or telephone and 
delete the original message from your mail system.

For details about what personal information we collect and why, please see our 
Privacy Notice on our website at: Read privacy 
policy