[PATCHES 0/4] perf bpf hardening

[PATCHES 0/4] perf bpf hardening

[Arnaldo Carvalho de Melo]

Hi,

	Please consider merging,

- Arnaldo

Arnaldo Carvalho de Melo (4):
  perf libbfd: Validate BPF prog info arrays before pointer cast
  perf header: Use write lock when translating BPF prog info pointers
  perf bpf: Add PROG_TAGS to required arrays in __bpf_event__print_bpf_prog_info()
  perf libbfd: Fix memory leaks and NULL fclose in BPF disassembly

 tools/perf/util/bpf-event.c |  3 ++-
 tools/perf/util/header.c    |  5 +++--
 tools/perf/util/libbfd.c    | 23 +++++++++++++++++++----
 3 files changed, 24 insertions(+), 7 deletions(-)

-- 
2.55.0

[PATCH 1/4] perf libbfd: Validate BPF prog info arrays before pointer cast

[Arnaldo Carvalho de Melo]

From: Arnaldo Carvalho de Melo <acme@redhat.com>

symbol__disassemble_bpf_libbfd() casts info_linear->info.jited_prog_insns
and info_linear->info.jited_ksyms to pointers without checking whether
bpil_offs_to_addr() actually converted the file offsets.  A crafted
perf.data with PERF_BPIL_* bits unset but non-zero counts causes raw
file offsets to be dereferenced as pointers.

Add bitmask checks for PERF_BPIL_JITED_INSNS and PERF_BPIL_JITED_KSYMS
before the casts, matching the validation added to bpf-event.c call
sites.

