[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <1a485744-67fc-4530-b131-304f2cb84a8c@arm.com>
Date: Mon, 17 Nov 2025 18:00:19 +0000
From: Robin Murphy <robin.murphy@....com>
To: Ma Ke <make24@...as.ac.cn>, joro@...tes.org, will@...nel.org,
t-kristo@...com, s-anna@...com
Cc: iommu@...ts.linux.dev, linux-kernel@...r.kernel.org,
akpm@...ux-foundation.org, stable@...r.kernel.org
Subject: Re: [PATCH] iommu/omap: Fix error handling in omap_iommu_probe_device
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@...r.kernel.org
> Fixes: 9d5018deec86 ("iommu/omap: Add support to program multiple iommus")
> Signed-off-by: Ma Ke <make24@...as.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)
Powered by blists - more mailing lists