diff mbox series

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

Message ID 20250410052712.206785-1-yangfeng59949@163.com (mailing list archive)
State Superseded
Delegated to: BPF
Headers show
Series [bpf-next] libbpf: Fix event name too long error | expand

Checks

Context Check Description
bpf/vmtest-bpf-next-PR success PR summary
bpf/vmtest-bpf-next-VM_Test-0 success Logs for Lint
bpf/vmtest-bpf-next-VM_Test-2 success Logs for Unittests
bpf/vmtest-bpf-next-VM_Test-1 success Logs for ShellCheck
bpf/vmtest-bpf-next-VM_Test-3 success Logs for Validate matrix.py
bpf/vmtest-bpf-next-VM_Test-4 success Logs for aarch64-gcc / GCC BPF
bpf/vmtest-bpf-next-VM_Test-5 success Logs for aarch64-gcc / build / build for aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-6 success Logs for aarch64-gcc / build-release
bpf/vmtest-bpf-next-VM_Test-10 success Logs for aarch64-gcc / test (test_verifier, false, 360) / test_verifier on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-15 success Logs for s390x-gcc / build-release
bpf/vmtest-bpf-next-VM_Test-12 success Logs for aarch64-gcc / veristat-meta
bpf/vmtest-bpf-next-VM_Test-11 success Logs for aarch64-gcc / veristat-kernel
bpf/vmtest-bpf-next-VM_Test-13 success Logs for s390x-gcc / GCC BPF
bpf/vmtest-bpf-next-VM_Test-14 success Logs for s390x-gcc / build / build for s390x with gcc
bpf/vmtest-bpf-next-VM_Test-18 success Logs for s390x-gcc / test (test_verifier, false, 360) / test_verifier on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-20 success Logs for s390x-gcc / veristat-meta
bpf/vmtest-bpf-next-VM_Test-19 success Logs for s390x-gcc / veristat-kernel
bpf/vmtest-bpf-next-VM_Test-21 success Logs for set-matrix
bpf/vmtest-bpf-next-VM_Test-23 success Logs for x86_64-gcc / build / build for x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-24 success Logs for x86_64-gcc / build-release
bpf/vmtest-bpf-next-VM_Test-7 success Logs for aarch64-gcc / test (test_maps, false, 360) / test_maps on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-25 success Logs for x86_64-gcc / test (test_maps, false, 360) / test_maps on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-31 success Logs for x86_64-gcc / veristat-kernel / x86_64-gcc veristat_kernel
bpf/vmtest-bpf-next-VM_Test-22 success Logs for x86_64-gcc / GCC BPF / GCC BPF
bpf/vmtest-bpf-next-VM_Test-29 success Logs for x86_64-gcc / test (test_progs_parallel, true, 30) / test_progs_parallel on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-30 success Logs for x86_64-gcc / test (test_verifier, false, 360) / test_verifier on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-32 success Logs for x86_64-gcc / veristat-meta / x86_64-gcc veristat_meta
bpf/vmtest-bpf-next-VM_Test-33 success Logs for x86_64-llvm-17 / GCC BPF / GCC BPF
bpf/vmtest-bpf-next-VM_Test-35 success Logs for x86_64-llvm-17 / build-release / build for x86_64 with llvm-17-O2
bpf/vmtest-bpf-next-VM_Test-34 success Logs for x86_64-llvm-17 / build / build for x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-36 success Logs for x86_64-llvm-17 / test (test_maps, false, 360) / test_maps on x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-40 success Logs for x86_64-llvm-17 / veristat-kernel
bpf/vmtest-bpf-next-VM_Test-42 success Logs for x86_64-llvm-18 / GCC BPF / GCC BPF
bpf/vmtest-bpf-next-VM_Test-45 success Logs for x86_64-llvm-18 / test (test_maps, false, 360) / test_maps on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-39 success Logs for x86_64-llvm-17 / test (test_verifier, false, 360) / test_verifier on x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-50 success Logs for x86_64-llvm-18 / veristat-kernel
bpf/vmtest-bpf-next-VM_Test-41 success Logs for x86_64-llvm-17 / veristat-meta
bpf/vmtest-bpf-next-VM_Test-43 success Logs for x86_64-llvm-18 / build / build for x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-44 success Logs for x86_64-llvm-18 / build-release / build for x86_64 with llvm-18-O2
bpf/vmtest-bpf-next-VM_Test-51 success Logs for x86_64-llvm-18 / veristat-meta
bpf/vmtest-bpf-next-VM_Test-49 success Logs for x86_64-llvm-18 / test (test_verifier, false, 360) / test_verifier on x86_64 with llvm-18
netdev/series_format success Single patches do not need cover letters
netdev/tree_selection success Clearly marked for bpf-next
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/build_tools success Errors and warnings before: 26 (+2) this patch: 26 (+2)
netdev/cc_maintainers success CCed 13 of 13 maintainers
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/checkpatch warning CHECK: Comparison to NULL could be written "last_slash"
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
bpf/vmtest-bpf-next-VM_Test-26 success Logs for x86_64-gcc / test (test_progs, false, 360) / test_progs on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-28 success Logs for x86_64-gcc / test (test_progs_no_alu32_parallel, true, 30) / test_progs_no_alu32_parallel on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-27 success Logs for x86_64-gcc / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-37 success Logs for x86_64-llvm-17 / test (test_progs, false, 360) / test_progs on x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-38 success Logs for x86_64-llvm-17 / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-48 success Logs for x86_64-llvm-18 / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-46 success Logs for x86_64-llvm-18 / test (test_progs, false, 360) / test_progs on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-47 success Logs for x86_64-llvm-18 / test (test_progs_cpuv4, false, 360) / test_progs_cpuv4 on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-8 success Logs for aarch64-gcc / test (test_progs, false, 360) / test_progs on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-9 success Logs for aarch64-gcc / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-17 success Logs for s390x-gcc / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-16 success Logs for s390x-gcc / test (test_progs, false, 360) / test_progs on s390x with gcc

