| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAEyhmHRZWB-ba_mFhAHQbho9geMHMswiY---dMsGCuE1uDSkwA@mail.gmail.com>
Date: Thu, 10 Apr 2025 19:27:10 +0800
From: Hengqi Chen <hengqi.chen@...il.com>
To: Feng Yang <yangfeng59949@....com>
Cc: ast@...nel.org, daniel@...earbox.net, andrii@...nel.org,
martin.lau@...ux.dev, eddyz87@...il.com, song@...nel.org,
yonghong.song@...ux.dev, john.fastabend@...il.com, kpsingh@...nel.org,
sdf@...ichev.me, haoluo@...gle.com, jolsa@...nel.org, bpf@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH bpf-next] libbpf: Fix event name too long error
Hi Feng,
On Thu, Apr 10, 2025 at 1:30 PM Feng Yang <yangfeng59949@....com> wrote:
>
> From: Feng Yang <yangfeng@...inos.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@...inos.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
Powered by blists - more mailing lists