Re: [PATCH 08/11] perf bpf: Use scnprintf() in snprintf_hex() and synthesize_bpf_prog_name()

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [Critical] Missing bounds and presence validation for `prog_tags` and `func_info` array offsets allows arbitrary out-of-bounds reads when parsing untrusted `perf.data` files.
--

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

perf bpf: Use scnprintf() in snprintf_hex() and synthesize_bpf_prog_name()
    
Both functions accumulate formatted output via ret += snprintf(buf + ret,
size - ret, ...). Switch to scnprintf() which returns the actual number of
bytes written, making the accumulation safe.

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

[ ... ]

> @@ -140,7 +140,7 @@ static int synthesize_bpf_prog_name(char *buf, int size,
>  	const struct btf_type *t;
>  	int name_len;
>  
> -	name_len = snprintf(buf, size, "bpf_prog_");
> +	name_len = scnprintf(buf, size, "bpf_prog_");
>  	name_len += snprintf_hex(buf + name_len, size - name_len,
>  				 prog_tags[sub_id], BPF_TAG_SIZE);

[Severity: Critical]
This isn't a bug introduced by this patch, but is it possible for the
unconditionally dereferenced prog_tags pointer to trigger an out-of-bounds
read when analyzing untrusted perf.data files?

When reading perf.data files, bpil_offs_to_addr() in
tools/perf/util/bpf-utils.c adds an offset read from the file directly to
the heap allocation pointer without bounds checking against data_len:

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

If the PERF_BPIL_PROG_TAGS bit is missing from the arrays mask, info->prog_tags
remains untranslated and points to whatever attacker-controlled value was
provided in the file. 

When synthesize_bpf_prog_name() dereferences prog_tags[sub_id], could this 
leak arbitrary memory into the output buffer? Does this require bounds and
presence validation for the prog_tags offset during parsing?

>  	if (btf) {
> @@ -153,9 +153,10 @@ static int synthesize_bpf_prog_name(char *buf, int size,
>  			short_name = info->name;
>  	} else
>  		short_name = "F";
> -	if (short_name)
> -		name_len += snprintf(buf + name_len, size - name_len,
> -				     "_%s", short_name);
> +	if (short_name) {
> +		name_len += scnprintf(buf + name_len, size - name_len,
> +				      "_%s", short_name);
> +	}
>  	return name_len;
>  }

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