Re: [PATCH] perf powerpc-vpadtl: Fix raw_size of DTL samples

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing bounds check in perf_sample__fprintf_synth_vpadtl allows out-of-bounds reads and array overflows.
- [High] Integer underflow and out-of-bounds read in powerpc_vpadtl_decode_all leading to infinite loop and memory leak.
--

--- Patch [1]: [PATCH] perf powerpc-vpadtl: Fix raw_size of DTL samples ---
commit 757f0f4d0a4831a576dcdc38cfd2b1f9262aa2e3
Author: Wang Yan <wangyan01@kylinos.cn>
Subject: perf powerpc-vpadtl: Fix raw_size of DTL samples

This commit fixes an issue where raw_size of DTL samples was incorrectly set
to the pointer size rather than the record size. This caused consumers to
only copy the first 8 bytes of each entry instead of the full record. The
fix uses sizeof(*record) so that raw_size reflects the actual length.

> diff --git a/tools/perf/util/powerpc-vpadtl.c b/tools/perf/util/powerpc-vpadtl.c
> index 710f3093f3f90..af6783cfdb53d 100644
> --- a/tools/perf/util/powerpc-vpadtl.c
> +++ b/tools/perf/util/powerpc-vpadtl.c
> @@ -196,7 +196,7 @@ static int powerpc_vpadtl_sample(struct powerpc_vpadtl_entry *record,
>  	sample.cpumode = PERF_RECORD_MISC_KERNEL;
>  	sample.time = save;
>  	sample.raw_data = record;
> -	sample.raw_size = sizeof(record);
> +	sample.raw_size = sizeof(*record);

[Severity: High]
This is a pre-existing issue, but could this lead to out-of-bounds array
reads when processing older or malformed perf.data files?

If an older file was recorded before this patch (where raw_size is 8),
could perf_sample__fprintf_synth_vpadtl() read beyond the provided raw_data
when accessing fields like timebase?

Additionally, if the dispatch_reason or preempt_reason values from an
untrusted file are not validated, could they cause out-of-bounds reads on
the dispatch_reasons and preempt_reasons arrays?

tools/perf/builtin-script.c:perf_sample__fprintf_synth_vpadtl() {
	...
	len = fprintf(fp, "timebase: %" PRIu64 " dispatch_reason:%s, preempt_reason:%s,\n"
			"enqueue_to_dispatch_time:%d, ready_to_enqueue_time:%d,"
			"waiting_to_ready_time:%d, processor_id: %d",
			get_unaligned_be64(&dtl->timebase),
			dispatch_reasons[dtl->dispatch_reason],
			preempt_reasons[dtl->preempt_reason],
	...
}

[Severity: High]
This is a pre-existing issue, but could a small auxtrace buffer trigger an
integer underflow and an infinite loop?

If an auxtrace buffer is provided that is smaller than 48 bytes, it appears
buf_len will be initialized to 0 here:

tools/perf/util/powerpc-vpadtl.c:powerpc_vpadtl_get_buffer() {
	...
	vpaq->buf_len = buffer->size;

	if (buffer->size % dtl_entry_size)
		vpaq->buf_len = buffer->size - (buffer->size % dtl_entry_size);
	...
}

Then later, if the timebase is positive, powerpc_vpadtl_decode_all() will
subtract dtl_entry_size (48) from buf_len without checking if it is 0:

tools/perf/util/powerpc-vpadtl.c:powerpc_vpadtl_decode_all() {
	...
	if (vpaq->buffer) {
		buf = vpaq->buffer->data;
		buf += vpaq->pkt_len;
		vpaq->dtl = (struct powerpc_vpadtl_entry *)buf;
		if ((long long)be64_to_cpu(vpaq->dtl->timebase) <= 0) {
			...
		}
		vpaq->pkt_len += dtl_entry_size;
		vpaq->buf_len -= dtl_entry_size;
	...
}

Could this underflow buf_len, bypass the buffer reload logic, and result in
an infinite loop that sequentially reads out-of-bounds heap memory?

>  	event.sample.header.type = PERF_RECORD_SAMPLE;
>  	event.sample.header.misc = sample.cpumode;
>  	event.sample.header.size = sizeof(struct perf_event_header);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260825094733.1031566-1-wangyan01@kylinos.cn?part=1