Re: [PATCH v7 03/23] perf evsel: Refactor evsel tracepoint sample accessors perf_sample

sashiko-bot
2026-04-08T08:20:23Z

Sashiko AI review found 1 potential issue(s):
- [Medium] Integer overflow and incomplete validation in the newly added tracepoint bounds checks allow out-of-bounds memory accesses.
--

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

perf evsel: Refactor evsel tracepoint sample accessors perf_sample

This commit refactors tracepoint sample accessors to remove the evsel
argument. It also adds bounds checks when reading values from tracepoint
fields based on review feedback.

> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
> index 2ee87fd84d3ea..b653e683d7a12 100644
> --- a/tools/perf/util/evsel.c
> +++ b/tools/perf/util/evsel.c
> @@ -3687,15 +3687,20 @@ struct tep_format_field *evsel__common_field(struct evsel *evsel, const char *na
> return tp_format ? tep_find_common_field(tp_format, name) : NULL;
> }
>
> -void *evsel__rawptr(struct evsel *evsel, struct perf_sample *sample, const char *name)
> +void *perf_sample__rawptr(struct perf_sample *sample, const char *name)
> {
> - struct tep_format_field *field = evsel__field(evsel, name);
> + struct tep_format_field *field = evsel__field(sample->evsel, name);
> int offset;
>
> if (!field)
> return NULL;
>
> offset = field->offset;
> + if ((u32)(offset + field->size) > sample->raw_size) {
> + pr_warning("Invalid trace point field offset %d for field of length %d in sample raw data of size %u\n",
> + offset, field->size, sample->raw_size);
> + return NULL;
> + }

Can this bounds check be bypassed due to integer overflow? If a malformed
perf.data file provides an exceptionally large offset (for example,
0xFFFFFFFC) and a size of 4, the sum could wrap around to 0. Would this
allow an out-of-bounds read when accessing the memory?

> if (field->flags & TEP_FIELD_IS_DYNAMIC) {
> offset = *(int *)(sample->raw_data + field->offset);
> offset &= 0xffff;
> if (tep_field_is_relative(field->flags))
> offset += field->offset + field->size;
> }
>
> return sample->raw_data + offset;

Is the dynamically computed offset validated against sample->raw_size? It
appears the initial check covers the initial field offset, but the newly
computed offset for dynamic fields is added directly to sample->raw_data
without a subsequent bounds check, which might lead to out-of-bounds memory
accesses.

[ ... ]

> @@ -3713,6 +3718,12 @@ u64 format_field__intval(struct tep_format_field *field, struct perf_sample *sam
> u64 value;
> void *ptr = sample->raw_data + field->offset;
>
> + if ((u32)(field->offset + field->size) > sample->raw_size) {
> + pr_warning("Invalid trace point field offset %d for field of length %d in sample raw data of size %u\n",
> + field->offset, field->size, sample->raw_size);
> + return 0;
> + }
> +

Can this suffer from the same integer overflow issue? If field->offset and
field->size wrap around when added together, it looks like an out-of-bounds
pointer could be dereferenced when evaluating the field.

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