Forum | Documentation | Website | Blog

Skip to content
Snippets Groups Projects
Commit 46ba0e49 authored by Andrii Nakryiko's avatar Andrii Nakryiko Committed by Alexei Starovoitov
Browse files

bpf: fix multi-uprobe PID filtering logic

Current implementation of PID filtering logic for multi-uprobes in
uprobe_prog_run() is filtering down to exact *thread*, while the intent
for PID filtering it to filter by *process* instead. The check in
uprobe_prog_run() also differs from the analogous one in
uprobe_multi_link_filter() for some reason. The latter is correct,
checking task->mm, not the task itself.

Fix the check in uprobe_prog_run() to perform the same task->mm check.

While doing this, we also update get_pid_task() use to use PIDTYPE_TGID
type of lookup, given the intent is to get a representative task of an
entire process. This doesn't change behavior, but seems more logical. It
would hold task group leader task now, not any random thread task.

Last but not least, given multi-uprobe support is half-broken due to
this PID filtering logic (depending on whether PID filtering is
important or not), we need to make it easy for user space consumers
(including libbpf) to easily detect whether PID fil...
parent 44382b3e
Branches
Tags
No related merge requests found
...@@ -3295,7 +3295,7 @@ static int uprobe_prog_run(struct bpf_uprobe *uprobe, ...@@ -3295,7 +3295,7 @@ static int uprobe_prog_run(struct bpf_uprobe *uprobe,
struct bpf_run_ctx *old_run_ctx; struct bpf_run_ctx *old_run_ctx;
int err = 0; int err = 0;
if (link->task && current != link->task) if (link->task && current->mm != link->task->mm)
return 0; return 0;
if (sleepable) if (sleepable)
...@@ -3396,8 +3396,9 @@ int bpf_uprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr ...@@ -3396,8 +3396,9 @@ int bpf_uprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr
upath = u64_to_user_ptr(attr->link_create.uprobe_multi.path); upath = u64_to_user_ptr(attr->link_create.uprobe_multi.path);
uoffsets = u64_to_user_ptr(attr->link_create.uprobe_multi.offsets); uoffsets = u64_to_user_ptr(attr->link_create.uprobe_multi.offsets);
cnt = attr->link_create.uprobe_multi.cnt; cnt = attr->link_create.uprobe_multi.cnt;
pid = attr->link_create.uprobe_multi.pid;
if (!upath || !uoffsets || !cnt) if (!upath || !uoffsets || !cnt || pid < 0)
return -EINVAL; return -EINVAL;
if (cnt > MAX_UPROBE_MULTI_CNT) if (cnt > MAX_UPROBE_MULTI_CNT)
return -E2BIG; return -E2BIG;
...@@ -3421,10 +3422,9 @@ int bpf_uprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr ...@@ -3421,10 +3422,9 @@ int bpf_uprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr
goto error_path_put; goto error_path_put;
} }
pid = attr->link_create.uprobe_multi.pid;
if (pid) { if (pid) {
rcu_read_lock(); rcu_read_lock();
task = get_pid_task(find_vpid(pid), PIDTYPE_PID); task = get_pid_task(find_vpid(pid), PIDTYPE_TGID);
rcu_read_unlock(); rcu_read_unlock();
if (!task) { if (!task) {
err = -ESRCH; err = -ESRCH;
......
...@@ -397,7 +397,7 @@ static void test_attach_api_fails(void) ...@@ -397,7 +397,7 @@ static void test_attach_api_fails(void)
link_fd = bpf_link_create(prog_fd, 0, BPF_TRACE_UPROBE_MULTI, &opts); link_fd = bpf_link_create(prog_fd, 0, BPF_TRACE_UPROBE_MULTI, &opts);
if (!ASSERT_ERR(link_fd, "link_fd")) if (!ASSERT_ERR(link_fd, "link_fd"))
goto cleanup; goto cleanup;
ASSERT_EQ(link_fd, -ESRCH, "pid_is_wrong"); ASSERT_EQ(link_fd, -EINVAL, "pid_is_wrong");
cleanup: cleanup:
if (link_fd >= 0) if (link_fd >= 0)
......
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment