[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <0468355f-1d95-d5df-4560-f9220c7a0d05@fb.com>
Date: Thu, 19 May 2022 12:43:18 -0700
From: Yonghong Song <yhs@...com>
To: Eugene Syromiatnikov <esyr@...hat.com>,
Jiri Olsa <jolsa@...nel.org>,
Masami Hiramatsu <mhiramat@...nel.org>,
Steven Rostedt <rostedt@...dmis.org>,
Ingo Molnar <mingo@...hat.com>,
Alexei Starovoitov <ast@...nel.org>,
Daniel Borkmann <daniel@...earbox.net>
Cc: Andrii Nakryiko <andrii@...nel.org>,
Martin KaFai Lau <kafai@...com>,
Song Liu <songliubraving@...com>,
John Fastabend <john.fastabend@...il.com>,
KP Singh <kpsingh@...nel.org>, netdev@...r.kernel.org,
bpf@...r.kernel.org, linux-kernel@...r.kernel.org,
Shuah Khan <shuah@...nel.org>, linux-kselftest@...r.kernel.org
Subject: Re: [PATCH bpf v4 3/3] libbpf, selftests/bpf: pass array of u64
values in kprobe_multi.addrs
On 5/19/22 11:14 AM, Eugene Syromiatnikov wrote:
> With the interface as defined, it is impossible to pass 64-bit kernel
> addresses from a 32-bit userspace process in BPF_LINK_TYPE_KPROBE_MULTI,
> which severly limits the useability of the interface, change the API
> to accept an array of u64 values instead of (kernel? user?) longs.
> This patch implements the user space part of the change (without
> the relevant kernel changes, since, as of now, an attempt to add
> kprobe_multi link will fail with -EOPNOTSUPP), to avoid changing
> the interface after a release.
>
> Fixes: 5117c26e877352bc ("libbpf: Add bpf_link_create support for multi kprobes")
> Fixes: ddc6b04989eb0993 ("libbpf: Add bpf_program__attach_kprobe_multi_opts function")
> Fixes: f7a11eeccb111854 ("selftests/bpf: Add kprobe_multi attach test")
> Fixes: 9271a0c7ae7a9147 ("selftests/bpf: Add attach test for bpf_program__attach_kprobe_multi_opts")
> Fixes: 2c6401c966ae1fbe ("selftests/bpf: Add kprobe_multi bpf_cookie test")
> Signed-off-by: Eugene Syromiatnikov <esyr@...hat.com>
> ---
> tools/lib/bpf/bpf.h | 2 +-
> tools/lib/bpf/libbpf.c | 8 ++++----
> tools/lib/bpf/libbpf.h | 2 +-
> tools/testing/selftests/bpf/prog_tests/bpf_cookie.c | 2 +-
> tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c | 8 ++++----
> 5 files changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h
> index f4b4afb..f677602 100644
> --- a/tools/lib/bpf/bpf.h
> +++ b/tools/lib/bpf/bpf.h
> @@ -417,7 +417,7 @@ struct bpf_link_create_opts {
> __u32 flags;
> __u32 cnt;
> const char **syms;
> - const unsigned long *addrs;
> + const __u64 *addrs;
Patch 2 and 3 will prevent supporting 64-bit kernel, 32-bit userspace
for kprobe_multi. So effectively, kprobe_multi only supports
64-bit kernel and 64-bit user space.
This is definitely an option, but it would be great
if other people can chime in as well for whether this choice
is best or not.
> const __u64 *cookies;
> } kprobe_multi;
> };
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 809fe20..03a14a6 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -10279,7 +10279,7 @@ static bool glob_match(const char *str, const char *pat)
>
> struct kprobe_multi_resolve {
> const char *pattern;
> - unsigned long *addrs;
> + __u64 *addrs;
> size_t cap;
> size_t cnt;
> };
> @@ -10294,12 +10294,12 @@ resolve_kprobe_multi_cb(unsigned long long sym_addr, char sym_type,
> if (!glob_match(sym_name, res->pattern))
> return 0;
>
> - err = libbpf_ensure_mem((void **) &res->addrs, &res->cap, sizeof(unsigned long),
> + err = libbpf_ensure_mem((void **) &res->addrs, &res->cap, sizeof(__u64),
> res->cnt + 1);
> if (err)
> return err;
>
> - res->addrs[res->cnt++] = (unsigned long) sym_addr;
> + res->addrs[res->cnt++] = sym_addr;
> return 0;
> }
>
> @@ -10314,7 +10314,7 @@ bpf_program__attach_kprobe_multi_opts(const struct bpf_program *prog,
> };
> struct bpf_link *link = NULL;
> char errmsg[STRERR_BUFSIZE];
> - const unsigned long *addrs;
> + const __u64 *addrs;
> int err, link_fd, prog_fd;
> const __u64 *cookies;
> const char **syms;
> diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> index 05dde85..ec1cb61 100644
> --- a/tools/lib/bpf/libbpf.h
> +++ b/tools/lib/bpf/libbpf.h
> @@ -431,7 +431,7 @@ struct bpf_kprobe_multi_opts {
> /* array of function symbols to attach */
> const char **syms;
> /* array of function addresses to attach */
> - const unsigned long *addrs;
> + const __u64 *addrs;
> /* array of user-provided values fetchable through bpf_get_attach_cookie */
> const __u64 *cookies;
> /* number of elements in syms/addrs/cookies arrays */
> diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_cookie.c b/tools/testing/selftests/bpf/prog_tests/bpf_cookie.c
> index 923a613..5aa482a 100644
> --- a/tools/testing/selftests/bpf/prog_tests/bpf_cookie.c
> +++ b/tools/testing/selftests/bpf/prog_tests/bpf_cookie.c
> @@ -137,7 +137,7 @@ static void kprobe_multi_link_api_subtest(void)
> cookies[6] = 7;
> cookies[7] = 8;
>
> - opts.kprobe_multi.addrs = (const unsigned long *) &addrs;
> + opts.kprobe_multi.addrs = (const __u64 *) &addrs;
> opts.kprobe_multi.cnt = ARRAY_SIZE(addrs);
> opts.kprobe_multi.cookies = (const __u64 *) &cookies;
> prog_fd = bpf_program__fd(skel->progs.test_kprobe);
> 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 b9876b5..fbf4cf2 100644
> --- a/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
> +++ b/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
> @@ -105,7 +105,7 @@ static void test_link_api_addrs(void)
> GET_ADDR("bpf_fentry_test7", addrs[6]);
> GET_ADDR("bpf_fentry_test8", addrs[7]);
>
> - opts.kprobe_multi.addrs = (const unsigned long*) addrs;
> + opts.kprobe_multi.addrs = (const __u64 *) addrs;
> opts.kprobe_multi.cnt = ARRAY_SIZE(addrs);
> test_link_api(&opts);
> }
> @@ -183,7 +183,7 @@ static void test_attach_api_addrs(void)
> GET_ADDR("bpf_fentry_test7", addrs[6]);
> GET_ADDR("bpf_fentry_test8", addrs[7]);
>
> - opts.addrs = (const unsigned long *) addrs;
> + opts.addrs = (const __u64 *) addrs;
> opts.cnt = ARRAY_SIZE(addrs);
> test_attach_api(NULL, &opts);
> }
> @@ -241,7 +241,7 @@ static void test_attach_api_fails(void)
> goto cleanup;
>
> /* fail_2 - both addrs and syms set */
> - opts.addrs = (const unsigned long *) addrs;
> + opts.addrs = (const __u64 *) addrs;
> opts.syms = syms;
> opts.cnt = ARRAY_SIZE(syms);
> opts.cookies = NULL;
> @@ -255,7 +255,7 @@ static void test_attach_api_fails(void)
> goto cleanup;
>
> /* fail_3 - pattern and addrs set */
> - opts.addrs = (const unsigned long *) addrs;
> + opts.addrs = (const __u64 *) addrs;
> opts.syms = NULL;
> opts.cnt = ARRAY_SIZE(syms);
> opts.cookies = NULL;
Powered by blists - more mailing lists