Re: [PATCH V2 3/6] powerpc/perf: Add AUX buffer management to capture HTM trace data

[Athira Rajeev <atrajeev@linux.ibm.com>]

> On 20 Jul 2026, at 4:40 PM, sashiko-bot@kernel.org wrote:
> 
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] Reentrancy corruption of the per-CPU perf_output_handle leads to Use-After-Free.
> - [High] Transient conditions permanently disable trace collection and falsely signal EOF.
> --
> 
> --- Patch [3]: [PATCH V2 3/6] powerpc/perf: Add AUX buffer management to capture HTM trace data ---
> commit edd04ff363591ec700b1e3d07e1003b1787ccb4a
> Author: Athira Rajeev <atrajeev@linux.ibm.com>
> 
> powerpc/perf: Add AUX buffer management to capture HTM trace data
> 
> This commit adds support for auxiliary (AUX) ring buffers in the HTM PMU
> driver. It implements the core lifecycle hooks and dynamically checks for
> physical continuity before dumping data to prevent memory corruption.
> 
>> diff --git a/arch/powerpc/perf/htm-perf.c b/arch/powerpc/perf/htm-perf.c
>> index a188a40287044..b4020a6e8f30b 100644
>> --- a/arch/powerpc/perf/htm-perf.c
>> +++ b/arch/powerpc/perf/htm-perf.c
> [ ... ]
>> +struct htm_pmu_ctx {
>> + struct perf_output_handle handle;
>> +};
>> +
>> +static DEFINE_PER_CPU(struct htm_pmu_ctx, htm_pmu_ctx);
>> +
> [ ... ]
>> +static int htm_dump_sample_data(struct perf_event *event)
>> +{
>> + struct htm_pmu_ctx *htm_ctx = this_cpu_ptr(&htm_pmu_ctx);
> [ ... ]
>> + /* Start AUX transaction session framework */
>> + aux_buf = perf_aux_output_begin(&htm_ctx->handle, event);
> 
> [Severity: High]
> What happens to the perf_output_handle if this is re-entered from an NMI
> context?
> 
> htm_event_read() initiates a synchronous AUX output transaction using a
> per-CPU struct perf_output_handle. Since this can be called from process
> context (for instance, userspace reading the perf event fd), an NMI
> interrupting this could trigger a BPF program that calls
> bpf_perf_event_read() on the same CPU.
> 
> Would this overwrite the per-CPU handle, causing the interrupted outer
> call to use a corrupted handle when it resumes, leading to underflowing
> reference counts and a use-after-free of the AUX pages? Could the
> handle be allocated on the stack instead to prevent this?

Thanks for the feedback Sashiko

The reentrancy concern is handled by perf_aux_output_begin() itself in kernel/events/ring_buffer.c:

nest = READ_ONCE(rb->aux_nest);
/*
 * Nesting is not supported for AUX area, make sure nested
 * writers are caught early
 */
if (WARN_ON_ONCE(nest))
    goto err_put;   /* returns NULL */


rb->aux_nest is a per-ring-buffer nesting counter maintained by the perf core. Before writing anything to the per-CPU handle, perf_aux_output_begin() checks this counter. If a nested caller (NMI, BPF, or any interrupt) attempts to begin an AUX transaction on the same ring buffer while one is already in progress, it hits the WARN_ON_ONCE and returns NULL.

Thanks
Athira

> 


>> + if (!aux_buf)
>> + return 0;
> [ ... ]
>> + /*
>> + * Assess constraints regarding space remaining across the mapping
>> + * context boundary
>> + */
>> + chunk_size = htm_ctx->handle.size;
>> + chunk_size &= PAGE_MASK;
>> +
>> + if (chunk_size > (aux_buf->size - dump_offset))
>> + chunk_size = aux_buf->size - dump_offset;
> [ ... ]
>> + /* Bound transfer length tightly within the validated contiguous window */
>> + if (chunk_size > max_contiguous_bytes)
>> + chunk_size = max_contiguous_bytes;
>> +
>> + if (!chunk_size) {
>> + aux_buf->collect_htm_trace = 0;
> 
> [Severity: High]
> Does this permanently disable trace collection during a transient buffer full
> condition?
> 
> If the remaining space in the user-space perf ring buffer is temporarily
> less than PAGE_SIZE, chunk_size will evaluate to 0. This sets
> collect_htm_trace to 0, which prevents any future trace collection for the
> lifetime of the buffer.

This is good point. I will fix this in V3
> 
>> + perf_aux_output_end(&htm_ctx->handle, 0);
>> + return 0;
>> + }
>> +
> [ ... ]
>> + do {
>> + /*
>> + * Invoke H_HTM call with:
>> + * - operation as htm dump (H_HTM_OP_DUMP_DATA)
>> + * - last three values are address, size and offset
>> + */
>> + rc = htm_hcall_wrapper(htmflags, cfg.nodeindex, cfg.nodalchipindex,
>> + cfg.coreindexonchip, cfg.htmtype, H_HTM_OP_DUMP_DATA,
>> + hypervisor_target_phys, chunk_size, aux_buf->head);
>> + ret = htm_return_check(rc);
>> + } while (ret == -EBUSY && ++retries < MAX_RETRIES);
>> +
>> + if (ret > 0) {
>> + aux_buf->head += chunk_size;
>> + aux_buf->trace_records++;
>> + perf_aux_output_end(&htm_ctx->handle, chunk_size);
>> + return ret;
>> + }
>> +
>> + aux_buf->collect_htm_trace = 0;
> 
> [Severity: High]
> If the hypervisor returns a transient long-busy status (like
> H_LONG_BUSY_ORDER_1_MSEC), htm_return_check() returns -EAGAIN.
> 
> Will the loop exit and fall through to this block, permanently disabling
> tracing for this buffer session?
> 
>> + perf_aux_output_end(&htm_ctx->handle, 0);
>> + return ret;
>> +}
>> +
>> +static void htm_event_read(struct perf_event *event)
>> +{
>> + int ret;
>> +
>> + /*
>> + * Update event->count as a binary indicator:
>> + * 1 if data was dumped into the
>> + * AUX buffer, 0 otherwise. Actual trace record
>> + * decoding is left to userspace.
>> + */
>> + ret = htm_dump_sample_data(event);
>> + if (ret <= 0)
>> + local64_set(&event->count, 0);
> 
> [Severity: High]
> Since the transient buffer full condition or hypervisor busy state causes
> htm_dump_sample_data() to return 0 or -EAGAIN, does this falsely signal
> an EOF to userspace by setting event->count to 0?
> 
>> + else
>> + local64_set(&event->count, 1);
>> +}
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260720104447.11843-1-atrajeev@linux.ibm.com?part=3