On Mon, Apr 22, 2024 at 5:13 AM Jiri Olsa <jolsa@xxxxxxxxxx> wrote:
>
> Adding kprobe multi session test and testing that the entry program
> return value controls execution of the return probe program.
>
> Signed-off-by: Jiri Olsa <jolsa@xxxxxxxxxx>
> ---
> tools/testing/selftests/bpf/bpf_kfuncs.h | 2 +
> .../bpf/prog_tests/kprobe_multi_test.c | 49 +++++++++
> .../bpf/progs/kprobe_multi_session.c | 100 ++++++++++++++++++
> 3 files changed, 151 insertions(+)
> create mode 100644 tools/testing/selftests/bpf/progs/kprobe_multi_session.c
>
> diff --git a/tools/testing/selftests/bpf/bpf_kfuncs.h b/tools/testing/selftests/bpf/bpf_kfuncs.h
> index 14ebe7d9e1a3..180030b5d828 100644
> --- a/tools/testing/selftests/bpf/bpf_kfuncs.h
> +++ b/tools/testing/selftests/bpf/bpf_kfuncs.h
> @@ -75,4 +75,6 @@ extern void bpf_key_put(struct bpf_key *key) __ksym;
> extern int bpf_verify_pkcs7_signature(struct bpf_dynptr *data_ptr,
> struct bpf_dynptr *sig_ptr,
> struct bpf_key *trusted_keyring) __ksym;
> +
> +extern bool bpf_session_is_return(void) __ksym;
> #endif
> diff --git a/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c b/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
> index 51628455b6f5..d1f116665551 100644
> --- a/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
> +++ b/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
> @@ -4,6 +4,7 @@
> #include "trace_helpers.h"
> #include "kprobe_multi_empty.skel.h"
> #include "kprobe_multi_override.skel.h"
> +#include "kprobe_multi_session.skel.h"
> #include "bpf/libbpf_internal.h"
> #include "bpf/hashmap.h"
>
> @@ -326,6 +327,52 @@ static void test_attach_api_fails(void)
> kprobe_multi__destroy(skel);
> }
>
> +static void test_session_skel_api(void)
> +{
> + struct kprobe_multi_session *skel = NULL;
> + LIBBPF_OPTS(bpf_kprobe_multi_opts, opts);
> + LIBBPF_OPTS(bpf_test_run_opts, topts);
> + struct bpf_link *link = NULL;
> + int err, prog_fd;
> +
> + skel = kprobe_multi_session__open_and_load();
> + if (!ASSERT_OK_PTR(skel, "kprobe_multi_session__open_and_load"))
> + goto cleanup;
return?
> +
> + skel->bss->pid = getpid();
> +
> + err = kprobe_multi_session__attach(skel);
nit: extra space
> + if (!ASSERT_OK(err, " kprobe_multi_session__attach"))
> + goto cleanup;
> +
> + prog_fd = bpf_program__fd(skel->progs.trigger);
> + err = bpf_prog_test_run_opts(prog_fd, &topts);
> + ASSERT_OK(err, "test_run");
> + ASSERT_EQ(topts.retval, 0, "test_run");
> +
> + ASSERT_EQ(skel->bss->kprobe_test1_result, 1, "kprobe_test1_result");
> + ASSERT_EQ(skel->bss->kprobe_test2_result, 1, "kprobe_test2_result");
> + ASSERT_EQ(skel->bss->kprobe_test3_result, 1, "kprobe_test3_result");
> + ASSERT_EQ(skel->bss->kprobe_test4_result, 1, "kprobe_test4_result");
> + ASSERT_EQ(skel->bss->kprobe_test5_result, 1, "kprobe_test5_result");
> + ASSERT_EQ(skel->bss->kprobe_test6_result, 1, "kprobe_test6_result");
> + ASSERT_EQ(skel->bss->kprobe_test7_result, 1, "kprobe_test7_result");
> + ASSERT_EQ(skel->bss->kprobe_test8_result, 1, "kprobe_test8_result");
> +
> + ASSERT_EQ(skel->bss->kretprobe_test1_result, 0, "kretprobe_test1_result");
> + ASSERT_EQ(skel->bss->kretprobe_test2_result, 1, "kretprobe_test2_result");
> + ASSERT_EQ(skel->bss->kretprobe_test3_result, 0, "kretprobe_test3_result");
> + ASSERT_EQ(skel->bss->kretprobe_test4_result, 1, "kretprobe_test4_result");
> + ASSERT_EQ(skel->bss->kretprobe_test5_result, 0, "kretprobe_test5_result");
> + ASSERT_EQ(skel->bss->kretprobe_test6_result, 1, "kretprobe_test6_result");
> + ASSERT_EQ(skel->bss->kretprobe_test7_result, 0, "kretprobe_test7_result");
> + ASSERT_EQ(skel->bss->kretprobe_test8_result, 1, "kretprobe_test8_result");
see below, even if array of ksym ptrs idea doesn't work out, at least
results can be an array (which is cleaner to work with both on BPF and
user space sides)
> +
> +cleanup:
> + bpf_link__destroy(link);
> + kprobe_multi_session__destroy(skel);
> +}
> +
> static size_t symbol_hash(long key, void *ctx __maybe_unused)
> {
> return str_hash((const char *) key);
> @@ -690,4 +737,6 @@ void test_kprobe_multi_test(void)
> test_attach_api_fails();
> if (test__start_subtest("attach_override"))
> test_attach_override();
> + if (test__start_subtest("session"))
> + test_session_skel_api();
> }
> diff --git a/tools/testing/selftests/bpf/progs/kprobe_multi_session.c b/tools/testing/selftests/bpf/progs/kprobe_multi_session.c
> new file mode 100644
> index 000000000000..9fb87f7f69da
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/kprobe_multi_session.c
> @@ -0,0 +1,100 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include <linux/bpf.h>
> +#include <bpf/bpf_helpers.h>
> +#include <bpf/bpf_tracing.h>
> +#include <stdbool.h>
> +#include "bpf_kfuncs.h"
> +
> +char _license[] SEC("license") = "GPL";
> +
> +extern const void bpf_fentry_test1 __ksym;
> +extern const void bpf_fentry_test2 __ksym;
> +extern const void bpf_fentry_test3 __ksym;
> +extern const void bpf_fentry_test4 __ksym;
> +extern const void bpf_fentry_test5 __ksym;
> +extern const void bpf_fentry_test6 __ksym;
> +extern const void bpf_fentry_test7 __ksym;
> +extern const void bpf_fentry_test8 __ksym;
> +
> +int pid = 0;
> +
> +__u64 kprobe_test1_result = 0;
> +__u64 kprobe_test2_result = 0;
> +__u64 kprobe_test3_result = 0;
> +__u64 kprobe_test4_result = 0;
> +__u64 kprobe_test5_result = 0;
> +__u64 kprobe_test6_result = 0;
> +__u64 kprobe_test7_result = 0;
> +__u64 kprobe_test8_result = 0;
> +
> +__u64 kretprobe_test1_result = 0;
> +__u64 kretprobe_test2_result = 0;
> +__u64 kretprobe_test3_result = 0;
> +__u64 kretprobe_test4_result = 0;
> +__u64 kretprobe_test5_result = 0;
> +__u64 kretprobe_test6_result = 0;
> +__u64 kretprobe_test7_result = 0;
> +__u64 kretprobe_test8_result = 0;
> +
> +static int session_check(void *ctx, bool is_return)
> +{
> + if (bpf_get_current_pid_tgid() >> 32 != pid)
> + return 1;
> +
> + __u64 addr = bpf_get_func_ip(ctx);
> +
> +#define SET(__var, __addr) ({ \
> + if ((const void *) addr == __addr) \
> + __var = 1; \
> +})
> +
> + if (is_return) {
> + SET(kretprobe_test1_result, &bpf_fentry_test1);
> + SET(kretprobe_test2_result, &bpf_fentry_test2);
> + SET(kretprobe_test3_result, &bpf_fentry_test3);
> + SET(kretprobe_test4_result, &bpf_fentry_test4);
> + SET(kretprobe_test5_result, &bpf_fentry_test5);
> + SET(kretprobe_test6_result, &bpf_fentry_test6);
> + SET(kretprobe_test7_result, &bpf_fentry_test7);
> + SET(kretprobe_test8_result, &bpf_fentry_test8);
> + } else {
> + SET(kprobe_test1_result, &bpf_fentry_test1);
> + SET(kprobe_test2_result, &bpf_fentry_test2);
> + SET(kprobe_test3_result, &bpf_fentry_test3);
> + SET(kprobe_test4_result, &bpf_fentry_test4);
> + SET(kprobe_test5_result, &bpf_fentry_test5);
> + SET(kprobe_test6_result, &bpf_fentry_test6);
> + SET(kprobe_test7_result, &bpf_fentry_test7);
> + SET(kprobe_test8_result, &bpf_fentry_test8);
> + }
> +
> +#undef SET
curious, have you tried implementing this through a proper for loop? I
wonder if something like
void *kfuncs[] = { &bpf_fentry_test1, ..., &bpf_fentry_test8 };
and then generic loop over this array would work. Can you please try?
> +
> + /*
> + * Force probes for function bpf_fentry_test[1357] not to
> + * install and execute the return probe
> + */
> + if (((const void *) addr == &bpf_fentry_test1) ||
> + ((const void *) addr == &bpf_fentry_test3) ||
> + ((const void *) addr == &bpf_fentry_test5) ||
> + ((const void *) addr == &bpf_fentry_test7))
> + return 1;
> +
> + return 0;
> +}
> +
> +/*
> + * No tests in here, just to trigger 'bpf_fentry_test*'
> + * through tracing test_run
> + */
> +SEC("fentry/bpf_modify_return_test")
> +int BPF_PROG(trigger)
> +{
> + return 0;
> +}
> +
> +SEC("kprobe.session/bpf_fentry_test*")
> +int test_kprobe(struct pt_regs *ctx)
> +{
> + return session_check(ctx, bpf_session_is_return());
> +}
> --
> 2.44.0
>
