Re: [PATCH v12 22/33] perf evsel: Don't pass evsel with sample

[sashiko-bot@kernel.org]

Sashiko AI review found 1 potential issue(s):
- [Medium] Integer Underflow and TOCTOU in `perf_evsel__parse_id_sample` Bounds Check
--

commit 763d668507b0088255a55ce64fb44fde670bed7b
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 bound checks in
perf_evsel__parse_id_sample by switching to using an index and checking that
it doesn't become negative.

> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
> index ee30e15af054c..bb48568b8101a 100644
> --- a/tools/perf/util/evsel.c
> +++ b/tools/perf/util/evsel.c
> @@ -3003,52 +3003,62 @@ int evsel__open_per_thread(struct evsel *evsel, struct perf_thread_map *threads)
>  	return ret;
>  }
>  
> -static int perf_evsel__parse_id_sample(const struct evsel *evsel,
> -				       const union perf_event *event,
> +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 = event->sample.array;
>  	bool swapped = evsel->needs_swap;
>  	union u64_swap u;
> -
> -	array += ((event->header.size -
> -		   sizeof(event->header)) / sizeof(u64)) - 1;
> +	int i = ((event->header.size - sizeof(event->header)) / sizeof(u64)) - 1;

Does this calculation properly protect against underflow on 32-bit platforms?

Since sizeof(event->header) evaluates to an unsigned size_t, the subtraction
could underflow if event->header.size is less than 8.

On 32-bit platforms, this would result in a large positive integer (0x1FFFFFFE).
When assigned to the signed 32-bit int i, the value would remain positive,
bypassing the i < 0 check.

If a local attacker modifies the size field to 0 via a Time-of-Check to
Time-of-Use (TOCTOU) race condition when the file is mapped with MAP_SHARED,
could this regression still lead to an out-of-bounds array access?

>  
>  	if (type & PERF_SAMPLE_IDENTIFIER) {
> -		sample->id = *array;
> -		array--;
> +		if (i < 0)
> +			return -EFAULT;
> +
> +		sample->id = array[i--];
>  	}

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