Re: [PATCH v10 27/29] perf evsel: Add bounds checking to trace point raw data accessors

sashiko-bot
2026-04-12T02:34:21Z

Sashiko AI review found 4 potential issue(s):
- [High] Missing bounds check for the computed offset of dynamic fields in `perf_sample__rawptr`.
- [High] Integer underflow/overflow in the newly added bounds checks allows bypass and out-of-bounds reads.
- [High] Unconditional 4-byte memory read for dynamic fields can read out-of-bounds.
- [Medium] Using `pr_warning` in a per-sample hot path can cause console flooding.
--

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

perf evsel: Add bounds checking to trace point raw data accessors

This commit adds bounds checking to tracepoint field accesses to prevent
out-of-bounds reads when retrieving raw data from samples. It verifies
that the field's offset and size fit within the sample's raw size.

> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
> index bb48568b8101a..c3a20f13d669e 100644
> --- a/tools/perf/util/evsel.c
> +++ b/tools/perf/util/evsel.c
> @@ -3709,6 +3709,11 @@ void *perf_sample__rawptr(struct perf_sample *sample, const char *name)
> return NULL;
>
> offset = field->offset;
> + if ((u32)(offset + field->size) > sample->raw_size) {

Are field->offset and field->size signed integers? If an attacker crafts a
format with a negative offset, could the sum evaluate to a small positive
number and bypass this check?

This could lead to an out-of-bounds read when accessing
sample->raw_data + offset.

> + 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);

Since this function is called for every sample, could using pr_warning() here
spam the console and degrade performance if the format is mismatched? Should
this use a ratelimited warning like pr_warning_once() instead?

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

Does this unconditional 4-byte read risk an out-of-bounds access?

If the dynamic field's size is less than 4 and it is located at the very end
of the sample buffer, the earlier size check will pass, but this cast and
dereference could read past the end of the raw_data buffer.

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

Does the newly computed offset for dynamic fields also need a bounds check
against sample->raw_size?

The code returns sample->raw_data + offset without validating if this new
dynamic offset fits within the allocated buffer, which could allow arbitrary
memory reads.

[ ... ]

> @@ -3726,6 +3731,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;
> + }
> +

Similar to perf_sample__rawptr(), does this bounds check allow a bypass if
field->offset is negative?

Also, could pr_warning() flood the console in this hot path if malformed
data is processed?

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