Re: [PATCH] iommu/omap: Fix error handling in omap_iommu_probe_device

[Robin Murphy <robin.murphy@arm.com>]

On 2025-11-17 3:39 am, Ma Ke wrote:
> omap_iommu_probe_device() calls of_find_device_by_node() which
> increments the reference count of the platform device, but fails to
> decrement the reference count in both success and error paths. This
> could lead to resource leakage and prevent proper device cleanup when
> the IOMMU is unbound or the device is removed.

This is already fixed by Johan's comprehensive cleanup series:

https://lore.kernel.org/linux-iommu/20251020045318.30690-1-johan@kernel.org/

Thanks,
Robin.

> Found by code review.
> 
> Cc: stable@vger.kernel.org
> Fixes: 9d5018deec86 ("iommu/omap: Add support to program multiple iommus")
> Signed-off-by: Ma Ke <make24@iscas.ac.cn>
> ---
>   drivers/iommu/omap-iommu.c | 32 +++++++++++++++++++++-----------
>   1 file changed, 21 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/iommu/omap-iommu.c b/drivers/iommu/omap-iommu.c
> index 5c6f5943f44b..4df06cb09623 100644
> --- a/drivers/iommu/omap-iommu.c
> +++ b/drivers/iommu/omap-iommu.c
> @@ -1637,6 +1637,7 @@ static struct iommu_device *omap_iommu_probe_device(struct device *dev)
>   	struct omap_iommu *oiommu;
>   	struct device_node *np;
>   	int num_iommus, i;
> +	int ret = 0;
>   
>   	/*
>   	 * Allocate the per-device iommu structure for DT-based devices.
> @@ -1663,28 +1664,26 @@ static struct iommu_device *omap_iommu_probe_device(struct device *dev)
>   	for (i = 0, tmp = arch_data; i < num_iommus; i++, tmp++) {
>   		np = of_parse_phandle(dev->of_node, "iommus", i);
>   		if (!np) {
> -			kfree(arch_data);
> -			return ERR_PTR(-EINVAL);
> +			ret = -EINVAL;
> +			goto err_cleanup;
>   		}
>   
>   		pdev = of_find_device_by_node(np);
> +		of_node_put(np);
>   		if (!pdev) {
> -			of_node_put(np);
> -			kfree(arch_data);
> -			return ERR_PTR(-ENODEV);
> +			ret = -ENODEV;
> +			goto err_cleanup;
>   		}
>   
>   		oiommu = platform_get_drvdata(pdev);
>   		if (!oiommu) {
> -			of_node_put(np);
> -			kfree(arch_data);
> -			return ERR_PTR(-EINVAL);
> +			put_device(&pdev->dev);
> +			ret = -EINVAL;
> +			goto err_cleanup;
>   		}
>   
>   		tmp->iommu_dev = oiommu;
>   		tmp->dev = &pdev->dev;
> -
> -		of_node_put(np);
>   	}
>   
>   	dev_iommu_priv_set(dev, arch_data);
> @@ -1697,17 +1696,28 @@ static struct iommu_device *omap_iommu_probe_device(struct device *dev)
>   	oiommu = arch_data->iommu_dev;
>   
>   	return &oiommu->iommu;
> +
> +err_cleanup:
> +	for (tmp = arch_data; tmp < arch_data + i; tmp++) {
> +		if (tmp->dev)
> +			put_device(tmp->dev);
> +	}
> +	kfree(arch_data);
> +	return ERR_PTR(ret);
>   }
>   
>   static void omap_iommu_release_device(struct device *dev)
>   {
>   	struct omap_iommu_arch_data *arch_data = dev_iommu_priv_get(dev);
> +	struct omap_iommu_arch_data *tmp;
>   
>   	if (!dev->of_node || !arch_data)
>   		return;
>   
> -	kfree(arch_data);
> +	for (tmp = arch_data; tmp->dev; tmp++)
> +		put_device(tmp->dev);
>   
> +	kfree(arch_data);
>   }
>   
>   static int omap_iommu_of_xlate(struct device *dev, const struct of_phandle_args *args)