Re: [PATCH v4 1/2] drivers/perf: riscv-iommu: add risc-v iommu pmu driver

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 8 potential issue(s) to consider:
- [High] Missing `.remove` callback in the auxiliary driver and incorrect `devm` usage tie resources to the wrong lifecycles, causing Use-After-Free on device unbind.
- [High] The PMU IRQ handler fails to properly clear the interrupt source in multiple edge cases, leading to a permanent PMU lockup or an infinite interrupt storm.
- [High] The stateful counter read optimization in `riscv_iommu_pmu_get_counter` overwrites its state, causing subsequent hardware wraparounds to be completely missed.
- [High] The interrupt handler blindly restarts all allocated counters, incorrectly unpausing events that were explicitly stopped by the perf core.
- [High] Stack buffer overflow on 32-bit systems due to writing a 64-bit value into a 32-bit bitmap array.
- [Medium] Group validation logic in `riscv_iommu_pmu_event_init` fails to enforce hardware constraints and allows invalid cross-PMU combinations.
- [Medium] Wrapping expected `irq_set_affinity` failures in `WARN_ON` causes unnecessary kernel stack traces.
- [Medium] Incorrect determination of `num_counters` using `hweight32` makes the highest counter inaccessible if lower counters are omitted.
--

--- Patch [1]: [PATCH v4 1/2] drivers/perf: riscv-iommu: add risc-v iommu pmu driver ---
Note: The format of this report has been altered to provide a free-form summary
due to system recitation restrictions. No original patch code is quoted.

commit f8f3c1f0f8ae563726a542519ac8eef772529601
Author: Zong Li <zong.li@sifive.com>
Subject: drivers/perf: riscv-iommu: add risc-v iommu pmu driver

This commit adds a new auxiliary device driver to support the risc-v iommu pmu.
It implements the performance monitoring counters, including hardware detection,
filtering support, and event configuration.

Link: https://lore.kernel.org/linux-riscv/20260618143634.7f3dd6c5@pumpkin/

[Severity: High]
Does the iommu_pmu_driver auxiliary driver leak resources or cause a
use-after-free on unbind?

The driver definition is missing a remove callback to unregister the pmu and
remove the cpu hotplug instance. Furthermore, riscv_iommu_pmu_request_irq()
binds the interrupt using devm_request_irq() against the parent iommu device
rather than the auxiliary device. If a user unbinds the pmu driver via sysfs,
the struct will be freed, but the interrupt and hotplug callbacks will remain
active with dangling pointers.

[Severity: High]
Will the pmu interrupt handler cause an interrupt storm or permanent lockup on
edge cases?

In riscv_iommu_pmu_irq_handler(), if the overflow register reads as zero, the
function returns early without clearing the interrupt pending bit in the status
register and without restarting the counters. Similarly, if an event is missing
from the array, the loop uses continue and skips clearing the hardware overflow
bit. Both cases leave the interrupt pending indefinitely.

[Severity: Medium]
Does the event initialization logic properly validate groups?

In riscv_iommu_pmu_event_init(), when iterating over siblings in a group, the
code explicitly skips checking if the sibling's pmu matches the event's pmu
when the sibling is a cycle event. Additionally, the loop counts the siblings
but seems to miss accounting for the new event itself when checking against
total_event_counters, which could allow exceeding hardware limits or creating
invalid cross-pmu groups.

[Severity: Medium]
Is it intentional to trigger a kernel warning for expected affinity failures?

In both riscv_iommu_pmu_probe() and riscv_iommu_pmu_offline_cpu(), the calls
to irq_set_affinity() are wrapped in a warning macro. On systems without
multiprocessor support or with interrupt controllers that do not support
affinity routing, this function safely returns an error. Wrapping this in a
warning causes unnecessary stack traces for valid hardware configurations.

[Severity: High]
Can the counter read optimization miss hardware wraparounds?

In riscv_iommu_pmu_get_counter(), when a wraparound is detected, the code sets
the previous low value to a sentinel of all ones to force the slow path on the
next read. However, this sentinel value is immediately overwritten
unconditionally by the actual low value read, which is zero. If another wrap
occurs before the next read, the check for whether the new low value is less
than or equal to the previous low value will fail, and the wraparound will be
missed.

[Severity: Medium]
Does using hweight32() to determine the number of counters cause issues with
sparse counter implementations?

In riscv_iommu_pmu_probe(), the number of counters is derived by counting the
set bits in the inhibit register using hweight32(). If the hardware omits
some lower counters but implements higher ones, hweight32() returns a count
that is strictly less than the highest implemented index. This means the bitmap
operations will use a bound that cuts off the highest available counters,
leaving them inaccessible. Should this use fls() on the bitmask instead?

[Severity: High]
Does the interrupt handler accidentally unpause stopped events?

In riscv_iommu_pmu_start_all(), the code clears the inhibit bits for every
counter that is marked in used_counters. However, used_counters only tracks
whether a counter has been allocated to an event, not whether that event is
currently active or explicitly stopped by the perf core. If an event was
stopped for throttling or a context switch, an interrupt from another event
will cause the stopped event to unconditionally resume counting.

[Severity: High]
Can the overflow bitmap trigger a stack buffer overflow on 32-bit architectures?

In riscv_iommu_pmu_irq_handler(), ovf_bitmap is declared with a size of 32
bits. On 32-bit systems, this allocates exactly 4 bytes on the stack. The
function then calls bitmap_from_u64() to copy the overflow register into it,
which unconditionally writes 64 bits (8 bytes), overwriting 4 bytes of adjacent
stack memory.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260714083625.1083606-1-zong.li@sifive.com?part=1