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

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 <[email protected]>

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/[email protected]?part=3