Sun Jian <[email protected]> writes:
> The test installs a kprobe on __sys_connect and checks that
> bpf_probe_write_user() can modify the syscall argument. However, any
> concurrent thread in any other test that calls connect() will also
> trigger the kprobe and have its sockaddr silently overwritten, causing
> flaky failures in unrelated tests.
>
> Constrain the hook to the current test process by filtering on a PID
> stored as a global variable in .bss. Initialize the .bss value from
> user space before bpf_object__load() using bpf_map__set_initial_value(),
> and validate the bss map value size to catch layout mismatches.
>
> No new map is introduced and the test keeps the existing non-skeleton
> flow.
>
> Signed-off-by: Sun Jian <[email protected]>
Acked-by: Mykyta Yatsenko <[email protected]>
> ---
> .../selftests/bpf/prog_tests/probe_user.c | 27 ++++++++++++++++++-
> .../selftests/bpf/progs/test_probe_user.c | 13 +++++++--
> 2 files changed, 37 insertions(+), 3 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/probe_user.c
> b/tools/testing/selftests/bpf/prog_tests/probe_user.c
> index 8721671321de..280dcdb5ddef 100644
> --- a/tools/testing/selftests/bpf/prog_tests/probe_user.c
> +++ b/tools/testing/selftests/bpf/prog_tests/probe_user.c
> @@ -20,6 +20,11 @@ void serial_test_probe_user(void)
> struct bpf_program *kprobe_progs[prog_count];
> struct bpf_object *obj;
> static const int zero = 0;
> + struct test_pro_bss {
> + struct sockaddr_in old;
> + __u32 test_pid;
> + };
> + struct test_pro_bss results = {};
> size_t i;
>
> obj = bpf_object__open_file(obj_file, &opts);
> @@ -34,6 +39,24 @@ void serial_test_probe_user(void)
> goto cleanup;
> }
>
> + {
> + struct bpf_map *bss_map;
> + struct test_pro_bss bss_init = {};
> +
> + bss_init.test_pid = getpid();
> + bss_map = bpf_object__find_map_by_name(obj, "test_pro.bss");
> + if (CHECK(!bss_map, "find_bss_map", "no bss map\n"))
> + goto cleanup;
> + if (CHECK(bpf_map__value_size(bss_map) != sizeof(bss_init),
> + "bss_size", "bss value_size %u != %zu\n",
> + bpf_map__value_size(bss_map), sizeof(bss_init)))
> + goto cleanup;
> + err = bpf_map__set_initial_value(bss_map, &bss_init,
> + sizeof(bss_init));
> + if (CHECK(err, "set_bss_init", "err %d\n", err))
> + goto cleanup;
> + }
> +
> err = bpf_object__load(obj);
> if (CHECK(err, "obj_load", "err %d\n", err))
> goto cleanup;
> @@ -62,11 +85,13 @@ void serial_test_probe_user(void)
> connect(sock_fd, &curr, sizeof(curr));
> close(sock_fd);
>
> - err = bpf_map_lookup_elem(results_map_fd, &zero, &tmp);
> + err = bpf_map_lookup_elem(results_map_fd, &zero, &results);
> if (CHECK(err, "get_kprobe_res",
> "failed to get kprobe res: %d\n", err))
> goto cleanup;
>
> + memcpy(&tmp, &results.old, sizeof(tmp));
> +
> in = (struct sockaddr_in *)&tmp;
> if (CHECK(memcmp(&tmp, &orig, sizeof(orig)), "check_kprobe_res",
> "wrong kprobe res from probe read: %s:%u\n",
> diff --git a/tools/testing/selftests/bpf/progs/test_probe_user.c
> b/tools/testing/selftests/bpf/progs/test_probe_user.c
> index a8e501af9604..4bc86c7654b1 100644
> --- a/tools/testing/selftests/bpf/progs/test_probe_user.c
> +++ b/tools/testing/selftests/bpf/progs/test_probe_user.c
> @@ -5,13 +5,22 @@
> #include <bpf/bpf_core_read.h>
> #include "bpf_misc.h"
>
> -static struct sockaddr_in old;
> +struct test_pro_bss {
> + struct sockaddr_in old;
> + __u32 test_pid;
> +};
> +
> +struct test_pro_bss bss;
>
> static int handle_sys_connect_common(struct sockaddr_in *uservaddr)
> {
> struct sockaddr_in new;
> + __u32 cur = bpf_get_current_pid_tgid() >> 32;
> +
> + if (bss.test_pid && cur != bss.test_pid)
> + return 0;
>
> - bpf_probe_read_user(&old, sizeof(old), uservaddr);
> + bpf_probe_read_user(&bss.old, sizeof(bss.old), uservaddr);
> __builtin_memset(&new, 0xab, sizeof(new));
> bpf_probe_write_user(uservaddr, &new, sizeof(new));
>
> --
> 2.43.0
