[PATCH bpf-next v2] libbpf: Fix event name too long error

Feng Yang posted 1 patch 8 months, 1 week ago
tools/lib/bpf/libbpf.c | 17 ++++++++++-------
1 file changed, 10 insertions(+), 7 deletions(-)
[PATCH bpf-next v2] libbpf: Fix event name too long error
Posted by Feng Yang 8 months, 1 week ago
From: Feng Yang <yangfeng@kylinos.cn>

When the binary path is excessively long, the generated probe_name in libbpf
exceeds the kernel's MAX_EVENT_NAME_LEN limit (64 bytes).
This causes legacy uprobe event attachment to fail with error code -22.

Use basename() to extract the base filename from the full binary path, removing directory prefixes.
Example: /root/loooooooooooooooooooooooooooooooooooong_name -> loooooooooooooooooooooooooooooooooooong_name.

String Length Limitation: Apply %.32s in snprintf to truncate the base filename to 32 characters.
Example: loooooooooooooooooooooooooooooooooooong_name -> looooooooooooooooooooooooooooooo.

Before Fix:
	libbpf: binary_path: /root/loooooooooooooooooooooooooooooooooooong_name
	libbpf: probe_name: libbpf_32296__root_loooooooooooooooooooooooooooooooooooong_name_0x1106
	libbpf: failed to add legacy uprobe event for /root/loooooooooooooooooooooooooooooooooooong_name:0x1106: -22

After Fix:
	libbpf: binary_path: /root/loooooooooooooooooooooooooooooooooooong_name
	libbpf: probe_name: libbpf_36178_looooooooooooooooooooooooooooooo_0x1106

Fixes: 46ed5fc33db9 ("libbpf: Refactor and simplify legacy kprobe code")
Fixes: cc10623c6810 ("libbpf: Add legacy uprobe attaching support")
Signed-off-by: Hengqi Chen <hengqi.chen@gmail.com>
Signed-off-by: Feng Yang <yangfeng@kylinos.cn>
---
Changes in v2:
- Use basename() and %.32s to fix. Thanks, Hengqi Chen!
- Link to v1: https://lore.kernel.org/all/20250410052712.206785-1-yangfeng59949@163.com/
---
 tools/lib/bpf/libbpf.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index b2591f5cab65..7e10c7c66819 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -60,6 +60,8 @@
 #define BPF_FS_MAGIC		0xcafe4a11
 #endif
 
+#define MAX_EVENT_NAME_LEN	64
+
 #define BPF_FS_DEFAULT_PATH "/sys/fs/bpf"
 
 #define BPF_INSN_SZ (sizeof(struct bpf_insn))