Commit Message

Feng Yang April 10, 2025, 5:27 a.m. UTC 
From: Feng Yang <yangfeng@kylinos.cn>

If the event name is too long, it will cause an EINVAL error.

The kernel error path is
probes_write
    trace_parse_run_command
        create_or_delete_trace_uprobe
            trace_uprobe_create
                trace_probe_create
                    __trace_uprobe_create
                        traceprobe_parse_event_name
                            else if (len >= MAX_EVENT_NAME_LEN)
Requires less than 64 bytes.

Signed-off-by: Feng Yang <yangfeng@kylinos.cn>
---
 tools/lib/bpf/libbpf.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

Comments

Hengqi Chen April 10, 2025, 11:27 a.m. UTC | #1 
Hi Feng,

On Thu, Apr 10, 2025 at 1:30 PM Feng Yang <yangfeng59949@163.com> wrote:
>
> From: Feng Yang <yangfeng@kylinos.cn>
>
> If the event name is too long, it will cause an EINVAL error.
>
> The kernel error path is
> probes_write
>     trace_parse_run_command
>         create_or_delete_trace_uprobe
>             trace_uprobe_create
>                 trace_probe_create
>                     __trace_uprobe_create
>                         traceprobe_parse_event_name
>                             else if (len >= MAX_EVENT_NAME_LEN)
> Requires less than 64 bytes.
>

Please don't submit patch in a hurry.
This patch does NOT fix the issue.

> Signed-off-by: Feng Yang <yangfeng@kylinos.cn>
> ---
>  tools/lib/bpf/libbpf.c | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index b2591f5cab65..8e48ba99f06c 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -12227,6 +12227,16 @@ bpf_program__attach_uprobe_multi(const struct bpf_program *prog,
>         return libbpf_err_ptr(err);
>  }
>
> +static const char *get_last_part(const char *path)
> +{
> +       const char *last_slash = strrchr(path, '/');
> +
> +       if (last_slash != NULL)
> +               return last_slash + 1;
> +       else
> +               return path;
> +}
> +

Use basename(3) instead.

