[RFC PATCH v1 0/1] tracing/uprobe: Add missing filter for uretprobe

[RFC PATCH v1 0/1] tracing/uprobe: Add missing filter for uretprobe

[Tianyi Liu]

U(ret)probes are designed to be filterable using the PID, which is the
second parameter in the perf_event_open syscall. Currently, uprobe works
well with the filtering, but uretprobe is not affected by it. This often
leads to users being disturbed by events from uninterested processes while
using uretprobe.

We found that the filter function was not invoked when uretprobe was
initially implemented, and this has been existing for ten years. We have
tested the patch under our workload, binding eBPF programs to uretprobe
tracepoints, and confirmed that it resolved our problem.

Following are the steps to reproduce the issue:

Step 1. Compile the following reproducer program:
```
#include <stdlib.h>
#include <unistd.h>
#include <stdio.h>

int main() {
    printf("pid: %d\n", getpid());
    while (1) {
        sleep(2);
        void *ptr = malloc(1024);
        free(ptr);
    }
}
```
We will then use uretprobe to trace the `malloc` function.

Step 2. Run two instances of the reproducer program and record their PIDs.

Step 3. Use uretprobe to trace each of the two running reproducers
separately. We use bpftrace to make it easier to reproduce. Please run two
instances of bpftrace simultaneously: the first instance filters events
from PID1, and the second instance filters events from PID2.

The expected behavior is that each bpftrace instance would only print
events matching its respective PID filter. However, in practice, both
bpftrace instances receive events from both processes, the PID filter is
ineffective at this moment:
```
PID1=55256
bpftrace -p $PID1 -e 'uretprobe:libc:malloc { printf("time=%llu pid=%d\n", elapsed / 1000000000, pid); }'
Attaching 1 probe...
time=0 pid=55256
time=2 pid=55273
time=2 pid=55256
time=4 pid=55273
time=4 pid=55256
time=6 pid=55273
time=6 pid=55256

PID2=55273
bpftrace -p $PID2 -e 'uretprobe:libc:malloc { printf("time=%llu pid=%d\n", elapsed / 1000000000, pid); }'
Attaching 1 probe...
time=0 pid=55273
time=0 pid=55256
time=2 pid=55273
time=2 pid=55256
time=4 pid=55273
time=4 pid=55256
time=6 pid=55273
time=6 pid=55256
```

After applying this patch, both bpftrace instances will show the expected
behavior, only printing events from the PID specified by their respective
filters:
```
PID1=1621
bpftrace -p $PID1 -e 'uretprobe:libc:malloc { printf("time=%llu pid=%d\n", elapsed / 1000000000, pid); }'
Attaching 1 probe...
time=0 pid=1621
time=2 pid=1621
time=4 pid=1621
time=6 pid=1621

PID2=1633
bpftrace -p $PID2 -e 'uretprobe:libc:malloc { printf("time=%llu pid=%d\n", elapsed / 1000000000, pid); }'
Attaching 1 probe...
time=0 pid=1633
time=2 pid=1633
time=4 pid=1633
time=6 pid=1633
```

Tianyi Liu (1):
  tracing/uprobe: Add missing filter for uretprobe

 kernel/trace/trace_uprobe.c | 3 +++
 1 file changed, 3 insertions(+)

-- 
2.34.1

[RFC PATCH v1 1/1] tracing/uprobe: Add missing filter for uretprobe

[Tianyi Liu]

U(ret)probes are designed to be filterable using the PID, which is the
second parameter in the perf_event_open syscall. Currently, uprobe works
well with the filtering, but uretprobe is not affected by it. This patch
adds the missing filter for uretprobe to make it work with the PID filter.

Fixes: c1ae5c75e103 ("uprobes/tracing: Introduce is_ret_probe() and uretprobe_dispatcher()")
Cc: Alban Crequy <albancrequy@linux.microsoft.com>
Signed-off-by: Francis Laniel <flaniel@linux.microsoft.com>
Signed-off-by: Tianyi Liu <i.pear@outlook.com>
---
 kernel/trace/trace_uprobe.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
