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.
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)
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.
1. See also once more: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Docu...
2. May the statement “ret = -EINVAL;” be moved behind an additional label?
3. Would it be helpful to append parentheses to the function name in the summary phrase?
Regards, Markus
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);
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)
linux-stable-mirror@lists.linaro.org
-
Ma Ke -
Markus Elfring -
Robin Murphy