Thread (57 messages) flat view 57 messages, 5 authors, 2024-09-10

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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help