From 900f362e2062e4a23a53bf89fd3b248cd021f5af Mon Sep 17 00:00:00 2001 From: Jiri Olsa Date: Thu, 5 Sep 2024 14:51:21 +0300 Subject: [PATCH 1/4] bpf: Fix uprobe multi pid filter check Uprobe multi link does its own process (thread leader) filtering before running the bpf program by comparing task's vm pointers. But as Oleg pointed out there can be processes sharing the vm (CLONE_VM), so we can't just compare task->vm pointers, but instead we need to use same_thread_group call. Suggested-by: Oleg Nesterov Signed-off-by: Jiri Olsa Signed-off-by: Andrii Nakryiko Acked-by: Oleg Nesterov Link: https://lore.kernel.org/bpf/20240905115124.1503998-2-jolsa@kernel.org --- kernel/trace/bpf_trace.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c index b69a39316c0c..98e395f1baae 100644 --- a/kernel/trace/bpf_trace.c +++ b/kernel/trace/bpf_trace.c @@ -3207,7 +3207,7 @@ static int uprobe_prog_run(struct bpf_uprobe *uprobe, struct bpf_run_ctx *old_run_ctx; int err = 0; - if (link->task && current->mm != link->task->mm) + if (link->task && !same_thread_group(current, link->task)) return 0; if (sleepable) From 0b0bb453716f1469105f075044c68ee9670fe4ee Mon Sep 17 00:00:00 2001 From: Jiri Olsa Date: Thu, 5 Sep 2024 14:51:22 +0300 Subject: [PATCH 2/4] selftests/bpf: Add child argument to spawn_child function Adding child argument to spawn_child function to allow to create multiple children in following change. Signed-off-by: Jiri Olsa Signed-off-by: Andrii Nakryiko Link: https://lore.kernel.org/bpf/20240905115124.1503998-3-jolsa@kernel.org --- .../bpf/prog_tests/uprobe_multi_test.c | 85 +++++++++---------- 1 file changed, 39 insertions(+), 46 deletions(-) diff --git a/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c b/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c index acb62675ff65..250eb47c68f9 100644 --- a/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c +++ b/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c @@ -68,29 +68,27 @@ static void kick_child(struct child *child) fflush(NULL); } -static struct child *spawn_child(void) +static int spawn_child(struct child *child) { - static struct child child; - int err; - int c; + int err, c; /* pipe to notify child to execute the trigger functions */ - if (pipe(child.go)) - return NULL; + if (pipe(child->go)) + return -1; - child.pid = child.tid = fork(); - if (child.pid < 0) { - release_child(&child); + child->pid = child->tid = fork(); + if (child->pid < 0) { + release_child(child); errno = EINVAL; - return NULL; + return -1; } /* child */ - if (child.pid == 0) { - close(child.go[1]); + if (child->pid == 0) { + close(child->go[1]); /* wait for parent's kick */ - err = read(child.go[0], &c, 1); + err = read(child->go[0], &c, 1); if (err != 1) exit(err); @@ -102,7 +100,7 @@ static struct child *spawn_child(void) exit(errno); } - return &child; + return 0; } static void *child_thread(void *ctx) @@ -131,39 +129,38 @@ static void *child_thread(void *ctx) pthread_exit(&err); } -static struct child *spawn_thread(void) +static int spawn_thread(struct child *child) { - static struct child child; int c, err; /* pipe to notify child to execute the trigger functions */ - if (pipe(child.go)) - return NULL; + if (pipe(child->go)) + return -1; /* pipe to notify parent that child thread is ready */ - if (pipe(child.c2p)) { - close(child.go[0]); - close(child.go[1]); - return NULL; + if (pipe(child->c2p)) { + close(child->go[0]); + close(child->go[1]); + return -1; } - child.pid = getpid(); + child->pid = getpid(); - err = pthread_create(&child.thread, NULL, child_thread, &child); + err = pthread_create(&child->thread, NULL, child_thread, child); if (err) { err = -errno; - close(child.go[0]); - close(child.go[1]); - close(child.c2p[0]); - close(child.c2p[1]); + close(child->go[0]); + close(child->go[1]); + close(child->c2p[0]); + close(child->c2p[1]); errno = -err; - return NULL; + return -1; } - err = read(child.c2p[0], &c, 1); + err = read(child->c2p[0], &c, 1); if (!ASSERT_EQ(err, 1, "child_thread_ready")) - return NULL; + return -1; - return &child; + return 0; } static void uprobe_multi_test_run(struct uprobe_multi *skel, struct child *child) @@ -304,24 +301,22 @@ __test_attach_api(const char *binary, const char *pattern, struct bpf_uprobe_mul static void test_attach_api(const char *binary, const char *pattern, struct bpf_uprobe_multi_opts *opts) { - struct child *child; + static struct child child; /* no pid filter */ __test_attach_api(binary, pattern, opts, NULL); /* pid filter */ - child = spawn_child(); - if (!ASSERT_OK_PTR(child, "spawn_child")) + if (!ASSERT_OK(spawn_child(&child), "spawn_child")) return; - __test_attach_api(binary, pattern, opts, child); + __test_attach_api(binary, pattern, opts, &child); /* pid filter (thread) */ - child = spawn_thread(); - if (!ASSERT_OK_PTR(child, "spawn_thread")) + if (!ASSERT_OK(spawn_thread(&child), "spawn_thread")) return; - __test_attach_api(binary, pattern, opts, child); + __test_attach_api(binary, pattern, opts, &child); } static void test_attach_api_pattern(void) @@ -712,24 +707,22 @@ static void __test_link_api(struct child *child) static void test_link_api(void) { - struct child *child; + static struct child child; /* no pid filter */ __test_link_api(NULL); /* pid filter */ - child = spawn_child(); - if (!ASSERT_OK_PTR(child, "spawn_child")) + if (!ASSERT_OK(spawn_child(&child), "spawn_child")) return; - __test_link_api(child); + __test_link_api(&child); /* pid filter (thread) */ - child = spawn_thread(); - if (!ASSERT_OK_PTR(child, "spawn_thread")) + if (!ASSERT_OK(spawn_thread(&child), "spawn_thread")) return; - __test_link_api(child); + __test_link_api(&child); } static struct bpf_program * From 8df43e859454952b0dd18c5821b32e6b2d48fddd Mon Sep 17 00:00:00 2001 From: Jiri Olsa Date: Thu, 5 Sep 2024 14:51:23 +0300 Subject: [PATCH 3/4] selftests/bpf: Add uprobe multi pid filter test for fork-ed processes The idea is to create and monitor 3 uprobes, each trigered in separate process and make sure the bpf program gets executed just for the proper PID specified via pid filter. Signed-off-by: Jiri Olsa Signed-off-by: Andrii Nakryiko Link: https://lore.kernel.org/bpf/20240905115124.1503998-4-jolsa@kernel.org --- .../bpf/prog_tests/uprobe_multi_test.c | 67 +++++++++++++++++++ .../bpf/progs/uprobe_multi_pid_filter.c | 40 +++++++++++ 2 files changed, 107 insertions(+) create mode 100644 tools/testing/selftests/bpf/progs/uprobe_multi_pid_filter.c diff --git a/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c b/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c index 250eb47c68f9..9c2f99233304 100644 --- a/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c +++ b/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c @@ -7,6 +7,7 @@ #include "uprobe_multi_bench.skel.h" #include "uprobe_multi_usdt.skel.h" #include "uprobe_multi_consumers.skel.h" +#include "uprobe_multi_pid_filter.skel.h" #include "bpf/libbpf_internal.h" #include "testing_helpers.h" #include "../sdt.h" @@ -935,6 +936,70 @@ static void test_consumers(void) uprobe_multi_consumers__destroy(skel); } +static struct bpf_program *uprobe_multi_program(struct uprobe_multi_pid_filter *skel, int idx) +{ + switch (idx) { + case 0: return skel->progs.uprobe_multi_0; + case 1: return skel->progs.uprobe_multi_1; + case 2: return skel->progs.uprobe_multi_2; + } + return NULL; +} + +#define TASKS 3 + +static void run_pid_filter(struct uprobe_multi_pid_filter *skel, bool retprobe) +{ + LIBBPF_OPTS(bpf_uprobe_multi_opts, opts, .retprobe = retprobe); + struct bpf_link *link[TASKS] = {}; + struct child child[TASKS] = {}; + int i; + + memset(skel->bss->test, 0, sizeof(skel->bss->test)); + + for (i = 0; i < TASKS; i++) { + if (!ASSERT_OK(spawn_child(&child[i]), "spawn_child")) + goto cleanup; + skel->bss->pids[i] = child[i].pid; + } + + for (i = 0; i < TASKS; i++) { + link[i] = bpf_program__attach_uprobe_multi(uprobe_multi_program(skel, i), + child[i].pid, "/proc/self/exe", + "uprobe_multi_func_1", &opts); + if (!ASSERT_OK_PTR(link[i], "bpf_program__attach_uprobe_multi")) + goto cleanup; + } + + for (i = 0; i < TASKS; i++) + kick_child(&child[i]); + + for (i = 0; i < TASKS; i++) { + ASSERT_EQ(skel->bss->test[i][0], 1, "pid"); + ASSERT_EQ(skel->bss->test[i][1], 0, "unknown"); + } + +cleanup: + for (i = 0; i < TASKS; i++) + bpf_link__destroy(link[i]); + for (i = 0; i < TASKS; i++) + release_child(&child[i]); +} + +static void test_pid_filter_process(void) +{ + struct uprobe_multi_pid_filter *skel; + + skel = uprobe_multi_pid_filter__open_and_load(); + if (!ASSERT_OK_PTR(skel, "uprobe_multi_pid_filter__open_and_load")) + return; + + run_pid_filter(skel, false); + run_pid_filter(skel, true); + + uprobe_multi_pid_filter__destroy(skel); +} + static void test_bench_attach_uprobe(void) { long attach_start_ns = 0, attach_end_ns = 0; @@ -1027,4 +1092,6 @@ void test_uprobe_multi_test(void) test_attach_uprobe_fails(); if (test__start_subtest("consumers")) test_consumers(); + if (test__start_subtest("filter_fork")) + test_pid_filter_process(); } diff --git a/tools/testing/selftests/bpf/progs/uprobe_multi_pid_filter.c b/tools/testing/selftests/bpf/progs/uprobe_multi_pid_filter.c new file mode 100644 index 000000000000..67fcbad36661 --- /dev/null +++ b/tools/testing/selftests/bpf/progs/uprobe_multi_pid_filter.c @@ -0,0 +1,40 @@ +// SPDX-License-Identifier: GPL-2.0 +#include "vmlinux.h" +#include +#include + +char _license[] SEC("license") = "GPL"; + +__u32 pids[3]; +__u32 test[3][2]; + +static void update_pid(int idx) +{ + __u32 pid = bpf_get_current_pid_tgid() >> 32; + + if (pid == pids[idx]) + test[idx][0]++; + else + test[idx][1]++; +} + +SEC("uprobe.multi") +int uprobe_multi_0(struct pt_regs *ctx) +{ + update_pid(0); + return 0; +} + +SEC("uprobe.multi") +int uprobe_multi_1(struct pt_regs *ctx) +{ + update_pid(1); + return 0; +} + +SEC("uprobe.multi") +int uprobe_multi_2(struct pt_regs *ctx) +{ + update_pid(2); + return 0; +} From d2520bdb1932fe615d87458ea2df2495bb25c886 Mon Sep 17 00:00:00 2001 From: Jiri Olsa Date: Thu, 5 Sep 2024 14:51:24 +0300 Subject: [PATCH 4/4] selftests/bpf: Add uprobe multi pid filter test for clone-ed processes The idea is to run same test as for test_pid_filter_process, but instead of standard fork-ed process we create the process with clone(CLONE_VM..) to make sure the thread leader process filter works properly in this case. Signed-off-by: Jiri Olsa Signed-off-by: Andrii Nakryiko Link: https://lore.kernel.org/bpf/20240905115124.1503998-5-jolsa@kernel.org --- .../bpf/prog_tests/uprobe_multi_test.c | 66 ++++++++++++------- 1 file changed, 42 insertions(+), 24 deletions(-) diff --git a/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c b/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c index 9c2f99233304..f160d01ba5da 100644 --- a/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c +++ b/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c @@ -40,6 +40,7 @@ struct child { int pid; int tid; pthread_t thread; + char stack[65536]; }; static void release_child(struct child *child) @@ -69,41 +70,56 @@ static void kick_child(struct child *child) fflush(NULL); } -static int spawn_child(struct child *child) +static int child_func(void *arg) { + struct child *child = arg; int err, c; + close(child->go[1]); + + /* wait for parent's kick */ + err = read(child->go[0], &c, 1); + if (err != 1) + exit(err); + + uprobe_multi_func_1(); + uprobe_multi_func_2(); + uprobe_multi_func_3(); + usdt_trigger(); + + exit(errno); +} + +static int spawn_child_flag(struct child *child, bool clone_vm) +{ /* pipe to notify child to execute the trigger functions */ if (pipe(child->go)) return -1; - child->pid = child->tid = fork(); + if (clone_vm) { + child->pid = child->tid = clone(child_func, child->stack + sizeof(child->stack)/2, + CLONE_VM|SIGCHLD, child); + } else { + child->pid = child->tid = fork(); + } if (child->pid < 0) { release_child(child); errno = EINVAL; return -1; } - /* child */ - if (child->pid == 0) { - close(child->go[1]); - - /* wait for parent's kick */ - err = read(child->go[0], &c, 1); - if (err != 1) - exit(err); - - uprobe_multi_func_1(); - uprobe_multi_func_2(); - uprobe_multi_func_3(); - usdt_trigger(); - - exit(errno); - } + /* fork-ed child */ + if (!clone_vm && child->pid == 0) + child_func(child); return 0; } +static int spawn_child(struct child *child) +{ + return spawn_child_flag(child, false); +} + static void *child_thread(void *ctx) { struct child *child = ctx; @@ -948,7 +964,7 @@ static struct bpf_program *uprobe_multi_program(struct uprobe_multi_pid_filter * #define TASKS 3 -static void run_pid_filter(struct uprobe_multi_pid_filter *skel, bool retprobe) +static void run_pid_filter(struct uprobe_multi_pid_filter *skel, bool clone_vm, bool retprobe) { LIBBPF_OPTS(bpf_uprobe_multi_opts, opts, .retprobe = retprobe); struct bpf_link *link[TASKS] = {}; @@ -958,7 +974,7 @@ static void run_pid_filter(struct uprobe_multi_pid_filter *skel, bool retprobe) memset(skel->bss->test, 0, sizeof(skel->bss->test)); for (i = 0; i < TASKS; i++) { - if (!ASSERT_OK(spawn_child(&child[i]), "spawn_child")) + if (!ASSERT_OK(spawn_child_flag(&child[i], clone_vm), "spawn_child")) goto cleanup; skel->bss->pids[i] = child[i].pid; } @@ -986,7 +1002,7 @@ static void run_pid_filter(struct uprobe_multi_pid_filter *skel, bool retprobe) release_child(&child[i]); } -static void test_pid_filter_process(void) +static void test_pid_filter_process(bool clone_vm) { struct uprobe_multi_pid_filter *skel; @@ -994,8 +1010,8 @@ static void test_pid_filter_process(void) if (!ASSERT_OK_PTR(skel, "uprobe_multi_pid_filter__open_and_load")) return; - run_pid_filter(skel, false); - run_pid_filter(skel, true); + run_pid_filter(skel, clone_vm, false); + run_pid_filter(skel, clone_vm, true); uprobe_multi_pid_filter__destroy(skel); } @@ -1093,5 +1109,7 @@ void test_uprobe_multi_test(void) if (test__start_subtest("consumers")) test_consumers(); if (test__start_subtest("filter_fork")) - test_pid_filter_process(); + test_pid_filter_process(false); + if (test__start_subtest("filter_clone_vm")) + test_pid_filter_process(true); }