Re: [PATCH 3/3] perf/arm-smmuv3: Avoid double-requesting shared SMMU MMIO for PMCG

sashiko-bot
2026-04-08T08:46:06Z

Sashiko AI review found 1 potential issue(s):
- [High] Incorrect error checking for `devm_ioremap()` return value leads to potential NULL pointer dereference.
--

commit 9648b36d883f07ba004ca17984ae1bfebc0a29df
Author: Peng Fan <peng.fan@nxp.com>

perf/arm-smmuv3: Avoid double-requesting shared SMMU MMIO for PMCG

This commit maps the PMCG memory resources directly using devm_ioremap
to avoid double-requesting the SMMU MMIO space that is already requested
by the arm-smmu-v3 driver.

> diff --git a/drivers/perf/arm_smmuv3_pmu.c b/drivers/perf/arm_smmuv3_pmu.c
> index 621f02a7f43be..1c9585ef26183 100644
> --- a/drivers/perf/arm_smmuv3_pmu.c
> +++ b/drivers/perf/arm_smmuv3_pmu.c

[ ... ]

> @@ -877,7 +877,16 @@ static int smmu_pmu_probe(struct platform_device *pdev)
> .capabilities = PERF_PMU_CAP_NO_EXCLUDE,
> };
>
> - smmu_pmu->reg_base = devm_platform_get_and_ioremap_resource(pdev, 0, &res_0);
> + res_0 = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (!res_0)
> + return dev_err_probe(dev, -ENOENT, "missing PMCG page 0 MMIO resource\n");
> +
> + /*
> + * PMCG registers might be a sub-region of the parent SMMU MMIO window,
> + * which is already requested by the SMMU driver. Do not call
> + * devm_ioremap_resource() here to avoid double-requesting the region.
> + */
> + smmu_pmu->reg_base = devm_ioremap(dev, res_0->start, resource_size(res_0));
> if (IS_ERR(smmu_pmu->reg_base))
> return PTR_ERR(smmu_pmu->reg_base);

Since devm_ioremap returns NULL on failure instead of an ERR_PTR, will this
IS_ERR check fail to catch unsuccessful mappings?

If devm_ioremap returns NULL, IS_ERR(NULL) evaluates to false, which would
allow the code to proceed and potentially dereference a NULL pointer shortly
after when reading cfgr.

Would it be better to check for !smmu_pmu->reg_base here and return
-ENOMEM?

>
> @@ -885,7 +894,11 @@ static int smmu_pmu_probe(struct platform_device *pdev)
>
> /* Determine if page 1 is present */
> if (cfgr & SMMU_PMCG_CFGR_RELOC_CTRS) {
> - smmu_pmu->reloc_base = devm_platform_ioremap_resource(pdev, 1);
> + res_1 = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> + if (!res_1)
> + return dev_err_probe(dev, -ENOENT, "missing PMCG page 1 MMIO resource\n");
> +
> + smmu_pmu->reloc_base = devm_ioremap(dev, res_1->start, resource_size(res_1));
> if (IS_ERR(smmu_pmu->reloc_base))
> return PTR_ERR(smmu_pmu->reloc_base);

This appears to have the same issue. Does this IS_ERR check also need to be
updated to handle a NULL return from devm_ioremap?

--
Sashiko AI review · https://sashiko.dev/#/patchset/20260408-smmu-perf-v1-0-d75dac96e828@nxp.com?part=3