>  LIBBPF_API struct bpf_link *
>  bpf_program__attach_uprobe_opts(const struct bpf_program *prog, pid_t pid,
>                                 const char *binary_path, size_t func_offset,
> @@ -12241,7 +12251,7 @@ bpf_program__attach_uprobe_opts(const struct bpf_program *prog, pid_t pid,
>         size_t ref_ctr_off;
>         int pfd, err;
>         bool retprobe, legacy;
> -       const char *func_name;
> +       const char *func_name, *binary_name;
>
>         if (!OPTS_VALID(opts, bpf_uprobe_opts))
>                 return libbpf_err_ptr(-EINVAL);
> @@ -12254,6 +12264,7 @@ bpf_program__attach_uprobe_opts(const struct bpf_program *prog, pid_t pid,
>         if (!binary_path)
>                 return libbpf_err_ptr(-EINVAL);
>
> +       binary_name = get_last_part(binary_path);

What if len(binary_name) >= MAX_EVENT_NAME_LEN ?

>         /* Check if "binary_path" refers to an archive. */
>         archive_sep = strstr(binary_path, "!/");
>         if (archive_sep) {
> @@ -12318,7 +12329,7 @@ bpf_program__attach_uprobe_opts(const struct bpf_program *prog, pid_t pid,
>                         return libbpf_err_ptr(-EINVAL);
>
>                 gen_uprobe_legacy_event_name(probe_name, sizeof(probe_name),
> -                                            binary_path, func_offset);
> +                                            binary_name, func_offset);
>
>                 legacy_probe = strdup(probe_name);
>                 if (!legacy_probe)
> --
> 2.43.0
>
>

FYI, when I mentioned this issue in ([0]), I tested with the following diff:
  [0]: https://github.com/iovisor/bcc/pull/5271

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index b2591f5cab65..4087fc3ae62f 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -11142,10 +11142,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,
+ 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 +11270,7 @@ int probe_kern_syscall_wrapper(int token_fd)

  return pfd >= 0 ? 1 : 0;
  } else { /* legacy mode */
- char probe_name[128];
+ char probe_name[64];

  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 +11328,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[64];

  gen_kprobe_legacy_event_name(probe_name, sizeof(probe_name),
       func_name, offset);
@@ -11880,7 +11880,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 +12313,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[64];

  if (ref_ctr_off)
  return libbpf_err_ptr(-EINVAL);
Andrii Nakryiko April 10, 2025, 5:36 p.m. UTC | #2 
On Thu, Apr 10, 2025 at 4:27 AM Hengqi Chen <hengqi.chen@gmail.com> wrote:
>
> Hi Feng,
>
> On Thu, Apr 10, 2025 at 1:30 PM Feng Yang <yangfeng59949@163.com> wrote:
> >
> > From: Feng Yang <yangfeng@kylinos.cn>
> >
> > If the event name is too long, it will cause an EINVAL error.
> >
> > The kernel error path is
> > probes_write
> >     trace_parse_run_command
> >         create_or_delete_trace_uprobe
> >             trace_uprobe_create
> >                 trace_probe_create
> >                     __trace_uprobe_create
> >                         traceprobe_parse_event_name
> >                             else if (len >= MAX_EVENT_NAME_LEN)
> > Requires less than 64 bytes.
> >
>
> Please don't submit patch in a hurry.
> This patch does NOT fix the issue.

It would be good to also have a bit more human-readable explanation of
the issue.

pw-bot: cr

>
> > Signed-off-by: Feng Yang <yangfeng@kylinos.cn>
> > ---
> >  tools/lib/bpf/libbpf.c | 15 +++++++++++++--
> >  1 file changed, 13 insertions(+), 2 deletions(-)
> >
> > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > index b2591f5cab65..8e48ba99f06c 100644
> > --- a/tools/lib/bpf/libbpf.c
> > +++ b/tools/lib/bpf/libbpf.c
> > @@ -12227,6 +12227,16 @@ bpf_program__attach_uprobe_multi(const struct bpf_program *prog,
> >         return libbpf_err_ptr(err);
> >  }
> >
> > +static const char *get_last_part(const char *path)
> > +{
> > +       const char *last_slash = strrchr(path, '/');
> > +
> > +       if (last_slash != NULL)
> > +               return last_slash + 1;
> > +       else
> > +               return path;
> > +}
> > +
>
> Use basename(3) instead.
>
> >  LIBBPF_API struct bpf_link *
> >  bpf_program__attach_uprobe_opts(const struct bpf_program *prog, pid_t pid,
> >                                 const char *binary_path, size_t func_offset,
> > @@ -12241,7 +12251,7 @@ bpf_program__attach_uprobe_opts(const struct bpf_program *prog, pid_t pid,
> >         size_t ref_ctr_off;
> >         int pfd, err;
> >         bool retprobe, legacy;
> > -       const char *func_name;
> > +       const char *func_name, *binary_name;
> >
> >         if (!OPTS_VALID(opts, bpf_uprobe_opts))
> >                 return libbpf_err_ptr(-EINVAL);
> > @@ -12254,6 +12264,7 @@ bpf_program__attach_uprobe_opts(const struct bpf_program *prog, pid_t pid,
> >         if (!binary_path)
> >                 return libbpf_err_ptr(-EINVAL);
> >
> > +       binary_name = get_last_part(binary_path);
>
> What if len(binary_name) >= MAX_EVENT_NAME_LEN ?
>
> >         /* Check if "binary_path" refers to an archive. */
> >         archive_sep = strstr(binary_path, "!/");
> >         if (archive_sep) {
> > @@ -12318,7 +12329,7 @@ bpf_program__attach_uprobe_opts(const struct bpf_program *prog, pid_t pid,
> >                         return libbpf_err_ptr(-EINVAL);
> >
> >                 gen_uprobe_legacy_event_name(probe_name, sizeof(probe_name),
> > -                                            binary_path, func_offset);
> > +                                            binary_name, func_offset);
> >
> >                 legacy_probe = strdup(probe_name);
> >                 if (!legacy_probe)
> > --
> > 2.43.0
> >
> >
>
> FYI, when I mentioned this issue in ([0]), I tested with the following diff:
>   [0]: https://github.com/iovisor/bcc/pull/5271
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index b2591f5cab65..4087fc3ae62f 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -11142,10 +11142,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,
> + 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 +11270,7 @@ int probe_kern_syscall_wrapper(int token_fd)
>
>   return pfd >= 0 ? 1 : 0;
>   } else { /* legacy mode */
> - char probe_name[128];
> + char probe_name[64];
>
>   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 +11328,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[64];
>
>   gen_kprobe_legacy_event_name(probe_name, sizeof(probe_name),
>        func_name, offset);
> @@ -11880,7 +11880,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 +12313,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[64];
>
>   if (ref_ctr_off)
>   return libbpf_err_ptr(-EINVAL);
> --
> 2.43.0
>
> Cheers,
> ---
> Hengqi
diff mbox series

Patch

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index b2591f5cab65..8e48ba99f06c 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -12227,6 +12227,16 @@  bpf_program__attach_uprobe_multi(const struct bpf_program *prog,
 	return libbpf_err_ptr(err);
 }
 
+static const char *get_last_part(const char *path)
+{
+	const char *last_slash = strrchr(path, '/');
+
+	if (last_slash != NULL)
+		return last_slash + 1;
+	else
+		return path;
+}
+
 LIBBPF_API struct bpf_link *
 bpf_program__attach_uprobe_opts(const struct bpf_program *prog, pid_t pid,
 				const char *binary_path, size_t func_offset,
@@ -12241,7 +12251,7 @@  bpf_program__attach_uprobe_opts(const struct bpf_program *prog, pid_t pid,
 	size_t ref_ctr_off;
 	int pfd, err;
 	bool retprobe, legacy;
-	const char *func_name;
+	const char *func_name, *binary_name;
 
 	if (!OPTS_VALID(opts, bpf_uprobe_opts))
 		return libbpf_err_ptr(-EINVAL);
@@ -12254,6 +12264,7 @@  bpf_program__attach_uprobe_opts(const struct bpf_program *prog, pid_t pid,
 	if (!binary_path)
 		return libbpf_err_ptr(-EINVAL);
 
+	binary_name = get_last_part(binary_path);
 	/* Check if "binary_path" refers to an archive. */
 	archive_sep = strstr(binary_path, "!/");
 	if (archive_sep) {
@@ -12318,7 +12329,7 @@  bpf_program__attach_uprobe_opts(const struct bpf_program *prog, pid_t pid,
 			return libbpf_err_ptr(-EINVAL);
 
 		gen_uprobe_legacy_event_name(probe_name, sizeof(probe_name),
-					     binary_path, func_offset);
+					     binary_name, func_offset);
 
 		legacy_probe = strdup(probe_name);
 		if (!legacy_probe)