Re: [PATCH v2] tracing/uprobe: Add missing PID filter for uretprobe
From: Jiri Olsa <hidden>
Date: 2024-08-27 13:32:15
Also in:
bpf
On Tue, Aug 27, 2024 at 12:40:52PM +0200, Oleg Nesterov wrote:
quoted hunk ↗ jump to hunk
On 08/27, Jiri Olsa wrote:quoted
On Mon, Aug 26, 2024 at 01:57:52PM +0200, Oleg Nesterov wrote:quoted
So perhaps we need - if (link->task && current->mm != link->task->mm) + if (link->task && !same_thread_group(current, link->task)) in uprobe_prog_run() to make "filter by *process*" true, but this won't fix the problem with link->task->mm == NULL in uprobe_multi_link_filter().would the same_thread_group(current, link->task) work in such case? (zombie leader with other alive threads)Why not? task_struct->signal is stable, it can't be changed. But again, uprobe_multi_link_filter() won't work if the leader, uprobe->link->task, exits or it has already exited. Perhaps something like the additional change below... Oleg.--- a/kernel/trace/bpf_trace.c +++ b/kernel/trace/bpf_trace.c@@ -3322,13 +3322,28 @@ static int uprobe_prog_run(struct bpf_uprobe *uprobe, return err; } + static bool uprobe_multi_link_filter(struct uprobe_consumer *con, struct mm_struct *mm) { struct bpf_uprobe *uprobe; + struct task_struct *task, *t; + bool ret = false; uprobe = container_of(con, struct bpf_uprobe, consumer); - return uprobe->link->task->mm == mm; + task = uprobe->link->task; + + rcu_read_lock(); + for_each_thread(task, t) { + struct mm_struct *mm = READ_ONCE(t->mm); + if (mm) { + ret = t->mm == mm; + break; + } + } + rcu_read_unlock();
that seems expensive if there's many threads could we check the leader first and only if it's gone fallback to this? thanks, jirka