* [PATCH bpf-next 1/2] libbpf: Add unique_match option for multi kprobe
@ 2024-12-18 22:52 Yonghong Song
2024-12-18 22:52 ` [PATCH bpf-next 2/2] selftests/bpf: Add a test for kprobe multi with unique_match Yonghong Song
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Yonghong Song @ 2024-12-18 22:52 UTC (permalink / raw)
To: bpf
Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann, kernel-team,
Martin KaFai Lau, Jordan Rome
Jordan reported an issue in Meta production environment where func
try_to_wake_up() is renamed to try_to_wake_up.llvm.<hash>() by clang
compiler at lto mode. The original 'kprobe/try_to_wake_up' does not
work any more since try_to_wake_up() does not match the actual func
name in /proc/kallsyms.
There are a couple of ways to resolve this issue. For example, in
attach_kprobe(), we could do lookup in /proc/kallsyms so try_to_wake_up()
can be replaced by try_to_wake_up.llvm.<hach>(). Or we can force users
to use bpf_program__attach_kprobe() where they need to lookup
/proc/kallsyms to find out try_to_wake_up.llvm.<hach>(). But these two
approaches requires extra work by either libbpf or user.
Luckily, suggested by Andrii, multi kprobe already supports wildcard ('*')
for symbol matching. In the above example, 'try_to_wake_up*' can match
to try_to_wake_up() or try_to_wake_up.llvm.<hash>() and this allows
bpf prog works for different kernels as some kernels may have
try_to_wake_up() and some others may have try_to_wake_up.llvm.<hash>().
The original intention is to kprobe try_to_wake_up() only, so an optional
field unique_match is added to struct bpf_kprobe_multi_opts. If the
field is set to true, the number of matched functions must be one.
Otherwise, the attachment will fail. In the above case, multi kprobe
with 'try_to_wake_up*' and unique_match preserves user functionality.
Reported-by: Jordan Rome <linux@jordanrome.com>
Suggested-by: Andrii Nakryiko <andrii@kernel.org>
Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
---
tools/lib/bpf/libbpf.c | 10 +++++++++-
tools/lib/bpf/libbpf.h | 4 +++-
2 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 66173ddb5a2d..649c6e92972a 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -11522,7 +11522,7 @@ bpf_program__attach_kprobe_multi_opts(const struct bpf_program *prog,
struct bpf_link *link = NULL;
const unsigned long *addrs;
int err, link_fd, prog_fd;
- bool retprobe, session;
+ bool retprobe, session, unique_match;
const __u64 *cookies;
const char **syms;
size_t cnt;
@@ -11558,6 +11558,14 @@ bpf_program__attach_kprobe_multi_opts(const struct bpf_program *prog,
err = libbpf_available_kallsyms_parse(&res);
if (err)
goto error;
+
+ unique_match = OPTS_GET(opts, unique_match, false);
+ if (unique_match && res.cnt != 1) {
+ pr_warn("prog '%s': failed to find unique match: cnt %lu\n",
+ prog->name, res.cnt);
+ return libbpf_err_ptr(-EINVAL);
+ }
+
addrs = res.addrs;
cnt = res.cnt;
}
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index d45807103565..3020ee45303a 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -552,10 +552,12 @@ struct bpf_kprobe_multi_opts {
bool retprobe;
/* create session kprobes */
bool session;
+ /* enforce unique match */
+ bool unique_match;
size_t :0;
};
-#define bpf_kprobe_multi_opts__last_field session
+#define bpf_kprobe_multi_opts__last_field unique_match
LIBBPF_API struct bpf_link *
bpf_program__attach_kprobe_multi_opts(const struct bpf_program *prog,
--
2.43.5
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH bpf-next 2/2] selftests/bpf: Add a test for kprobe multi with unique_match
2024-12-18 22:52 [PATCH bpf-next 1/2] libbpf: Add unique_match option for multi kprobe Yonghong Song
@ 2024-12-18 22:52 ` Yonghong Song
2024-12-18 23:12 ` [PATCH bpf-next 1/2] libbpf: Add unique_match option for multi kprobe Jordan Rome
2025-01-07 0:24 ` Andrii Nakryiko
2 siblings, 0 replies; 6+ messages in thread
From: Yonghong Song @ 2024-12-18 22:52 UTC (permalink / raw)
To: bpf
Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann, kernel-team,
Martin KaFai Lau
Add a kprobe multi subtest to test kprobe multi unique_match option.
Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
---
.../bpf/prog_tests/kprobe_multi_test.c | 27 +++++++++++++++++++
1 file changed, 27 insertions(+)
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 66ab1cae923e..e19ef509ebf8 100644
--- a/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
+++ b/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
@@ -397,6 +397,31 @@ static void test_session_cookie_skel_api(void)
kprobe_multi_session_cookie__destroy(skel);
}
+static void test_unique_match(void)
+{
+ LIBBPF_OPTS(bpf_kprobe_multi_opts, opts);
+ struct kprobe_multi *skel = NULL;
+ struct bpf_link *link = NULL;
+
+ skel = kprobe_multi__open_and_load();
+ if (!ASSERT_OK_PTR(skel, "kprobe_multi__open_and_load"))
+ return;
+
+ opts.unique_match = true;
+ skel->bss->pid = getpid();
+ link = bpf_program__attach_kprobe_multi_opts(skel->progs.test_kprobe_manual,
+ "bpf_fentry_test*", &opts);
+ if (!ASSERT_ERR_PTR(link, "bpf_program__attach_kprobe_multi_opts"))
+ bpf_link__destroy(link);
+
+ link = bpf_program__attach_kprobe_multi_opts(skel->progs.test_kprobe_manual,
+ "bpf_fentry_test8*", &opts);
+ if (ASSERT_OK_PTR(link, "bpf_program__attach_kprobe_multi_opts"))
+ bpf_link__destroy(link);
+
+ kprobe_multi__destroy(skel);
+}
+
static size_t symbol_hash(long key, void *ctx __maybe_unused)
{
return str_hash((const char *) key);
@@ -765,5 +790,7 @@ void test_kprobe_multi_test(void)
test_session_skel_api();
if (test__start_subtest("session_cookie"))
test_session_cookie_skel_api();
+ if (test__start_subtest("unique_match"))
+ test_unique_match();
RUN_TESTS(kprobe_multi_verifier);
}
--
2.43.5
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH bpf-next 1/2] libbpf: Add unique_match option for multi kprobe
2024-12-18 22:52 [PATCH bpf-next 1/2] libbpf: Add unique_match option for multi kprobe Yonghong Song
2024-12-18 22:52 ` [PATCH bpf-next 2/2] selftests/bpf: Add a test for kprobe multi with unique_match Yonghong Song
@ 2024-12-18 23:12 ` Jordan Rome
2025-01-07 0:24 ` Andrii Nakryiko
2 siblings, 0 replies; 6+ messages in thread
From: Jordan Rome @ 2024-12-18 23:12 UTC (permalink / raw)
To: Yonghong Song
Cc: bpf, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
kernel-team, Martin KaFai Lau
On Wed, Dec 18, 2024 at 5:53 PM Yonghong Song <yonghong.song@linux.dev> wrote:
>
> Jordan reported an issue in Meta production environment where func
> try_to_wake_up() is renamed to try_to_wake_up.llvm.<hash>() by clang
> compiler at lto mode. The original 'kprobe/try_to_wake_up' does not
> work any more since try_to_wake_up() does not match the actual func
> name in /proc/kallsyms.
>
> There are a couple of ways to resolve this issue. For example, in
> attach_kprobe(), we could do lookup in /proc/kallsyms so try_to_wake_up()
> can be replaced by try_to_wake_up.llvm.<hach>(). Or we can force users
> to use bpf_program__attach_kprobe() where they need to lookup
> /proc/kallsyms to find out try_to_wake_up.llvm.<hach>(). But these two
> approaches requires extra work by either libbpf or user.
>
> Luckily, suggested by Andrii, multi kprobe already supports wildcard ('*')
> for symbol matching. In the above example, 'try_to_wake_up*' can match
> to try_to_wake_up() or try_to_wake_up.llvm.<hash>() and this allows
> bpf prog works for different kernels as some kernels may have
> try_to_wake_up() and some others may have try_to_wake_up.llvm.<hash>().
>
> The original intention is to kprobe try_to_wake_up() only, so an optional
> field unique_match is added to struct bpf_kprobe_multi_opts. If the
> field is set to true, the number of matched functions must be one.
> Otherwise, the attachment will fail. In the above case, multi kprobe
> with 'try_to_wake_up*' and unique_match preserves user functionality.
>
> Reported-by: Jordan Rome <linux@jordanrome.com>
> Suggested-by: Andrii Nakryiko <andrii@kernel.org>
> Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
> ---
> tools/lib/bpf/libbpf.c | 10 +++++++++-
> tools/lib/bpf/libbpf.h | 4 +++-
> 2 files changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 66173ddb5a2d..649c6e92972a 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -11522,7 +11522,7 @@ bpf_program__attach_kprobe_multi_opts(const struct bpf_program *prog,
> struct bpf_link *link = NULL;
> const unsigned long *addrs;
> int err, link_fd, prog_fd;
> - bool retprobe, session;
> + bool retprobe, session, unique_match;
> const __u64 *cookies;
> const char **syms;
> size_t cnt;
> @@ -11558,6 +11558,14 @@ bpf_program__attach_kprobe_multi_opts(const struct bpf_program *prog,
> err = libbpf_available_kallsyms_parse(&res);
> if (err)
> goto error;
> +
> + unique_match = OPTS_GET(opts, unique_match, false);
> + if (unique_match && res.cnt != 1) {
> + pr_warn("prog '%s': failed to find unique match: cnt %lu\n",
> + prog->name, res.cnt);
nit: "failed to find a unique match. Num matches: %lu\n"
or "Failed to find a unique match for %s'. Num matches: %lu\n"
> + return libbpf_err_ptr(-EINVAL);
> + }
> +
> addrs = res.addrs;
> cnt = res.cnt;
> }
> diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> index d45807103565..3020ee45303a 100644
> --- a/tools/lib/bpf/libbpf.h
> +++ b/tools/lib/bpf/libbpf.h
> @@ -552,10 +552,12 @@ struct bpf_kprobe_multi_opts {
> bool retprobe;
> /* create session kprobes */
> bool session;
> + /* enforce unique match */
> + bool unique_match;
> size_t :0;
> };
>
> -#define bpf_kprobe_multi_opts__last_field session
> +#define bpf_kprobe_multi_opts__last_field unique_match
>
> LIBBPF_API struct bpf_link *
> bpf_program__attach_kprobe_multi_opts(const struct bpf_program *prog,
> --
> 2.43.5
>
Ack. Thanks for the quick work, Yonghong!
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bpf-next 1/2] libbpf: Add unique_match option for multi kprobe
2024-12-18 22:52 [PATCH bpf-next 1/2] libbpf: Add unique_match option for multi kprobe Yonghong Song
2024-12-18 22:52 ` [PATCH bpf-next 2/2] selftests/bpf: Add a test for kprobe multi with unique_match Yonghong Song
2024-12-18 23:12 ` [PATCH bpf-next 1/2] libbpf: Add unique_match option for multi kprobe Jordan Rome
@ 2025-01-07 0:24 ` Andrii Nakryiko
2025-01-07 18:29 ` Yonghong Song
2 siblings, 1 reply; 6+ messages in thread
From: Andrii Nakryiko @ 2025-01-07 0:24 UTC (permalink / raw)
To: Yonghong Song
Cc: bpf, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
kernel-team, Martin KaFai Lau, Jordan Rome
On Wed, Dec 18, 2024 at 2:53 PM Yonghong Song <yonghong.song@linux.dev> wrote:
>
> Jordan reported an issue in Meta production environment where func
> try_to_wake_up() is renamed to try_to_wake_up.llvm.<hash>() by clang
> compiler at lto mode. The original 'kprobe/try_to_wake_up' does not
> work any more since try_to_wake_up() does not match the actual func
> name in /proc/kallsyms.
>
> There are a couple of ways to resolve this issue. For example, in
> attach_kprobe(), we could do lookup in /proc/kallsyms so try_to_wake_up()
> can be replaced by try_to_wake_up.llvm.<hach>(). Or we can force users
> to use bpf_program__attach_kprobe() where they need to lookup
> /proc/kallsyms to find out try_to_wake_up.llvm.<hach>(). But these two
> approaches requires extra work by either libbpf or user.
>
> Luckily, suggested by Andrii, multi kprobe already supports wildcard ('*')
> for symbol matching. In the above example, 'try_to_wake_up*' can match
> to try_to_wake_up() or try_to_wake_up.llvm.<hash>() and this allows
> bpf prog works for different kernels as some kernels may have
> try_to_wake_up() and some others may have try_to_wake_up.llvm.<hash>().
>
> The original intention is to kprobe try_to_wake_up() only, so an optional
> field unique_match is added to struct bpf_kprobe_multi_opts. If the
> field is set to true, the number of matched functions must be one.
> Otherwise, the attachment will fail. In the above case, multi kprobe
> with 'try_to_wake_up*' and unique_match preserves user functionality.
>
> Reported-by: Jordan Rome <linux@jordanrome.com>
> Suggested-by: Andrii Nakryiko <andrii@kernel.org>
> Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
> ---
> tools/lib/bpf/libbpf.c | 10 +++++++++-
> tools/lib/bpf/libbpf.h | 4 +++-
> 2 files changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 66173ddb5a2d..649c6e92972a 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -11522,7 +11522,7 @@ bpf_program__attach_kprobe_multi_opts(const struct bpf_program *prog,
> struct bpf_link *link = NULL;
> const unsigned long *addrs;
> int err, link_fd, prog_fd;
> - bool retprobe, session;
> + bool retprobe, session, unique_match;
> const __u64 *cookies;
> const char **syms;
> size_t cnt;
> @@ -11558,6 +11558,14 @@ bpf_program__attach_kprobe_multi_opts(const struct bpf_program *prog,
> err = libbpf_available_kallsyms_parse(&res);
> if (err)
> goto error;
> +
> + unique_match = OPTS_GET(opts, unique_match, false);
> + if (unique_match && res.cnt != 1) {
> + pr_warn("prog '%s': failed to find unique match: cnt %lu\n",
> + prog->name, res.cnt);
> + return libbpf_err_ptr(-EINVAL);
goto error, leaking resources here
we should also think about interaction of unique_match interaction for
!pattern case, and either reject it (if it makes no sense), or enforce
it (if it does, I haven't really thought about which case do we have)
pw-bot: cr
> + }
> +
> addrs = res.addrs;
> cnt = res.cnt;
> }
> diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> index d45807103565..3020ee45303a 100644
> --- a/tools/lib/bpf/libbpf.h
> +++ b/tools/lib/bpf/libbpf.h
> @@ -552,10 +552,12 @@ struct bpf_kprobe_multi_opts {
> bool retprobe;
> /* create session kprobes */
> bool session;
> + /* enforce unique match */
> + bool unique_match;
> size_t :0;
> };
>
> -#define bpf_kprobe_multi_opts__last_field session
> +#define bpf_kprobe_multi_opts__last_field unique_match
>
> LIBBPF_API struct bpf_link *
> bpf_program__attach_kprobe_multi_opts(const struct bpf_program *prog,
> --
> 2.43.5
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bpf-next 1/2] libbpf: Add unique_match option for multi kprobe
2025-01-07 0:24 ` Andrii Nakryiko
@ 2025-01-07 18:29 ` Yonghong Song
2025-01-08 22:38 ` Andrii Nakryiko
0 siblings, 1 reply; 6+ messages in thread
From: Yonghong Song @ 2025-01-07 18:29 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: bpf, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
kernel-team, Martin KaFai Lau, Jordan Rome
On 1/6/25 4:24 PM, Andrii Nakryiko wrote:
> On Wed, Dec 18, 2024 at 2:53 PM Yonghong Song <yonghong.song@linux.dev> wrote:
>> Jordan reported an issue in Meta production environment where func
>> try_to_wake_up() is renamed to try_to_wake_up.llvm.<hash>() by clang
>> compiler at lto mode. The original 'kprobe/try_to_wake_up' does not
>> work any more since try_to_wake_up() does not match the actual func
>> name in /proc/kallsyms.
>>
>> There are a couple of ways to resolve this issue. For example, in
>> attach_kprobe(), we could do lookup in /proc/kallsyms so try_to_wake_up()
>> can be replaced by try_to_wake_up.llvm.<hach>(). Or we can force users
>> to use bpf_program__attach_kprobe() where they need to lookup
>> /proc/kallsyms to find out try_to_wake_up.llvm.<hach>(). But these two
>> approaches requires extra work by either libbpf or user.
>>
>> Luckily, suggested by Andrii, multi kprobe already supports wildcard ('*')
>> for symbol matching. In the above example, 'try_to_wake_up*' can match
>> to try_to_wake_up() or try_to_wake_up.llvm.<hash>() and this allows
>> bpf prog works for different kernels as some kernels may have
>> try_to_wake_up() and some others may have try_to_wake_up.llvm.<hash>().
>>
>> The original intention is to kprobe try_to_wake_up() only, so an optional
>> field unique_match is added to struct bpf_kprobe_multi_opts. If the
>> field is set to true, the number of matched functions must be one.
>> Otherwise, the attachment will fail. In the above case, multi kprobe
>> with 'try_to_wake_up*' and unique_match preserves user functionality.
>>
>> Reported-by: Jordan Rome <linux@jordanrome.com>
>> Suggested-by: Andrii Nakryiko <andrii@kernel.org>
>> Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
>> ---
>> tools/lib/bpf/libbpf.c | 10 +++++++++-
>> tools/lib/bpf/libbpf.h | 4 +++-
>> 2 files changed, 12 insertions(+), 2 deletions(-)
>>
>> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
>> index 66173ddb5a2d..649c6e92972a 100644
>> --- a/tools/lib/bpf/libbpf.c
>> +++ b/tools/lib/bpf/libbpf.c
>> @@ -11522,7 +11522,7 @@ bpf_program__attach_kprobe_multi_opts(const struct bpf_program *prog,
>> struct bpf_link *link = NULL;
>> const unsigned long *addrs;
>> int err, link_fd, prog_fd;
>> - bool retprobe, session;
>> + bool retprobe, session, unique_match;
>> const __u64 *cookies;
>> const char **syms;
>> size_t cnt;
>> @@ -11558,6 +11558,14 @@ bpf_program__attach_kprobe_multi_opts(const struct bpf_program *prog,
>> err = libbpf_available_kallsyms_parse(&res);
>> if (err)
>> goto error;
>> +
>> + unique_match = OPTS_GET(opts, unique_match, false);
>> + if (unique_match && res.cnt != 1) {
>> + pr_warn("prog '%s': failed to find unique match: cnt %lu\n",
>> + prog->name, res.cnt);
>> + return libbpf_err_ptr(-EINVAL);
> goto error, leaking resources here
Ack. Will fix.
>
>
> we should also think about interaction of unique_match interaction for
> !pattern case, and either reject it (if it makes no sense), or enforce
> it (if it does, I haven't really thought about which case do we have)
The unique_match only makes sense for pattern case. So I suggest to
reject the case unique_match && !pattern. WDYT?
>
> pw-bot: cr
>
>> + }
>> +
>> addrs = res.addrs;
>> cnt = res.cnt;
>> }
>> diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
>> index d45807103565..3020ee45303a 100644
>> --- a/tools/lib/bpf/libbpf.h
>> +++ b/tools/lib/bpf/libbpf.h
>> @@ -552,10 +552,12 @@ struct bpf_kprobe_multi_opts {
>> bool retprobe;
>> /* create session kprobes */
>> bool session;
>> + /* enforce unique match */
>> + bool unique_match;
>> size_t :0;
>> };
>>
>> -#define bpf_kprobe_multi_opts__last_field session
>> +#define bpf_kprobe_multi_opts__last_field unique_match
>>
>> LIBBPF_API struct bpf_link *
>> bpf_program__attach_kprobe_multi_opts(const struct bpf_program *prog,
>> --
>> 2.43.5
>>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bpf-next 1/2] libbpf: Add unique_match option for multi kprobe
2025-01-07 18:29 ` Yonghong Song
@ 2025-01-08 22:38 ` Andrii Nakryiko
0 siblings, 0 replies; 6+ messages in thread
From: Andrii Nakryiko @ 2025-01-08 22:38 UTC (permalink / raw)
To: Yonghong Song
Cc: bpf, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
kernel-team, Martin KaFai Lau, Jordan Rome
On Tue, Jan 7, 2025 at 10:29 AM Yonghong Song <yonghong.song@linux.dev> wrote:
>
>
>
>
> On 1/6/25 4:24 PM, Andrii Nakryiko wrote:
> > On Wed, Dec 18, 2024 at 2:53 PM Yonghong Song <yonghong.song@linux.dev> wrote:
> >> Jordan reported an issue in Meta production environment where func
> >> try_to_wake_up() is renamed to try_to_wake_up.llvm.<hash>() by clang
> >> compiler at lto mode. The original 'kprobe/try_to_wake_up' does not
> >> work any more since try_to_wake_up() does not match the actual func
> >> name in /proc/kallsyms.
> >>
> >> There are a couple of ways to resolve this issue. For example, in
> >> attach_kprobe(), we could do lookup in /proc/kallsyms so try_to_wake_up()
> >> can be replaced by try_to_wake_up.llvm.<hach>(). Or we can force users
> >> to use bpf_program__attach_kprobe() where they need to lookup
> >> /proc/kallsyms to find out try_to_wake_up.llvm.<hach>(). But these two
> >> approaches requires extra work by either libbpf or user.
> >>
> >> Luckily, suggested by Andrii, multi kprobe already supports wildcard ('*')
> >> for symbol matching. In the above example, 'try_to_wake_up*' can match
> >> to try_to_wake_up() or try_to_wake_up.llvm.<hash>() and this allows
> >> bpf prog works for different kernels as some kernels may have
> >> try_to_wake_up() and some others may have try_to_wake_up.llvm.<hash>().
> >>
> >> The original intention is to kprobe try_to_wake_up() only, so an optional
> >> field unique_match is added to struct bpf_kprobe_multi_opts. If the
> >> field is set to true, the number of matched functions must be one.
> >> Otherwise, the attachment will fail. In the above case, multi kprobe
> >> with 'try_to_wake_up*' and unique_match preserves user functionality.
> >>
> >> Reported-by: Jordan Rome <linux@jordanrome.com>
> >> Suggested-by: Andrii Nakryiko <andrii@kernel.org>
> >> Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
> >> ---
> >> tools/lib/bpf/libbpf.c | 10 +++++++++-
> >> tools/lib/bpf/libbpf.h | 4 +++-
> >> 2 files changed, 12 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> >> index 66173ddb5a2d..649c6e92972a 100644
> >> --- a/tools/lib/bpf/libbpf.c
> >> +++ b/tools/lib/bpf/libbpf.c
> >> @@ -11522,7 +11522,7 @@ bpf_program__attach_kprobe_multi_opts(const struct bpf_program *prog,
> >> struct bpf_link *link = NULL;
> >> const unsigned long *addrs;
> >> int err, link_fd, prog_fd;
> >> - bool retprobe, session;
> >> + bool retprobe, session, unique_match;
> >> const __u64 *cookies;
> >> const char **syms;
> >> size_t cnt;
> >> @@ -11558,6 +11558,14 @@ bpf_program__attach_kprobe_multi_opts(const struct bpf_program *prog,
> >> err = libbpf_available_kallsyms_parse(&res);
> >> if (err)
> >> goto error;
> >> +
> >> + unique_match = OPTS_GET(opts, unique_match, false);
> >> + if (unique_match && res.cnt != 1) {
> >> + pr_warn("prog '%s': failed to find unique match: cnt %lu\n",
> >> + prog->name, res.cnt);
> >> + return libbpf_err_ptr(-EINVAL);
> > goto error, leaking resources here
>
> Ack. Will fix.
>
> >
> >
> > we should also think about interaction of unique_match interaction for
> > !pattern case, and either reject it (if it makes no sense), or enforce
> > it (if it does, I haven't really thought about which case do we have)
>
> The unique_match only makes sense for pattern case. So I suggest to
> reject the case unique_match && !pattern. WDYT?
>
Yep, let's reject (we could make it behave well, just making sure that
cnt == 1 if unique_match == true, but why bother, it's not intended to
be used together).
> >
> > pw-bot: cr
> >
> >> + }
> >> +
> >> addrs = res.addrs;
> >> cnt = res.cnt;
> >> }
> >> diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> >> index d45807103565..3020ee45303a 100644
> >> --- a/tools/lib/bpf/libbpf.h
> >> +++ b/tools/lib/bpf/libbpf.h
> >> @@ -552,10 +552,12 @@ struct bpf_kprobe_multi_opts {
> >> bool retprobe;
> >> /* create session kprobes */
> >> bool session;
> >> + /* enforce unique match */
> >> + bool unique_match;
> >> size_t :0;
> >> };
> >>
> >> -#define bpf_kprobe_multi_opts__last_field session
> >> +#define bpf_kprobe_multi_opts__last_field unique_match
> >>
> >> LIBBPF_API struct bpf_link *
> >> bpf_program__attach_kprobe_multi_opts(const struct bpf_program *prog,
> >> --
> >> 2.43.5
> >>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-01-08 22:38 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-12-18 22:52 [PATCH bpf-next 1/2] libbpf: Add unique_match option for multi kprobe Yonghong Song
2024-12-18 22:52 ` [PATCH bpf-next 2/2] selftests/bpf: Add a test for kprobe multi with unique_match Yonghong Song
2024-12-18 23:12 ` [PATCH bpf-next 1/2] libbpf: Add unique_match option for multi kprobe Jordan Rome
2025-01-07 0:24 ` Andrii Nakryiko
2025-01-07 18:29 ` Yonghong Song
2025-01-08 22:38 ` Andrii Nakryiko
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox