Re: [PATCH 8/9] perf bpf: Validate array presence before casting BPF prog info pointers

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] The newly added array bitmask validation in `__bpf_event__print_bpf_prog_info()` is incomplete, missing critical validations for `PERF_BPIL_PROG_TAGS` and `PERF_BPIL_FUNC_INFO` arrays.
- [High] The libbfd annotation path completely misses the newly added array bitmask validations, allowing arbitrary file offsets for `jited_prog_insns`, `line_info`, and `jited_line_info` to be dereferenced.
--

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

perf bpf: Validate array presence before casting BPF prog info pointers

This commit adds bitmask validations to ensure that BPF program info fields
like jited_ksyms and jited_func_lens are actually valid pointers before
they are dereferenced. It aims to prevent issues where crafted perf.data
files leave these bits unset, causing functions to dereference raw file
offsets.

>     A crafted perf.data can leave these bits unset while setting non-zero
>     counts and offset values, causing the functions to dereference raw file
>     offsets as pointers.
>     
>     Add array bitmask validation to all perf.data processing paths:

[Severity: High]
This is a pre-existing issue, but does the libbfd annotation path also need
similar validation?

The commit message states validations are added to all perf.data processing
paths, but symbol__disassemble_bpf_libbfd() in tools/perf/util/libbfd.c
accesses info_linear->info.jited_prog_insns and passes it to
bpf_prog_linfo__new(). 

bpf_prog_linfo__new() then casts info->line_info and info->jited_line_info
to pointers and calls memcpy():

tools/lib/bpf/bpf_prog_linfo.c:bpf_prog_linfo__new() {
    ...
    memcpy(prog_linfo->raw_linfo, (void *)(long)info->line_info, data_sz);
    ...
    memcpy(prog_linfo->raw_jited_linfo,
           (void *)(long)info->jited_line_info, data_sz);
    ...
}

Can a crafted perf.data file omitting PERF_BPIL_JITED_INSNS,
PERF_BPIL_LINE_INFO, or PERF_BPIL_JITED_LINE_INFO bits cause an
out-of-bounds read or segmentation fault here?

>       - __bpf_event__print_bpf_prog_info(): check JITED_KSYMS and
>         JITED_FUNC_LENS (changed to take struct perf_bpil *)
>       - machine__process_bpf_event_load(): check JITED_KSYMS
>       - bpf_read(): check JITED_INSNS before memcpy from jited_prog_insns
>       - dso__disassemble_filename(): check JITED_INSNS before returning
>         jited_prog_insns pointer

[ ... ]

> diff --git a/tools/perf/util/bpf-event.c b/tools/perf/util/bpf-event.c
> index 57d53ba848359..fa3ebc8ea7f09 100644
> --- a/tools/perf/util/bpf-event.c
> +++ b/tools/perf/util/bpf-event.c

[ ... ]

> @@ -959,12 +963,15 @@ int evlist__add_bpf_sb_event(struct evlist *evlist, struct perf_env *env)
>  	return evlist__add_sb_event(evlist, &attr, bpf_event__sb_cb, env);
>  }
>  
> -void __bpf_event__print_bpf_prog_info(struct bpf_prog_info *info,
> +void __bpf_event__print_bpf_prog_info(struct perf_bpil *info_linear,
>  				      struct perf_env *env,
>  				      FILE *fp)
>  {
> -	__u32 *prog_lens = (__u32 *)(uintptr_t)(info->jited_func_lens);
> -	__u64 *prog_addrs = (__u64 *)(uintptr_t)(info->jited_ksyms);
> +	struct bpf_prog_info *info = &info_linear->info;
> +	__u64 required_arrays = (1UL << PERF_BPIL_JITED_KSYMS) |
> +				(1UL << PERF_BPIL_JITED_FUNC_LENS);

[Severity: High]
This is a pre-existing issue, but is this required_arrays bitmask missing
validations for PERF_BPIL_PROG_TAGS and PERF_BPIL_FUNC_INFO?

__bpf_event__print_bpf_prog_info() calls synthesize_bpf_prog_name(), which
casts info->prog_tags and info->func_info to pointers and dereferences them:

tools/perf/util/bpf-event.c:synthesize_bpf_prog_name() {
    ...
    u8 (*prog_tags)[BPF_TAG_SIZE] = (void *)(uintptr_t)(info->prog_tags);
    void *func_infos = (void *)(uintptr_t)(info->func_info);
    ...
    name_len += snprintf_hex(buf + name_len, size - name_len,
                             prog_tags[sub_id], BPF_TAG_SIZE);
    ...
}

If a crafted perf.data file leaves the PERF_BPIL_PROG_TAGS and
PERF_BPIL_FUNC_INFO bits unset but provides non-zero offset values, will 
untranslated raw offsets be dereferenced here?

> +	__u32 *prog_lens;
> +	__u64 *prog_addrs;
>  	char name[KSYM_NAME_LEN];
>  	struct btf *btf = NULL;
>  	u32 sub_prog_cnt, i;

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