Fixes: 6987561c9e86 ("perf annotate: Enable annotation of BPF programs")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Cc: Song Liu <songliubraving@fb.com>
Cc: Ian Rogers <irogers@google.com>
Assisted-by: Claude:claude-opus-4.6
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/libbfd.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/tools/perf/util/libbfd.c b/tools/perf/util/libbfd.c
index d8241c7caac50836..0b7164f0e9fdbbed 100644
--- a/tools/perf/util/libbfd.c
+++ b/tools/perf/util/libbfd.c
@@ -552,6 +552,11 @@ int symbol__disassemble_bpf_libbfd(struct symbol *sym __maybe_unused,
 	info_linear = info_node->info_linear;
 	sub_id = dso__bpf_prog(dso)->sub_id;
 
+	/* jited_prog_insns is only valid if bpil_offs_to_addr() converted it */
+	if (!(info_linear->arrays & (1UL << PERF_BPIL_JITED_INSNS))) {
+		ret = SYMBOL_ANNOTATE_ERRNO__BPF_MISSING_BTF;
+		goto out;
+	}
 	info.buffer = (void *)(uintptr_t)(info_linear->info.jited_prog_insns);
 	info.buffer_length = info_linear->info.jited_prog_len;
 
@@ -581,6 +586,12 @@ int symbol__disassemble_bpf_libbfd(struct symbol *sym __maybe_unused,
 	if (disassemble == NULL)
 		abort();
 
+	/* jited_ksyms is only valid if bpil_offs_to_addr() converted it */
+	if (!(info_linear->arrays & (1UL << PERF_BPIL_JITED_KSYMS))) {
+		ret = SYMBOL_ANNOTATE_ERRNO__BPF_MISSING_BTF;
+		goto out;
+	}
+
 	fflush(s);
 	do {
 		const struct bpf_line_info *linfo = NULL;
-- 
2.55.0

Re: [PATCH 1/4] perf libbfd: Validate BPF prog info arrays before pointer cast

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing bitmask validation for several PERF_BPIL_* flags before calling bpf_prog_linfo__new(), leading to raw file offset dereferences.
- [High] Missing bitmask validation for PERF_BPIL_PROG_TAGS and PERF_BPIL_FUNC_INFO in bpf-event.c call sites.
--

--- Patch [1]: [PATCH 1/4] perf libbfd: Validate BPF prog info arrays before pointer cast ---
commit 91de4e21d8f65551a65e6afd999ee44cac7b52f7
Author: Arnaldo Carvalho de Melo <acme@redhat.com>

perf libbfd: Validate BPF prog info arrays before pointer cast

This commit adds bitmask checks for PERF_BPIL_JITED_INSNS and
PERF_BPIL_JITED_KSYMS in libbfd.c before casting them to pointers. This
prevents raw file offsets from being dereferenced when processing a crafted
perf.data file.

> perf libbfd: Validate BPF prog info arrays before pointer cast
>
> symbol__disassemble_bpf_libbfd() casts info_linear->info.jited_prog_insns
> and info_linear->info.jited_ksyms to pointers without checking whether
> bpil_offs_to_addr() actually converted the file offsets.  A crafted
> perf.data with PERF_BPIL_* bits unset but non-zero counts causes raw
> file offsets to be dereferenced as pointers.
>
> Add bitmask checks for PERF_BPIL_JITED_INSNS and PERF_BPIL_JITED_KSYMS
> before the casts, matching the validation added to bpf-event.c call
> sites.

[Severity: High]
This is a pre-existing issue, but is the validation in the bpf-event.c call
sites complete?

In tools/perf/util/bpf-event.c:__bpf_event__print_bpf_prog_info(), the
required_arrays bitmask fails to include PERF_BPIL_PROG_TAGS and
PERF_BPIL_FUNC_INFO.

It then proceeds to call synthesize_bpf_prog_name(), which dereferences
info->prog_tags without checking if they were converted:

tools/perf/util/bpf-event.c:synthesize_bpf_prog_name() {
    ...
    name_len += snprintf_hex(buf + name_len, size - name_len,
                             prog_tags[sub_id], BPF_TAG_SIZE);
    ...
}

Could a crafted perf.data file provide non-zero counts but omit the
corresponding bits in the array mask, leaving them as unconverted raw
file offsets?

> diff --git a/tools/perf/util/libbfd.c b/tools/perf/util/libbfd.c
> index c1c12308cc12f..d6c2d9807fc9c 100644
> --- a/tools/perf/util/libbfd.c
> +++ b/tools/perf/util/libbfd.c

[ ... ]

> @@ -581,6 +586,12 @@ int symbol__disassemble_bpf_libbfd(struct symbol *sym __maybe_unused,
>  	if (disassemble == NULL)
>  		abort();
>  
> +	/* jited_ksyms is only valid if bpil_offs_to_addr() converted it */
> +	if (!(info_linear->arrays & (1UL << PERF_BPIL_JITED_KSYMS))) {
> +		ret = SYMBOL_ANNOTATE_ERRNO__BPF_MISSING_BTF;
> +		goto out;
> +	}
> +

[Severity: High]
This is a pre-existing issue, but does calling bpf_prog_linfo__new() earlier
in this function before validating the PERF_BPIL_JITED_KSYMS bitmask risk
dereferencing raw file offsets?

Because bpf_prog_linfo__new() relies on jited_ksyms internally, putting the
validation for PERF_BPIL_JITED_KSYMS here means the validation happens after
the structure has already been accessed:

tools/lib/bpf/bpf_prog_linfo.c:dissect_jited_func() {
    ...
    if (ksym_func[0] != *jited_linfo)
    ...
}

It also seems that bpf_prog_linfo__new() uses info->line_info, which doesn't
have a PERF_BPIL_LINE_INFO check before the call.

Should the validation for PERF_BPIL_JITED_KSYMS (and other arrays like
line_info) be moved earlier, before info_linear->info is passed to
bpf_prog_linfo__new()?

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

[PATCH 2/4] perf header: Use write lock when translating BPF prog info pointers

[Arnaldo Carvalho de Melo]

From: Arnaldo Carvalho de Melo <acme@redhat.com>

write_bpf_prog_info() holds a read lock while temporarily mutating
info_linear via bpil_addr_to_offs()/bpil_offs_to_addr().  Between these
two calls, the pointers in info_linear contain file offsets instead of
heap addresses.  Concurrent readers holding the same read lock see the
file offsets and dereference them as pointers.

Use down_write()/up_write() instead of down_read()/up_read() to exclude
concurrent readers during the addr-to-offset-to-addr translation window.

Fixes: 63ac7968a1fb ("perf bpf: Save bpf_prog_info information as headers to perf.data")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Cc: Song Liu <songliubraving@fb.com>
Assisted-by: Claude:claude-opus-4.6
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/header.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index e90e541f546b4537..7db7da090a1e0c78 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -1120,7 +1120,8 @@ static int write_bpf_prog_info(struct feat_fd *ff  __maybe_unused,
 	struct rb_node *next;
 	int ret = 0;
 
-	down_read(&env->bpf_progs.lock);
+	/* write lock: bpil_addr_to_offs() temporarily mutates info_linear */
+	down_write(&env->bpf_progs.lock);
 
 	ret = do_write(ff, &env->bpf_progs.infos_cnt,
 		       sizeof(env->bpf_progs.infos_cnt));
@@ -1150,7 +1151,7 @@ static int write_bpf_prog_info(struct feat_fd *ff  __maybe_unused,
 			goto out;
 	}
 out:
-	up_read(&env->bpf_progs.lock);
+	up_write(&env->bpf_progs.lock);
 	return ret;
 #else
 	pr_err("ERROR: Trying to write bpf_prog_info without libbpf support.\n");
-- 
2.55.0

Re: [PATCH 2/4] perf header: Use write lock when translating BPF prog info pointers

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] The patch changes `down_read` to `down_write` to protect in-place pointer mutations, but the fix is ineffective because primary readers access the data locklessly.
- [Medium] `bpil_offs_to_addr()` permanently corrupts the in-memory `info_linear->arrays` mask when restoring a valid object containing empty trailing arrays.
--

--- Patch [2]: [PATCH 2/4] perf header: Use write lock when translating BPF prog info pointers ---
commit b51042073da80b7c616b5e20db21243852bbccec
Author: Arnaldo Carvalho de Melo <acme@redhat.com>

perf header: Use write lock when translating BPF prog info pointers

write_bpf_prog_info() temporarily mutates pointers into file offsets,
causing concurrent readers to dereference invalid addresses. This patch
upgrades the synchronization from a read lock to a write lock during the
translation window to exclude concurrent readers.

> diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
> index 091d8f7f6bd2c..70ba9570e3179 100644
> --- a/tools/perf/util/header.c
> +++ b/tools/perf/util/header.c
> @@ -1120,7 +1120,8 @@ static int write_bpf_prog_info(struct feat_fd *ff  __maybe_unused,
>  	struct rb_node *next;
>  	int ret = 0;
>  
> -	down_read(&env->bpf_progs.lock);
> +	/* write lock: bpil_addr_to_offs() temporarily mutates info_linear */
> +	down_write(&env->bpf_progs.lock);

[Severity: High]
This is a pre-existing issue, but does upgrading to a write lock actually
prevent concurrent accesses from seeing the mutated pointers?

Looking at perf_env__find_bpf_prog_info(), it acquires the read lock but
drops it before returning the node pointer:

tools/perf/util/env.c:perf_env__find_bpf_prog_info() {
    ...
    up_read(&env->bpf_progs.lock);
    return node;
}

This means callers like machine__process_bpf_event_load() will dereference
the pointer locklessly:

tools/perf/util/bpf-event.c:machine__process_bpf_event_load() {
    ...
    u64 *addrs = (u64 *)(uintptr_t)(info_linear->info.jited_ksyms);
    ...
}

Because the primary readers are accessing the data outside the critical
section, could they still observe a file offset instead of a valid pointer
even with down_write() here?

>  
>  	ret = do_write(ff, &env->bpf_progs.infos_cnt,
>  		       sizeof(env->bpf_progs.infos_cnt));
> @@ -1150,7 +1151,7 @@ static int write_bpf_prog_info(struct feat_fd *ff  __maybe_unused,
>  			goto out;

[Severity: Medium]
This isn't a bug introduced by this patch, but could calling
bpil_offs_to_addr() just before this return permanently corrupt the state
for empty trailing arrays?

If a BPF program contains an empty array (count = 0) at the end of the
serialized struct, its offset is exactly data_len. When this function calls
bpil_offs_to_addr(node->info_linear) to reverse the offsets, it hits this
bounds check:

tools/perf/util/bpf-utils.c:bpil_offs_to_addr() {
    ...
    if (offs >= info_linear->data_len || ...)
        ...
        info_linear->arrays &= ~(1UL << i);
    ...
}

Since offs >= data_len evaluates to true, will this falsely mark an empty
array as absent in the arrays bitmask, corrupting the live in-memory object?

>  	}
>  out:
> -	up_read(&env->bpf_progs.lock);
> +	up_write(&env->bpf_progs.lock);
>  	return ret;

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

[PATCH 3/4] perf bpf: Add PROG_TAGS to required arrays in __bpf_event__print_bpf_prog_info()

[Arnaldo Carvalho de Melo]

From: Arnaldo Carvalho de Melo <acme@redhat.com>

synthesize_bpf_prog_name() unconditionally dereferences prog_tags[sub_id]
(line: u8 (*prog_tags)[BPF_TAG_SIZE] = (void *)(uintptr_t)(info->prog_tags))
but __bpf_event__print_bpf_prog_info() only requires JITED_KSYMS and
JITED_FUNC_LENS in its required_arrays bitmask.

If a crafted perf.data has the PROG_TAGS bit cleared (or the array was
invalidated by bpil_offs_to_addr() bounds checking), info->prog_tags
contains either zero or a raw file offset.  Dereferencing it causes a
NULL pointer dereference or an arbitrary memory read.

Add PERF_BPIL_PROG_TAGS to required_arrays so the function returns early
when prog_tags was not present or failed validation.

Fixes: f8dfeae009effc0b ("perf bpf: Show more BPF program info in print_bpf_prog_info()")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Cc: Song Liu <songliubraving@fb.com>
Cc: Ian Rogers <irogers@google.com>
Assisted-by: Claude:claude-opus-4.6
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/bpf-event.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/bpf-event.c b/tools/perf/util/bpf-event.c
index fa3ebc8ea7f09cdd..e67f28a8e92bdeb0 100644
--- a/tools/perf/util/bpf-event.c
+++ b/tools/perf/util/bpf-event.c
@@ -969,7 +969,8 @@ void __bpf_event__print_bpf_prog_info(struct perf_bpil *info_linear,
 {
 	struct bpf_prog_info *info = &info_linear->info;
 	__u64 required_arrays = (1UL << PERF_BPIL_JITED_KSYMS) |
-				(1UL << PERF_BPIL_JITED_FUNC_LENS);
+				(1UL << PERF_BPIL_JITED_FUNC_LENS) |
+				(1UL << PERF_BPIL_PROG_TAGS);
 	__u32 *prog_lens;
 	__u64 *prog_addrs;
 	char name[KSYM_NAME_LEN];
-- 
2.55.0

Re: [PATCH 3/4] perf bpf: Add PROG_TAGS to required arrays in __bpf_event__print_bpf_prog_info()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [Medium] The patch fixes an arbitrary memory read by adding PERF_BPIL_PROG_TAGS to required_arrays in __bpf_event__print_bpf_prog_info(), but fails to add corresponding validation for PERF_BPIL_FUNC_INFO, which is also accessed by synthesize_bpf_prog_name().
- [Medium] Missing validation for PERF_BPIL_LINE_INFO and PERF_BPIL_JITED_LINE_INFO before passing bpf_prog_info to bpf_prog_linfo__new().
--

--- Patch [3]: [PATCH 3/4] perf bpf: Add PROG_TAGS to required arrays in __bpf_event__print_bpf_prog_info() ---
commit 34f6adfee35f6522cc507083e8ec384e57659823
Author: Arnaldo Carvalho de Melo <acme@redhat.com>

perf bpf: Add PROG_TAGS to required arrays in __bpf_event__print_bpf_prog_info()

This commit fixes an arbitrary memory read by ensuring PERF_BPIL_PROG_TAGS is
present in the required_arrays bitmask in __bpf_event__print_bpf_prog_info().
If the bit is cleared, prog_tags contains a raw file offset which causes a
crash on dereference.

> diff --git a/tools/perf/util/bpf-event.c b/tools/perf/util/bpf-event.c
> index fa3ebc8ea7f09..e67f28a8e92bd 100644
> --- a/tools/perf/util/bpf-event.c
> +++ b/tools/perf/util/bpf-event.c
> @@ -969,7 +969,8 @@ void __bpf_event__print_bpf_prog_info(struct perf_bpil *info_linear,
>  {
>  	struct bpf_prog_info *info = &info_linear->info;
>  	__u64 required_arrays = (1UL << PERF_BPIL_JITED_KSYMS) |
> -				(1UL << PERF_BPIL_JITED_FUNC_LENS);
> +				(1UL << PERF_BPIL_JITED_FUNC_LENS) |
> +				(1UL << PERF_BPIL_PROG_TAGS);

[Severity: Medium]
This is a pre-existing issue, but does required_arrays also need to include
PERF_BPIL_FUNC_INFO?

In synthesize_bpf_prog_name(), which is called later in this function, we do:

tools/perf/util/bpf-event.c:synthesize_bpf_prog_name() {
    void *func_infos = (void *)(uintptr_t)(info->func_info);
    ...
    if (btf &&
        info->func_info_rec_size >= sizeof(*finfo) &&
        sub_id < info->nr_func_info) {
            finfo = func_infos + sub_id * info->func_info_rec_size;
            t = btf__type_by_id(btf, finfo->type_id);
    ...
}

If a crafted perf.data file has nr_func_info > 0 but the PERF_BPIL_FUNC_INFO
bit is cleared in the array mask, info->func_info remains an untranslated raw
file offset, which could lead to an arbitrary memory read.

[Severity: Medium]
This isn't a bug introduced by this patch, but there appears to be a similar
missing validation for PERF_BPIL_LINE_INFO and PERF_BPIL_JITED_LINE_INFO
when disassembling:

tools/perf/util/libbfd.c:symbol__disassemble_bpf_libbfd() {
    if (info_linear->info.nr_line_info)
        prog_linfo = bpf_prog_linfo__new(&info_linear->info);
    ...
}

tools/lib/bpf/bpf_prog_linfo.c:bpf_prog_linfo__new() {
    ...
    memcpy(prog_linfo->raw_linfo, (void *)(long)info->line_info, data_sz);
    ...
}

If the structure is blindly passed without checking if PERF_BPIL_LINE_INFO was
set and converted, info->line_info is dereferenced as an unvalidated raw file
offset in memcpy(). Could this cause an out-of-bounds read and crash?

>  	__u32 *prog_lens;
>  	__u64 *prog_addrs;

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

[PATCH 4/4] perf libbfd: Fix memory leaks and NULL fclose in BPF disassembly

[Arnaldo Carvalho de Melo]

From: Arnaldo Carvalho de Melo <acme@redhat.com>

symbol__disassemble_bpf_libbfd() has four resource management bugs:

1. free(prog_linfo) leaks internal arrays.  bpf_prog_linfo contains
   raw_linfo, raw_jited_linfo, nr_jited_linfo_per_func, and
   jited_linfo_func_idx pointers that are only freed by the proper
   destructor bpf_prog_linfo__free().

2. open_memstream(&buf, &buf_size) allocates a dynamic buffer that the
   caller must free after fclose().  The function calls fclose(s) but
   never free(buf), leaking the stream buffer on every call.

3. args->line = strdup(srcline) is immediately consumed by
   disasm_line__new(args) which internally calls strdup(args->line)
   again via annotation_line__init().  The first strdup result is then
   overwritten by args->line = buf + prev_buf_size without being freed.

4. If open_memstream() fails, the error path jumps to 'out:' which
   calls fclose(s) with s == NULL — undefined behavior.

Fix by using bpf_prog_linfo__free(), initializing buf to NULL, adding
free(buf) after fclose(s), guarding fclose() against NULL, and removing
the redundant strdup since annotation_line__init() makes its own copy.

Fixes: 6987561c9e86eace ("perf annotate: Enable annotation of BPF programs")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Cc: Song Liu <songliubraving@fb.com>
Cc: Ian Rogers <irogers@google.com>
Assisted-by: Claude:claude-opus-4.6
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/libbfd.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/libbfd.c b/tools/perf/util/libbfd.c
index 0b7164f0e9fdbbed..33dc6158b2b1ffab 100644
--- a/tools/perf/util/libbfd.c
+++ b/tools/perf/util/libbfd.c
@@ -15,6 +15,7 @@
 #ifdef HAVE_LIBBPF_SUPPORT
 #include <bpf/bpf.h>
 #include <bpf/btf.h>
+#include <bpf/libbpf.h>
 #endif
 #include <fcntl.h>
 #include <stdio.h>
@@ -510,7 +511,7 @@ int symbol__disassemble_bpf_libbfd(struct symbol *sym __maybe_unused,
 	char tpath[PATH_MAX];
 	size_t buf_size;
 	int nr_skip = 0;
-	char *buf;
+	char *buf = NULL;
 	bfd *bfdf;
 	int ret;
 	FILE *s;
@@ -620,7 +621,7 @@ int symbol__disassemble_bpf_libbfd(struct symbol *sym __maybe_unused,
 
 		if (!annotate_opts.hide_src_code && srcline) {
 			args->offset = -1;
-			args->line = strdup(srcline);
+			args->line = (char *)srcline;
 			args->line_nr = 0;
 			args->fileloc = NULL;
 			args->ms->sym = sym;
@@ -645,9 +646,12 @@ int symbol__disassemble_bpf_libbfd(struct symbol *sym __maybe_unused,
 
 	ret = 0;
 out:
-	free(prog_linfo);
+	bpf_prog_linfo__free(prog_linfo);
 	btf__free(btf);
-	fclose(s);
+	if (s) {
+		fclose(s);
+		free(buf);
+	}
 	bfd_close(bfdf);
 	return ret;
 #else
-- 
2.55.0