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

sashiko-bot
2026-04-13T04:42:18Z

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