Re: [PATCH v9 23/29] perf evsel: Don't pass evsel with sample

[sashiko-bot@kernel.org]

Sashiko AI review found 1 potential issue(s):
- [High] Undefined Behavior in bounds check allows bypass and Out-of-Bounds Read
--

commit ff15872035e54097547806aeac88548255b7e6b5
Author: Ian Rogers <irogers@google.com>

perf evsel: Don't pass evsel with sample

This commit arranges for the sample to contain the evsel, making it
unnecessary to pass the evsel separately. It also adds missing bounds checks
in perf_evsel__parse_id_sample.

> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
> index ee30e15af054c..3c10b840778e0 100644
> --- a/tools/perf/util/evsel.c
> +++ b/tools/perf/util/evsel.c
[ ... ]
>  static int perf_evsel__parse_id_sample(const union perf_event *event,
>  				       struct perf_sample *sample)
>  {
> +	const struct evsel *evsel = sample->evsel;
>  	u64 type = evsel->core.attr.sample_type;
> +	const __u64 *array, *array_begin = event->sample.array;
[ ... ]
> +	array = array_begin + ((event->header.size - sizeof(event->header)) / sizeof(u64)) - 1;
>  	if (type & PERF_SAMPLE_IDENTIFIER) {
> +		if (array < array_begin)
> +			return -EFAULT;
> +
>  		sample->id = *array;
>  		array--;
>  	}
>  
>  	if (type & PERF_SAMPLE_CPU) {
> +		if (array < array_begin)
> +			return -EFAULT;
> +

Is there a risk that the compiler optimizes away this bounds check due to
undefined behavior?

The array pointer is decremented at the end of the previous block via
array--. If the pointer underflows the object boundaries, decrementing it
before the beginning of the object is undefined behavior in C.

Since modern compilers track this, could the optimizer treat the
underflowed pointer as a poison value and silently remove the
array < array_begin check entirely?

If a malformed perf.data file with an event size smaller than the requested
fields is processed, could this bypass the check and lead to an
out-of-bounds read?

Would it be safer to use an integer counter to track the available size
instead of pointer comparisons?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260411191710.524998-1-irogers@google.com?part=23