Re: [PATCH 12/15] perf bpf: Bounds-check array offsets in bpil_offs_to_addr()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Information leak of heap address due to incomplete offset sanitization.
--

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

perf bpf: Bounds-check array offsets in bpil_offs_to_addr()

This commit adds a bounds check to bpil_offs_to_addr() for offsets coming from
untrusted perf.data files. When an offset exceeds data_len, the offset fields
are zeroed out and the pointer conversion is skipped to prevent out-of-bounds
pointer construction.

> diff --git a/tools/perf/util/bpf-utils.c b/tools/perf/util/bpf-utils.c
> --- a/tools/perf/util/bpf-utils.c
> +++ b/tools/perf/util/bpf-utils.c

[ ... ]

> @@ -264,12 +264,26 @@ void bpil_offs_to_addr(struct perf_bpil *info_linear)
>  	for (i = PERF_BPIL_FIRST_ARRAY; i < PERF_BPIL_LAST_ARRAY; ++i) {
>  		const struct bpil_array_desc *desc = &bpil_array_desc[i];
>  		__u64 addr, offs;
> +		__u32 count, size;
>  
>  		if ((info_linear->arrays & (1UL << i)) == 0)
>  			continue;
>  
>  		offs = bpf_prog_info_read_offset_u64(&info_linear->info,
>  						     desc->array_offset);
> +		count = bpf_prog_info_read_offset_u32(&info_linear->info,
> +						      desc->count_offset);
> +		size = bpf_prog_info_read_offset_u32(&info_linear->info,
> +						     desc->size_offset);
> +		/* offset and extent from perf.data are untrusted — keep within data[] */
> +		if (offs >= info_linear->data_len ||
> +		    (u64)count * size > info_linear->data_len - offs) {
> +			bpf_prog_info_set_offset_u64(&info_linear->info,
> +						     desc->array_offset, 0);
> +			bpf_prog_info_set_offset_u32(&info_linear->info,
> +						     desc->count_offset, 0);
> +			continue;

[Severity: Medium]
Does skipping this conversion without clearing the corresponding array bit
in info_linear->arrays leak the heap address later?

When the bounds check fails, the offset is set to 0, but the active array
bit remains set in the info_linear->arrays bitmask.

Later, if the tool rewrites the data (such as during perf inject or archive),
write_bpf_prog_info() will call bpil_addr_to_offs(), which processes all
arrays that still have their bits set. It subtracts the heap base pointer
(info_linear->data) from the array address.

Since the address was explicitly forced to 0 here, the calculation becomes
0 - ptr_to_u64(info_linear->data). This results in the two's complement of
the heap address, which is then saved into the bpf_prog_info struct and
written back out to the perf.data file.

Would it be safer to also clear the active array bit via
info_linear->arrays &= ~(1UL << i) when zeroing out these offsets?

> +		}
>  		addr = offs + ptr_to_u64(info_linear->data);
>  		bpf_prog_info_set_offset_u64(&info_linear->info,
>  					     desc->array_offset, addr);
>  	}
>  }

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