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

sashiko-bot
2026-04-11T07:23:50Z

Sashiko AI review found 3 potential issue(s):
- [High] The newly introduced bounds checks are vulnerable to integer underflow, allowing a bypass that leads to out-of-bounds memory reads.
- [High] The bounds checking for dynamic fields is incomplete; the dynamically extracted offset is never validated against the sample size.
- [High] Out-of-bounds read when extracting the dynamic field locator.
--

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

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

Avoid a tracepoint field accidentally reading out of bounds by
checking the size of read fits.

> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
> index 3c10b840778e0..1e0bb4cb995d4 100644
> --- a/tools/perf/util/evsel.c
> +++ b/tools/perf/util/evsel.c
> @@ -3727,6 +3727,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) {

Since offset is a signed integer, could a negative offset bypass this check?

If a negative offset and a positive size are provided (e.g., offset = -4,
size = 8), their sum could be a small positive number that passes the
> sample->raw_size check, leading to an out-of-bounds read before the
start of the buffer.

> + 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;
> + }
>
> if (field->flags & TEP_FIELD_IS_DYNAMIC) {
> offset = *(int *)(sample->raw_data + field->offset);

Does this 4-byte read have a bounds checking issue if field->size is less
than 4?

If the field is placed at the very end of sample->raw_data with a size of 2,
the earlier check would pass, but this would read 2 bytes out of bounds.

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

Should this dynamically extracted offset be bounds checked against
sample->raw_size before returning the pointer?

Callers like perf_sample__strval() dereference the returned pointer,
which could result in an out-of-bounds read if the extracted offset is
beyond the sample size.

> }
>
> @@ -3744,6 +3749,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) {

Could this check also be bypassed by a negative field->offset, similar to
the check in perf_sample__rawptr() above?

> + 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;
> + }
> +
> switch (field->size) {
> case 1:
> return *(u8 *)ptr;

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