@@ -11142,10 +11144,10 @@ static void gen_kprobe_legacy_event_name(char *buf, size_t buf_sz,
 	static int index = 0;
 	int i;
 
-	snprintf(buf, buf_sz, "libbpf_%u_%s_0x%zx_%d", getpid(), kfunc_name, offset,
-		 __sync_fetch_and_add(&index, 1));
+	snprintf(buf, buf_sz, "libbpf_%u_%.32s_0x%zx_%d", getpid(), kfunc_name,
+		 offset, __sync_fetch_and_add(&index, 1));
 
-	/* sanitize binary_path in the probe name */
+	/* sanitize kfunc_name in the probe name */
 	for (i = 0; buf[i]; i++) {
 		if (!isalnum(buf[i]))
 			buf[i] = '_';
@@ -11270,7 +11272,7 @@ int probe_kern_syscall_wrapper(int token_fd)
 
 		return pfd >= 0 ? 1 : 0;
 	} else { /* legacy mode */
-		char probe_name[128];
+		char probe_name[MAX_EVENT_NAME_LEN];
 
 		gen_kprobe_legacy_event_name(probe_name, sizeof(probe_name), syscall_name, 0);
 		if (add_kprobe_event_legacy(probe_name, false, syscall_name, 0) < 0)
@@ -11328,7 +11330,7 @@ bpf_program__attach_kprobe_opts(const struct bpf_program *prog,
 					    func_name, offset,
 					    -1 /* pid */, 0 /* ref_ctr_off */);
 	} else {
-		char probe_name[256];
+		char probe_name[MAX_EVENT_NAME_LEN];
 
 		gen_kprobe_legacy_event_name(probe_name, sizeof(probe_name),
 					     func_name, offset);
@@ -11880,7 +11882,8 @@ static void gen_uprobe_legacy_event_name(char *buf, size_t buf_sz,
 {
 	int i;
 
-	snprintf(buf, buf_sz, "libbpf_%u_%s_0x%zx", getpid(), binary_path, (size_t)offset);
+	snprintf(buf, buf_sz, "libbpf_%u_%.32s_0x%zx", getpid(),
+		 basename((void *)binary_path), (size_t)offset);
 
 	/* sanitize binary_path in the probe name */
 	for (i = 0; buf[i]; i++) {
@@ -12312,7 +12315,7 @@ bpf_program__attach_uprobe_opts(const struct bpf_program *prog, pid_t pid,
 		pfd = perf_event_open_probe(true /* uprobe */, retprobe, binary_path,
 					    func_offset, pid, ref_ctr_off);
 	} else {
-		char probe_name[PATH_MAX + 64];
+		char probe_name[MAX_EVENT_NAME_LEN];
 
 		if (ref_ctr_off)
 			return libbpf_err_ptr(-EINVAL);
-- 
2.43.0
Re: [PATCH bpf-next v2] libbpf: Fix event name too long error
Posted by Andrii Nakryiko 8 months, 1 week ago
On Fri, Apr 11, 2025 at 1:06 AM Feng Yang <yangfeng59949@163.com> wrote:
>
> From: Feng Yang <yangfeng@kylinos.cn>
>
> When the binary path is excessively long, the generated probe_name in libbpf
> exceeds the kernel's MAX_EVENT_NAME_LEN limit (64 bytes).
> This causes legacy uprobe event attachment to fail with error code -22.
>
> Use basename() to extract the base filename from the full binary path, removing directory prefixes.
> Example: /root/loooooooooooooooooooooooooooooooooooong_name -> loooooooooooooooooooooooooooooooooooong_name.
>
> String Length Limitation: Apply %.32s in snprintf to truncate the base filename to 32 characters.
> Example: loooooooooooooooooooooooooooooooooooong_name -> looooooooooooooooooooooooooooooo.
>
> Before Fix:
>         libbpf: binary_path: /root/loooooooooooooooooooooooooooooooooooong_name
>         libbpf: probe_name: libbpf_32296__root_loooooooooooooooooooooooooooooooooooong_name_0x1106
>         libbpf: failed to add legacy uprobe event for /root/loooooooooooooooooooooooooooooooooooong_name:0x1106: -22
>
> After Fix:
>         libbpf: binary_path: /root/loooooooooooooooooooooooooooooooooooong_name
>         libbpf: probe_name: libbpf_36178_looooooooooooooooooooooooooooooo_0x1106
>
> Fixes: 46ed5fc33db9 ("libbpf: Refactor and simplify legacy kprobe code")
> Fixes: cc10623c6810 ("libbpf: Add legacy uprobe attaching support")
> Signed-off-by: Hengqi Chen <hengqi.chen@gmail.com>
> Signed-off-by: Feng Yang <yangfeng@kylinos.cn>
> ---
> Changes in v2:
> - Use basename() and %.32s to fix. Thanks, Hengqi Chen!
> - Link to v1: https://lore.kernel.org/all/20250410052712.206785-1-yangfeng59949@163.com/
> ---
>  tools/lib/bpf/libbpf.c | 17 ++++++++++-------
>  1 file changed, 10 insertions(+), 7 deletions(-)
>

Please add a selftest demonstrating a problem in the next revision.

> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index b2591f5cab65..7e10c7c66819 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -60,6 +60,8 @@
>  #define BPF_FS_MAGIC           0xcafe4a11
>  #endif
>
> +#define MAX_EVENT_NAME_LEN     64
> +
>  #define BPF_FS_DEFAULT_PATH "/sys/fs/bpf"
>
>  #define BPF_INSN_SZ (sizeof(struct bpf_insn))
> @@ -11142,10 +11144,10 @@ static void gen_kprobe_legacy_event_name(char *buf, size_t buf_sz,
>         static int index = 0;
>         int i;
>
> -       snprintf(buf, buf_sz, "libbpf_%u_%s_0x%zx_%d", getpid(), kfunc_name, offset,
> -                __sync_fetch_and_add(&index, 1));
> +       snprintf(buf, buf_sz, "libbpf_%u_%.32s_0x%zx_%d", getpid(), kfunc_name,
> +                offset, __sync_fetch_and_add(&index, 1));

libbpf_%u_%.32s_0x%zx_%d -> we have 12 fixed characters, offset's %zx
can be up to 16, pid is easily 6-7 characters, and then index probably
1-2 digits... so 63 - 12 - 6 - 16 - 2 = 27, so if kfunc_name length is
32, we'll be stripping out that index meant to make event name unique
(with high probability).

Let's swap around the order, and put index before %.32s_%zx, shall we?

And I'd just get rid of .32, and let snprintf() do the trimming, if necessary.


pw-bot: cr

>
> -       /* sanitize binary_path in the probe name */
> +       /* sanitize kfunc_name in the probe name */
>         for (i = 0; buf[i]; i++) {
>                 if (!isalnum(buf[i]))
>                         buf[i] = '_';
> @@ -11270,7 +11272,7 @@ int probe_kern_syscall_wrapper(int token_fd)
>
>                 return pfd >= 0 ? 1 : 0;
>         } else { /* legacy mode */
> -               char probe_name[128];
> +               char probe_name[MAX_EVENT_NAME_LEN];
>
>                 gen_kprobe_legacy_event_name(probe_name, sizeof(probe_name), syscall_name, 0);
>                 if (add_kprobe_event_legacy(probe_name, false, syscall_name, 0) < 0)
> @@ -11328,7 +11330,7 @@ bpf_program__attach_kprobe_opts(const struct bpf_program *prog,
>                                             func_name, offset,
>                                             -1 /* pid */, 0 /* ref_ctr_off */);
>         } else {
> -               char probe_name[256];
> +               char probe_name[MAX_EVENT_NAME_LEN];
>
>                 gen_kprobe_legacy_event_name(probe_name, sizeof(probe_name),
>                                              func_name, offset);
> @@ -11880,7 +11882,8 @@ static void gen_uprobe_legacy_event_name(char *buf, size_t buf_sz,
>  {
>         int i;
>
> -       snprintf(buf, buf_sz, "libbpf_%u_%s_0x%zx", getpid(), binary_path, (size_t)offset);
> +       snprintf(buf, buf_sz, "libbpf_%u_%.32s_0x%zx", getpid(),
> +                basename((void *)binary_path), (size_t)offset);
>

let's add that __sync_fetch_and_add(&index) here as well?

>         /* sanitize binary_path in the probe name */
>         for (i = 0; buf[i]; i++) {
> @@ -12312,7 +12315,7 @@ bpf_program__attach_uprobe_opts(const struct bpf_program *prog, pid_t pid,
>                 pfd = perf_event_open_probe(true /* uprobe */, retprobe, binary_path,
>                                             func_offset, pid, ref_ctr_off);
>         } else {
> -               char probe_name[PATH_MAX + 64];
> +               char probe_name[MAX_EVENT_NAME_LEN];
>
>                 if (ref_ctr_off)
>                         return libbpf_err_ptr(-EINVAL);
> --
> 2.43.0
>