Re: [PATCH 19/23] perf bpf: Add NULL check for btf__type_by_id() in synthesize_bpf_prog_name()

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [Critical] Unvalidated array offsets and skipped pointer translations in perf.data BPF payload parsing lead to arbitrary memory read vulnerabilities.
- [High] Missing bounds and array existence checks for func_infos allow out-of-bounds reads and segmentation faults when parsing crafted BPF program info.
--

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

perf bpf: Add NULL check for btf__type_by_id() in synthesize_bpf_prog_name()

synthesize_bpf_prog_name() calls btf__type_by_id() and immediately
dereferences the result via t->name_off without checking for NULL.
btf__type_by_id() returns NULL when the type_id is invalid or out
of range, so an invalid ID from untrusted input causes a NULL pointer
dereference. This patch adds a NULL check before dereferencing t.

> diff --git a/tools/perf/util/bpf-event.c b/tools/perf/util/bpf-event.c
> index 2c09842469f1f..fe01551dc3e6c 100644
> --- a/tools/perf/util/bpf-event.c
> +++ b/tools/perf/util/bpf-event.c
> @@ -146,7 +146,8 @@ static int synthesize_bpf_prog_name(char *buf, int size,
>  	if (btf) {
>  		finfo = func_infos + sub_id * info->func_info_rec_size;

[Severity: High]
This is a pre-existing issue, but is there a missing bounds check on sub_id
before we calculate the finfo pointer?

In __bpf_event__print_bpf_prog_info(), this function is called in a loop
bounded by info->nr_jited_ksyms, which is passed as sub_id.  However,
func_infos is sized according to info->nr_func_info.

Since these lengths and func_info_rec_size can be attacker-controlled from an
untrusted perf.data file, couldn't a large nr_jited_ksyms and a small
nr_func_info cause the pointer arithmetic here to point out of bounds,
leading to a segmentation fault when accessing finfo->type_id below?

>  		t = btf__type_by_id(btf, finfo->type_id);

[Severity: Critical]
This isn't a bug introduced by this patch, but does the broader handling of
perf.data BPF payloads allow for arbitrary memory reads?

When parsing HEADER_BPF_PROG_INFO from an untrusted perf.data file,
process_bpf_prog_info() reads attacker-controlled lengths and fields into
info_linear->info.

The helper bpil_offs_to_addr() translates these offsets into memory pointers,
but if the corresponding bit in the arrays bitmask is unset, the translation
is skipped, leaving the raw attacker-controlled 64-bit value intact:

tools/perf/util/bpf-utils.c:bpil_offs_to_addr() {
	...
	if ((info_linear->arrays & (1UL << i)) == 0)
		continue;
	offs = bpf_prog_info_read_offset_u64(&info_linear->info,
					     desc->array_offset);
	addr = offs + ptr_to_u64(info_linear->data);
	...
}

Furthermore, even if the bit is set, it appears there is no verification that
the calculated offset falls within info_linear->data_len.

When functions like synthesize_bpf_prog_name() or
__bpf_event__print_bpf_prog_info() dereference these unvalidated pointers
(like info->func_info or info->prog_tags) and print their contents, could this
provide an arbitrary memory read primitive if a privileged user runs
perf report -D on an untrusted file?

> -		short_name = btf__name_by_offset(btf, t->name_off);
> +		if (t)
> +			short_name = btf__name_by_offset(btf, t->name_off);
>  	} else if (sub_id == 0 && sub_prog_cnt == 1) {
[ ... ]

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