index c98e3b3386ba..c7e2a0962928 100644
--- a/kernel/trace/trace_uprobe.c
+++ b/kernel/trace/trace_uprobe.c
@@ -1443,6 +1443,9 @@ static void uretprobe_perf_func(struct trace_uprobe *tu, unsigned long func,
 				struct pt_regs *regs,
 				struct uprobe_cpu_buffer **ucbp)
 {
+	if (!uprobe_perf_filter(&tu->consumer, 0, current->mm))
+		return;
+
 	__uprobe_perf_func(tu, func, regs, ucbp);
 }
 
-- 
2.34.1

Re: [RFC PATCH v1 0/1] tracing/uprobe: Add missing filter for uretprobe

[Steven Rostedt]

On Fri, 23 Aug 2024 01:36:20 +0800
Tianyi Liu <i.pear@outlook.com> wrote:

> U(ret)probes are designed to be filterable using the PID, which is the
> second parameter in the perf_event_open syscall. Currently, uprobe works
> well with the filtering, but uretprobe is not affected by it. This often
> leads to users being disturbed by events from uninterested processes while
> using uretprobe.
> 
> We found that the filter function was not invoked when uretprobe was
> initially implemented, and this has been existing for ten years. We have
> tested the patch under our workload, binding eBPF programs to uretprobe
> tracepoints, and confirmed that it resolved our problem.
> 
> Following are the steps to reproduce the issue:
> 
> Step 1. Compile the following reproducer program:
> ```
> #include <stdlib.h>
> #include <unistd.h>
> #include <stdio.h>
> 
> int main() {
>     printf("pid: %d\n", getpid());
>     while (1) {
>         sleep(2);
>         void *ptr = malloc(1024);
>         free(ptr);
>     }
> }
> ```
> We will then use uretprobe to trace the `malloc` function.
> 
> Step 2. Run two instances of the reproducer program and record their PIDs.
> 
> Step 3. Use uretprobe to trace each of the two running reproducers
> separately. We use bpftrace to make it easier to reproduce. Please run two
> instances of bpftrace simultaneously: the first instance filters events
> from PID1, and the second instance filters events from PID2.
> 
> The expected behavior is that each bpftrace instance would only print
> events matching its respective PID filter. However, in practice, both
> bpftrace instances receive events from both processes, the PID filter is
> ineffective at this moment:
> ```
> PID1=55256
> bpftrace -p $PID1 -e 'uretprobe:libc:malloc { printf("time=%llu pid=%d\n", elapsed / 1000000000, pid); }'
> Attaching 1 probe...
> time=0 pid=55256
> time=2 pid=55273
> time=2 pid=55256
> time=4 pid=55273
> time=4 pid=55256
> time=6 pid=55273
> time=6 pid=55256
> 
> PID2=55273
> bpftrace -p $PID2 -e 'uretprobe:libc:malloc { printf("time=%llu pid=%d\n", elapsed / 1000000000, pid); }'
> Attaching 1 probe...
> time=0 pid=55273
> time=0 pid=55256
> time=2 pid=55273
> time=2 pid=55256
> time=4 pid=55273
> time=4 pid=55256
> time=6 pid=55273
> time=6 pid=55256
> ```
> 
> After applying this patch, both bpftrace instances will show the expected
> behavior, only printing events from the PID specified by their respective
> filters:
> ```
> PID1=1621
> bpftrace -p $PID1 -e 'uretprobe:libc:malloc { printf("time=%llu pid=%d\n", elapsed / 1000000000, pid); }'
> Attaching 1 probe...
> time=0 pid=1621
> time=2 pid=1621
> time=4 pid=1621
> time=6 pid=1621
> 
> PID2=1633
> bpftrace -p $PID2 -e 'uretprobe:libc:malloc { printf("time=%llu pid=%d\n", elapsed / 1000000000, pid); }'
> Attaching 1 probe...
> time=0 pid=1633
> time=2 pid=1633
> time=4 pid=1633
> time=6 pid=1633
> ```

This did not need a cover letter. The above could be added to the patch
itself.

Just don't say "After applying this patch", it should be:

    
  The expected behavior is that each bpftrace instance would only print
  events matching its respective PID filter. However, in practice, both
  bpftrace instances receive events from both processes, the PID filter is
  ineffective at this moment:

  Before:
  ```
  PID1=55256
  [..]
  time=6 pid=55256
  ```
 
  After: Both bpftrace instances will show the expected behavior, only
  printing events from the PID specified by their respective filters:
  ```
  PID1=1621


-- Steve