1. Patchew
  2. linux
  3. View series

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

Feng Yang posted 1 patch 8 months, 1 week ago
There is a newer version of this series
tools/lib/bpf/libbpf.c | 15 +++++++++++++--
1 file changed, 13 insertions(+), 2 deletions(-)
[PATCH bpf-next] libbpf: Fix event name too long error
Posted by Feng Yang 8 months, 1 week ago 
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(-)

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)
-- 
2.43.0
Re: [PATCH bpf-next] libbpf: Fix event name too long error
Posted by Hengqi Chen 8 months, 1 week ago 
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);
-- 
2.43.0

Cheers,
---
Hengqi
Re: [PATCH bpf-next] libbpf: Fix event name too long error
Posted by Andrii Nakryiko 8 months, 1 week ago 
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

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.