Re: [PATCH 20/23] perf bpf: Fix map data leak in bpf_metadata_create() on alloc failure

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Integer truncation of `event_size` causes a heap buffer overflow in `synthesize_perf_record_bpf_metadata()`.
--

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

perf bpf: Fix map data leak in bpf_metadata_create() on alloc failure

This patch fixes a memory leak in bpf_metadata_create() where map.btf
and map.rodata were leaked. It addresses this by calling
bpf_metadata_free_map_data() before continuing if bpf_metadata_alloc()
fails.

> diff --git a/tools/perf/util/bpf-event.c b/tools/perf/util/bpf-event.c
> index fe01551dc3e6c..b65ad28cd950f 100644
> --- a/tools/perf/util/bpf-event.c
> +++ b/tools/perf/util/bpf-event.c
> @@ -395,8 +395,10 @@ static struct bpf_metadata *bpf_metadata_create(struct bpf_prog_info *info)
>  			continue;
>  
>  		metadata = bpf_metadata_alloc(info->nr_prog_tags, map.num_vars);
[Severity: High]
This is a pre-existing issue, but this allocation can lead to a heap buffer
overflow due to integer truncation.

In bpf_metadata_alloc(), the calculated event_size is stored into a __u16:

    event_size = sizeof(metadata->event->bpf_metadata) +
        nr_variables * sizeof(metadata->event->bpf_metadata.entries[0]);
    ...
    metadata->event->bpf_metadata = (struct perf_record_bpf_metadata) {
        .header = {
            .type = PERF_RECORD_BPF_METADATA,
            .size = event_size,
        },

If a BPF program contains 204 or more .rodata variables prefixed with
"bpf_metadata_", event_size exceeds 65535 and silently truncates (e.g.,
65808 becomes 272).

Later, synthesize_perf_record_bpf_metadata() allocates a buffer using
this truncated size:

    const size_t event_size = metadata->event->header.size;
    ...
    event = zalloc(event_size + machine->id_hdr_size);

It then copies 512 bytes (BPF_PROG_NAME_LEN), overflowing the truncated
heap allocation:

    memcpy(event->bpf_metadata.prog_name,
           metadata->prog_names[index], BPF_PROG_NAME_LEN);

Could we add bounds checking for map.num_vars or validate event_size before
truncation?

> -		if (!metadata)
> +		if (!metadata) {
> +			bpf_metadata_free_map_data(&map);
>  			continue;
> +		}
>  
>  		bpf_metadata_fill_event(&map, &metadata->event->bpf_metadata);
>

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