Re: [PATCH v3 2/2] iommu/riscv: create a auxiliary device for HPM

[sashiko-bot@kernel.org]

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

New issues:
- [High] Sysfs name collision due to hardcoded auxiliary device ID.
- [Medium] Lifecycle mismatch between IOMMU teardown and PMU devres cleanup.
- [Low] Missing Kconfig dependency for CONFIG_AUXILIARY_BUS.
- [Low] Direct usage of internal double-underscored API `__devm_auxiliary_device_create`.

Pre-existing issues:
- [High] Use-After-Free in PMU driver upon manual unbind due to devres mismatch.
- [High] Resource leak of perf PMU and CPU hotplug nodes on driver unbind.
--

commit 872414a6347d3097cbc668062dca56187aba912d
Author: Zong Li <zong.li@sifive.com>

iommu/riscv: create a auxiliary device for HPM

Create an auxiliary device for HPM when the IOMMU supports a
hardware performance monitor.

> diff --git a/drivers/iommu/riscv/iommu.c b/drivers/iommu/riscv/iommu.c
> index cec3ddd7ab103..36ad4aa814651 100644
> --- a/drivers/iommu/riscv/iommu.c
> +++ b/drivers/iommu/riscv/iommu.c

[ ... ]

> @@ -565,6 +566,21 @@ static irqreturn_t riscv_iommu_fltq_process(int irq, void *data)
>  	return IRQ_HANDLED;
>  }
>  
> +/*
> + * IOMMU Hardware performance monitor
> + */
> +static int riscv_iommu_hpm_enable(struct riscv_iommu_device *iommu)
> +{
> +	struct auxiliary_device *auxdev;
> +
> +	auxdev = __devm_auxiliary_device_create(iommu->dev, KBUILD_MODNAME,

[Severity: Low]
Does this code require a Kconfig dependency for CONFIG_AUXILIARY_BUS?

Building the kernel without the auxiliary bus subsystem enabled leads to
an unresolved symbol for __devm_auxiliary_device_create.

[Severity: Low]
Is there a reason to invoke the internal __devm_auxiliary_device_create()
function directly?

The driver passes KBUILD_MODNAME and 0 manually instead of using the
standard devm_auxiliary_device_create() macro wrapper.

> +						"pmu", iommu, 0);

[Severity: High]
Will this hardcoded device ID of 0 cause a sysfs name collision for
secondary IOMMUs?

Booting a system with multiple RISC-V IOMMU instances (e.g., multiple PCIe
root complexes) will cause the first instance to register
riscv_iommu.pmu.0. The second instance passes id=0 and attempts to register
the exact same name, causing device_add() to fail with -EEXIST.

[Severity: Medium]
Does registering the PMU auxiliary device via devres here create a lifecycle
mismatch during driver removal?

In riscv_iommu_remove(), the IOMMU hardware queues and capabilities are
manually disabled before the devres-managed PMU auxiliary device is
destroyed. This asymmetry leaves the PMU driver bound to disabled hardware
during the window between remove() and devres cleanup.

[Severity: High]
This isn't a bug introduced by this patch, but enabling the auxiliary device
surfaces a use-after-free regression in the PMU driver during unbind:

drivers/perf/riscv_iommu_pmu.c:riscv_iommu_pmu_request_irq() {
    ...
    return devm_request_irq(iommu->dev, irq, riscv_iommu_pmu_irq_handler,
            IRQF_ONESHOT | IRQF_SHARED, dev_name(iommu->dev), pmu);
}

If an administrator manually unbinds the PMU driver via sysfs, the auxiliary
device's devres runs and frees the struct riscv_iommu_pmu. However, the IRQ
is incorrectly requested against the parent iommu->dev, so it is not freed.
If a PMU interrupt fires subsequently, the handler will dereference the
freed state.

[Severity: High]
This is a pre-existing issue, but enabling the PMU auxiliary device also
exposes a resource leak in the auxiliary driver definition:

drivers/perf/riscv_iommu_pmu.c:
static struct auxiliary_driver iommu_pmu_driver = {
    .probe      = riscv_iommu_pmu_probe,
    .id_table   = riscv_iommu_pmu_id_table,
};

Does the omission of the .remove callback here leave perf PMU and CPU hotplug
nodes permanently registered?

Manually unbinding the driver fails to call perf_pmu_unregister() and
cpuhp_state_remove_instance_nocalls(), leading to subsequent crashes if
userspace interacts with the dangling objects.

> +	if (!auxdev)
> +		return -ENODEV;
> +
> +	return 0;
> +}

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