Re: [PATCH 13/13] perf header: Add sanity checks to HEADER_BPF_BTF processing

sashiko-bot
2026-04-10T01:30:29Z

Sashiko AI review found 1 potential issue(s):
- [Low] Brittle dependency on macros defined in another function's scope.
--

commit 2ef335810a8ef7ba04bf003c542540f62231696e
Author: Arnaldo Carvalho de Melo <acme@redhat.com>

perf header: Add sanity checks to HEADER_BPF_BTF processing

This commit adds bounds checking for BTF entry counts and data sizes
when reading HEADER_BPF_BTF from perf.data files to prevent excessive
memory allocations.

> diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
> --- a/tools/perf/util/header.c
> +++ b/tools/perf/util/header.c
> @@ -3619,6 +3619,17 @@ static int process_bpf_btf(struct feat_fd *ff __maybe_unused, void *data __mayb
> if (do_read_u32(ff, &count))
> return -1;
>
> + if (count > MAX_BPF_PROGS) {
> + pr_err("bpf btf count %u too large (max %u)\n", count, MAX_BPF_PROGS);
> + return -1;
> + }

This isn't a bug, but is it safe to rely on MAX_BPF_PROGS and
MAX_BPF_DATA_LEN being available here?

Looking at tools/perf/util/header.c, both of these macros are defined locally
inside the body of process_bpf_prog_info(), which happens to precede this
function in the file:

static int process_bpf_prog_info(...)
{
...
#define MAX_BPF_PROGS 131072
...
#define MAX_BPF_DATA_LEN (256 * 1024 * 1024)
...
}

While the C preprocessor allows this since process_bpf_prog_info() is
evaluated first, would it make sense to move these macro definitions to
the file scope or a shared header? This would prevent hidden dependencies
and compilation failures if these functions are ever reordered.

--
Sashiko AI review · https://sashiko.dev/#/patchset/20260410004000.148138-1-acme@kernel.org?part=13