On Tue, Aug 27, 2024 at 12:40:52PM +0200, Oleg Nesterov wrote:
> On 08/27, Jiri Olsa wrote:
> >
> > On Mon, Aug 26, 2024 at 01:57:52PM +0200, Oleg Nesterov wrote:
> > >
> > > 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
