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] Incomplete array validation in __bpf_event__print_bpf_prog_info() leaves prog_tags and func_info vulnerable to raw offset dereferences.
- [High] symbol__disassemble_bpf_libbfd() completely lacks array validation, leading to raw offset dereferences.
--

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

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

Several functions cast bpf_prog_info fields (jited_ksyms,
jited_func_lens, jited_prog_insns) from u64 to pointers and
dereference them.  These fields are only valid pointers if
bpil_offs_to_addr() converted their file offsets to addresses, which
only happens when the corresponding PERF_BPIL_* bits are set in
info_linear->arrays.

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:
> 
>   - __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

[Severity: High]
This isn't a bug introduced by this patch, but since the commit message
mentions adding validation to all perf.data processing paths, are we missing
the validation in symbol__disassemble_bpf_libbfd() located in
tools/perf/util/libbfd.c?

The function symbol__disassemble_bpf_libbfd() directly casts
jited_prog_insns and jited_ksyms to pointers, and also passes info to
bpf_prog_linfo__new() which accesses line_info. Because it never checks
info_linear->arrays for the presence of PERF_BPIL_JITED_INSNS,
PERF_BPIL_JITED_KSYMS, etc., a crafted perf.data file with missing array
bits could cause the function to pass raw file offsets to the disassembler,
resulting in arbitrary memory reads and crashes when running perf annotate.

> 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 isn't a bug introduced by this patch, but should this required_arrays
bitmask also include PERF_BPIL_PROG_TAGS and PERF_BPIL_FUNC_INFO?

The current bitmask omits these arrays, meaning if a crafted perf.data file
sets offsets for these fields but leaves their bits unset in the arrays
bitmask, bpil_offs_to_addr() will not convert the raw file offsets into safe
pointers.

These unvalidated offsets are then passed to synthesize_bpf_prog_name() which
is called below:

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

Processing an untrusted perf.data file could trigger arbitrary memory reads or
segmentation faults when the raw offset is treated as a pointer and
dereferenced.

> +	__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/20260616193033.10035-1-acme@kernel.org?part=8