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

[sashiko-bot@kernel.org]

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