> On 1 Jul 2026, at 2:20 PM, [email protected] wrote:
>
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s)
> to consider:
> - [High] The driver violates the perf AUX ring buffer API by writing trace
> data to independent offsets, ignoring the core perf ring buffer head.
> - [Medium] Signed integer overflow when calculating the AUX buffer size leads
> to a massive 64-bit value.
Thanks for catching this:
I will be sending a V2 with fix to:
- Handle the fragmented memory writes
- Fix the aux buffer size calculation
Thanks
Athira
> --
>
> commit 10ce4df3e9571041276c72fea1e10e3955d5eea1
> Author: Athira Rajeev <[email protected]>
> Subject: powerpc/htm: Add support to setup and free aux buffer for capturing
> HTM data
>
> This commit adds PMU callbacks for setup_aux and free_aux to enable
> capturing HTM trace data into a perf auxiliary buffer. It allocates
> memory and records trace data directly into the provided pages for
> later post-processing.
>
>> diff --git a/arch/powerpc/perf/htm-perf.c b/arch/powerpc/perf/htm-perf.c
>> index e22a7fdce2f5d..ae7f469b6840d 100644
>> --- a/arch/powerpc/perf/htm-perf.c
>> +++ b/arch/powerpc/perf/htm-perf.c
> [ ... ]
>> @@ -126,6 +143,74 @@ static ssize_t htm_return_check(int rc)
>> return -EINVAL;
>> }
>>
>> +static int htm_dump_sample_data(struct perf_event *event)
>> +{
> [ ... ]
>> + /*
>> + * Invoke H_HTM call with:
>> + * - operation as htm dump (H_HTM_OP_DUMP_DATA)
>> + * - last three values are address, size and offset
>> + */
>> + if (aux_buf->collect_htm_trace) {
>> + do {
>> + rc = htm_hcall_wrapper(htmflags, nodeindex, nodalchipindex,
>> coreindexonchip,
>> + htmtype, H_HTM_OP_DUMP_DATA, virt_to_phys(aux_buf->base),
>> + (aux_buf->nr_pages * PAGE_SIZE), aux_buf->head);
>> + ret = htm_return_check(rc);
>> + } while (ret == -EBUSY && ++retries < 100);
>> +
>> + if (ret > 0) {
>> + aux_buf->head += (aux_buf->nr_pages * PAGE_SIZE);
>> + perf_aux_output_end(&htm_ctx->handle, (aux_buf->nr_pages * PAGE_SIZE));
>
> [Severity: High]
> Does this code write trace data to independent offsets, ignoring the core
> perf ring buffer head?
>
> If the user does not consume data fast enough, perf_aux_output_end() will
> truncate the write to the available space. Because the driver ignores
> htm_ctx->handle.head (the true write offset within the ring buffer) and
> tracks its own offsets via aux_buf->base and aux_buf->head, could any
> truncation desynchronize the driver from the perf core?
>
> This could lead to subsequent writes going to the wrong offset while
> userspace is told the data is at a different offset, resulting in silent
> corruption of the perf AUX trace data.
>
>> + } else {
>> + aux_buf->collect_htm_trace = 0;
>> + perf_aux_output_end(&htm_ctx->handle, 0);
>> + }
>> + }
>> +
>> + return ret;
>> +}
> [ ... ]
>> +static void *htm_setup_aux(struct perf_event *event, void **pages,
>> + int nr_pages, bool snapshot)
>> +{
>> + int cpu = event->cpu;
>> + struct htm_pmu_buf *buf;
>> +
>> + /* We need at least one page for this to work. */
>> + if (!nr_pages)
>> + return NULL;
>> +
>> + if (cpu == -1)
>> + cpu = raw_smp_processor_id();
>> +
>> + buf = kzalloc_node(sizeof(*buf), GFP_KERNEL, cpu_to_node(cpu));
>> + if (!buf)
>> + return NULL;
>> +
>> + buf->base = pages[0];
>> +
>> + if (!buf->base) {
>> + kfree(buf);
>> + return NULL;
>> + }
>> +
>> + buf->nr_pages = nr_pages;
>> + buf->snapshot = false;
>> + buf->size = nr_pages << PAGE_SHIFT;
>
> [Severity: Medium]
> Can the size calculation overflow for large values of nr_pages?
>
> Since both nr_pages and PAGE_SHIFT evaluate as 32-bit signed integers,
> a large AUX buffer allocation (e.g., nr_pages = 32768) could overflow into
> the sign bit. When assigned to the u64 size field, this would sign-extend
> to a massive 64-bit value.
>
> Could this result in bypassing internal bounds checks when the size is
> later returned to the perf subsystem?
>
>> + buf->head = 0;
>> + buf->head_size = 0;
>> + buf->htm_stopped = 0;
>> + buf->collect_htm_trace = 1;
>> + return buf;
>> +}
>
> --
> Sashiko AI review ·
> https://sashiko.dev/#/patchset/[email protected]?part=2
