[PATCH v2 00/14] iommu: fix device leaks

[PATCH v2 00/14] iommu: fix device leaks

[Johan Hovold]

This series fixes device leaks in the iommu drivers, which pretty
consistently failed to drop the reference taken by
of_find_device_by_node() when looking up iommu platform devices.

Included are also a couple of related cleanups.

Johan


Changes in v2
 - drop omap reference sooner; remove unused pointer
 - amend exynos commit message with reference to partial fix
 - amend tegra commit message with reference to partial fix
 - add Robin's ack


Johan Hovold (14):
  iommu/apple-dart: fix device leak on of_xlate()
  iommu/qcom: fix device leak on of_xlate()
  iommu/exynos: fix device leak on of_xlate()
  iommu/ipmmu-vmsa: fix device leak on of_xlate()
  iommu/mediatek: fix device leak on of_xlate()
  iommu/mediatek: fix device leaks on probe()
  iommu/mediatek: simplify dt parsing error handling
  iommu/mediatek-v1: fix device leak on probe_device()
  iommu/mediatek-v1: fix device leaks on probe()
  iommu/mediatek-v1: add missing larb count sanity check
  iommu/omap: fix device leaks on probe_device()
  iommu/omap: simplify probe_device() error handling
  iommu/sun50i: fix device leak on of_xlate()
  iommu/tegra: fix device leak on probe_device()

 drivers/iommu/apple-dart.c              |  2 ++
 drivers/iommu/arm/arm-smmu/qcom_iommu.c | 10 +++-----
 drivers/iommu/exynos-iommu.c            |  9 +++----
 drivers/iommu/ipmmu-vmsa.c              |  2 ++
 drivers/iommu/mtk_iommu.c               | 33 +++++++++++++++++--------
 drivers/iommu/mtk_iommu_v1.c            | 28 +++++++++++++++++----
 drivers/iommu/omap-iommu.c              |  7 ++----
 drivers/iommu/omap-iommu.h              |  2 --
 drivers/iommu/sun50i-iommu.c            |  2 ++
 drivers/iommu/tegra-smmu.c              |  5 ++--
 10 files changed, 63 insertions(+), 37 deletions(-)

-- 
2.49.1

[PATCH v2 01/14] iommu/apple-dart: fix device leak on of_xlate()

[Johan Hovold]

Make sure to drop the reference taken to the iommu platform device when
looking up its driver data during of_xlate().

Fixes: 46d1fb072e76 ("iommu/dart: Add DART iommu driver")
Cc: stable@vger.kernel.org	# 5.15
Cc: Sven Peter <sven@kernel.org>
Acked-by: Robin Murphy <robin.murphy@arm.com>
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/iommu/apple-dart.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/iommu/apple-dart.c b/drivers/iommu/apple-dart.c
index 190f28d76615..1aa7c10262a8 100644
--- a/drivers/iommu/apple-dart.c
+++ b/drivers/iommu/apple-dart.c
@@ -790,6 +790,8 @@ static int apple_dart_of_xlate(struct device *dev,
 	struct apple_dart *cfg_dart;
 	int i, sid;
 
+	put_device(&iommu_pdev->dev);
+
 	if (args->args_count != 1)
 		return -EINVAL;
 	sid = args->args[0];
-- 
2.49.1

[PATCH v2 02/14] iommu/qcom: fix device leak on of_xlate()

[Johan Hovold]

Make sure to drop the reference taken to the iommu platform device when
looking up its driver data during of_xlate().

Note that commit e2eae09939a8 ("iommu/qcom: add missing put_device()
call in qcom_iommu_of_xlate()") fixed the leak in a couple of error
paths, but the reference is still leaking on success and late failures.

Fixes: 0ae349a0f33f ("iommu/qcom: Add qcom_iommu")
Cc: stable@vger.kernel.org	# 4.14: e2eae09939a8
Cc: Rob Clark <robin.clark@oss.qualcomm.com>
Cc: Yu Kuai <yukuai3@huawei.com>
Acked-by: Robin Murphy <robin.murphy@arm.com>
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/iommu/arm/arm-smmu/qcom_iommu.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/iommu/arm/arm-smmu/qcom_iommu.c b/drivers/iommu/arm/arm-smmu/qcom_iommu.c
index c5be95e56031..9c1166a3af6c 100644
--- a/drivers/iommu/arm/arm-smmu/qcom_iommu.c
+++ b/drivers/iommu/arm/arm-smmu/qcom_iommu.c
@@ -565,14 +565,14 @@ static int qcom_iommu_of_xlate(struct device *dev,
 
 	qcom_iommu = platform_get_drvdata(iommu_pdev);
 
+	put_device(&iommu_pdev->dev);
+
 	/* make sure the asid specified in dt is valid, so we don't have
 	 * to sanity check this elsewhere:
 	 */
 	if (WARN_ON(asid > qcom_iommu->max_asid) ||
-	    WARN_ON(qcom_iommu->ctxs[asid] == NULL)) {
-		put_device(&iommu_pdev->dev);
+	    WARN_ON(qcom_iommu->ctxs[asid] == NULL))
 		return -EINVAL;
-	}
 
 	if (!dev_iommu_priv_get(dev)) {
 		dev_iommu_priv_set(dev, qcom_iommu);
@@ -581,10 +581,8 @@ static int qcom_iommu_of_xlate(struct device *dev,
 		 * multiple different iommu devices.  Multiple context
 		 * banks are ok, but multiple devices are not:
 		 */
-		if (WARN_ON(qcom_iommu != dev_iommu_priv_get(dev))) {
-			put_device(&iommu_pdev->dev);
+		if (WARN_ON(qcom_iommu != dev_iommu_priv_get(dev)))
 			return -EINVAL;
-		}
 	}
 
 	return iommu_fwspec_add_ids(dev, &asid, 1);
-- 
2.49.1

[PATCH v2 03/14] iommu/exynos: fix device leak on of_xlate()

[Johan Hovold]

Make sure to drop the reference taken to the iommu platform device when
looking up its driver data during of_xlate().

Note that commit 1a26044954a6 ("iommu/exynos: add missing put_device()
call in exynos_iommu_of_xlate()") fixed the leak in a couple of error
paths, but the reference is still leaking on success.

Fixes: aa759fd376fb ("iommu/exynos: Add callback for initializing devices from device tree")
Cc: stable@vger.kernel.org	# 4.2: 1a26044954a6
Cc: Marek Szyprowski <m.szyprowski@samsung.com>
Cc: Yu Kuai <yukuai3@huawei.com>
Acked-by: Robin Murphy <robin.murphy@arm.com>
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/iommu/exynos-iommu.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
index b6edd178fe25..ce9e935cb84c 100644
--- a/drivers/iommu/exynos-iommu.c
+++ b/drivers/iommu/exynos-iommu.c
@@ -1446,17 +1446,14 @@ static int exynos_iommu_of_xlate(struct device *dev,
 		return -ENODEV;
 
 	data = platform_get_drvdata(sysmmu);
-	if (!data) {
-		put_device(&sysmmu->dev);
+	put_device(&sysmmu->dev);
+	if (!data)
 		return -ENODEV;
-	}
 
 	if (!owner) {
 		owner = kzalloc(sizeof(*owner), GFP_KERNEL);
-		if (!owner) {
-			put_device(&sysmmu->dev);
+		if (!owner)
 			return -ENOMEM;
-		}
 
 		INIT_LIST_HEAD(&owner->controllers);
 		mutex_init(&owner->rpm_lock);
-- 
2.49.1

[PATCH v2 04/14] iommu/ipmmu-vmsa: fix device leak on of_xlate()

[Johan Hovold]

Make sure to drop the reference taken to the iommu platform device when
looking up its driver data during of_xlate().

Fixes: 7b2d59611fef ("iommu/ipmmu-vmsa: Replace local utlb code with fwspec ids")
Cc: stable@vger.kernel.org	# 4.14
Cc: Magnus Damm <damm+renesas@opensource.se>
Acked-by: Robin Murphy <robin.murphy@arm.com>
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/iommu/ipmmu-vmsa.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
index ffa892f65714..02a2a55ffa0a 100644
--- a/drivers/iommu/ipmmu-vmsa.c
+++ b/drivers/iommu/ipmmu-vmsa.c
@@ -720,6 +720,8 @@ static int ipmmu_init_platform_device(struct device *dev,
 
 	dev_iommu_priv_set(dev, platform_get_drvdata(ipmmu_pdev));
 
+	put_device(&ipmmu_pdev->dev);
+
 	return 0;
 }
 
-- 
2.49.1

[PATCH v2 05/14] iommu/mediatek: fix device leak on of_xlate()

[Johan Hovold]

Make sure to drop the reference taken to the iommu platform device when
looking up its driver data during of_xlate().

Fixes: 0df4fabe208d ("iommu/mediatek: Add mt8173 IOMMU driver")
Cc: stable@vger.kernel.org	# 4.6
Cc: Yong Wu <yong.wu@mediatek.com>
Acked-by: Robin Murphy <robin.murphy@arm.com>
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/iommu/mtk_iommu.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index 0e0285348d2b..8d8e85186188 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -974,6 +974,8 @@ static int mtk_iommu_of_xlate(struct device *dev,
 			return -EINVAL;
 
 		dev_iommu_priv_set(dev, platform_get_drvdata(m4updev));
+
+		put_device(&m4updev->dev);
 	}
 
 	return iommu_fwspec_add_ids(dev, args->args, 1);
-- 
2.49.1

[PATCH v2 06/14] iommu/mediatek: fix device leaks on probe()

[Johan Hovold]

Make sure to drop the references taken to the larb devices during
probe on probe failure (e.g. probe deferral) and on driver unbind.

Note that commit 26593928564c ("iommu/mediatek: Add error path for loop
of mm_dts_parse") fixed the leaks in a couple of error paths, but the
references are still leaking on success and late failures.

Fixes: 0df4fabe208d ("iommu/mediatek: Add mt8173 IOMMU driver")
Cc: stable@vger.kernel.org	# 4.6
Cc: Yong Wu <yong.wu@mediatek.com>
Acked-by: Robin Murphy <robin.murphy@arm.com>
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/iommu/mtk_iommu.c | 24 ++++++++++++++++++------
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index 8d8e85186188..20a5ba80f983 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -1216,13 +1216,17 @@ static int mtk_iommu_mm_dts_parse(struct device *dev, struct component_match **m
 		platform_device_put(plarbdev);
 	}
 
-	if (!frst_avail_smicomm_node)
-		return -EINVAL;
+	if (!frst_avail_smicomm_node) {
+		ret = -EINVAL;
+		goto err_larbdev_put;
+	}
 
 	pcommdev = of_find_device_by_node(frst_avail_smicomm_node);
 	of_node_put(frst_avail_smicomm_node);
-	if (!pcommdev)
-		return -ENODEV;
+	if (!pcommdev) {
+		ret = -ENODEV;
+		goto err_larbdev_put;
+	}
 	data->smicomm_dev = &pcommdev->dev;
 
 	link = device_link_add(data->smicomm_dev, dev,
@@ -1230,7 +1234,8 @@ static int mtk_iommu_mm_dts_parse(struct device *dev, struct component_match **m
 	platform_device_put(pcommdev);
 	if (!link) {
 		dev_err(dev, "Unable to link %s.\n", dev_name(data->smicomm_dev));
-		return -EINVAL;
+		ret = -EINVAL;
+		goto err_larbdev_put;
 	}
 	return 0;
 
@@ -1402,8 +1407,12 @@ static int mtk_iommu_probe(struct platform_device *pdev)
 	iommu_device_sysfs_remove(&data->iommu);
 out_list_del:
 	list_del(&data->list);
-	if (MTK_IOMMU_IS_TYPE(data->plat_data, MTK_IOMMU_TYPE_MM))
+	if (MTK_IOMMU_IS_TYPE(data->plat_data, MTK_IOMMU_TYPE_MM)) {
 		device_link_remove(data->smicomm_dev, dev);
+
+		for (i = 0; i < MTK_LARB_NR_MAX; i++)
+			put_device(data->larb_imu[i].dev);
+	}
 out_runtime_disable:
 	pm_runtime_disable(dev);
 	return ret;
@@ -1423,6 +1432,9 @@ static void mtk_iommu_remove(struct platform_device *pdev)
 	if (MTK_IOMMU_IS_TYPE(data->plat_data, MTK_IOMMU_TYPE_MM)) {
 		device_link_remove(data->smicomm_dev, &pdev->dev);
 		component_master_del(&pdev->dev, &mtk_iommu_com_ops);
+
+		for (i = 0; i < MTK_LARB_NR_MAX; i++)
+			put_device(data->larb_imu[i].dev);
 	}
 	pm_runtime_disable(&pdev->dev);
 	for (i = 0; i < data->plat_data->banks_num; i++) {
-- 
2.49.1

[PATCH v2 07/14] iommu/mediatek: simplify dt parsing error handling

[Johan Hovold]

As previously documented by commit 26593928564c ("iommu/mediatek: Add
error path for loop of mm_dts_parse"), the id mapping may not be linear
so the whole larb array needs to be iterated on devicetree parsing
errors.

Simplify the loop by iterating from index zero while dropping the
redundant NULL check for consistency with later cleanups.

Also add back the comment which was removed by commit 462e768b55a2
("iommu/mediatek: Fix forever loop in error handling") to prevent anyone
from trying to optimise the loop by iterating backwards from 'i'.

Cc: Yong Wu <yong.wu@mediatek.com>
Acked-by: Robin Murphy <robin.murphy@arm.com>
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/iommu/mtk_iommu.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index 20a5ba80f983..24bb8b646edc 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -1240,11 +1240,10 @@ static int mtk_iommu_mm_dts_parse(struct device *dev, struct component_match **m
 	return 0;
 
 err_larbdev_put:
-	for (i = MTK_LARB_NR_MAX - 1; i >= 0; i--) {
-		if (!data->larb_imu[i].dev)
-			continue;
+	/* id mapping may not be linear, loop the whole array */
+	for (i = 0; i < MTK_LARB_NR_MAX; i++)
 		put_device(data->larb_imu[i].dev);
-	}
+
 	return ret;
 }
 
-- 
2.49.1

[PATCH v2 08/14] iommu/mediatek-v1: fix device leak on probe_device()

[Johan Hovold]

Make sure to drop the reference taken to the iommu platform device when
looking up its driver data during probe_device().

Fixes: b17336c55d89 ("iommu/mediatek: add support for mtk iommu generation one HW")
Cc: stable@vger.kernel.org	# 4.8
Cc: Honghui Zhang <honghui.zhang@mediatek.com>
Acked-by: Robin Murphy <robin.murphy@arm.com>
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/iommu/mtk_iommu_v1.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/iommu/mtk_iommu_v1.c b/drivers/iommu/mtk_iommu_v1.c
index 10cc0b1197e8..de9153c0a82f 100644
--- a/drivers/iommu/mtk_iommu_v1.c
+++ b/drivers/iommu/mtk_iommu_v1.c
@@ -435,6 +435,8 @@ static int mtk_iommu_v1_create_mapping(struct device *dev,
 			return -EINVAL;
 
 		dev_iommu_priv_set(dev, platform_get_drvdata(m4updev));
+
+		put_device(&m4updev->dev);
 	}
 
 	ret = iommu_fwspec_add_ids(dev, args->args, 1);
-- 
2.49.1

[PATCH v2 09/14] iommu/mediatek-v1: fix device leaks on probe()

[Johan Hovold]

Make sure to drop the references taken to the larb devices during
probe on probe failure (e.g. probe deferral) and on driver unbind.

Fixes: b17336c55d89 ("iommu/mediatek: add support for mtk iommu generation one HW")
Cc: stable@vger.kernel.org	# 4.8
Cc: Honghui Zhang <honghui.zhang@mediatek.com>
Acked-by: Robin Murphy <robin.murphy@arm.com>
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/iommu/mtk_iommu_v1.c | 23 ++++++++++++++++++-----
 1 file changed, 18 insertions(+), 5 deletions(-)

diff --git a/drivers/iommu/mtk_iommu_v1.c b/drivers/iommu/mtk_iommu_v1.c
index de9153c0a82f..44b965a2db92 100644
--- a/drivers/iommu/mtk_iommu_v1.c
+++ b/drivers/iommu/mtk_iommu_v1.c
@@ -648,8 +648,10 @@ static int mtk_iommu_v1_probe(struct platform_device *pdev)
 		struct platform_device *plarbdev;
 
 		larbnode = of_parse_phandle(dev->of_node, "mediatek,larbs", i);
-		if (!larbnode)
-			return -EINVAL;
+		if (!larbnode) {
+			ret = -EINVAL;
+			goto out_put_larbs;
+		}
 
 		if (!of_device_is_available(larbnode)) {
 			of_node_put(larbnode);
@@ -659,11 +661,14 @@ static int mtk_iommu_v1_probe(struct platform_device *pdev)
 		plarbdev = of_find_device_by_node(larbnode);
 		if (!plarbdev) {
 			of_node_put(larbnode);
-			return -ENODEV;
+			ret = -ENODEV;
+			goto out_put_larbs;
 		}
 		if (!plarbdev->dev.driver) {
 			of_node_put(larbnode);
-			return -EPROBE_DEFER;
+			put_device(&plarbdev->dev);
+			ret = -EPROBE_DEFER;
+			goto out_put_larbs;
 		}
 		data->larb_imu[i].dev = &plarbdev->dev;
 
@@ -675,7 +680,7 @@ static int mtk_iommu_v1_probe(struct platform_device *pdev)
 
 	ret = mtk_iommu_v1_hw_init(data);
 	if (ret)
-		return ret;
+		goto out_put_larbs;
 
 	ret = iommu_device_sysfs_add(&data->iommu, &pdev->dev, NULL,
 				     dev_name(&pdev->dev));
@@ -697,12 +702,17 @@ static int mtk_iommu_v1_probe(struct platform_device *pdev)
 	iommu_device_sysfs_remove(&data->iommu);
 out_clk_unprepare:
 	clk_disable_unprepare(data->bclk);
+out_put_larbs:
+	for (i = 0; i < MTK_LARB_NR_MAX; i++)
+		put_device(data->larb_imu[i].dev);
+
 	return ret;
 }
 
 static void mtk_iommu_v1_remove(struct platform_device *pdev)
 {
 	struct mtk_iommu_v1_data *data = platform_get_drvdata(pdev);
+	int i;
 
 	iommu_device_sysfs_remove(&data->iommu);
 	iommu_device_unregister(&data->iommu);
@@ -710,6 +720,9 @@ static void mtk_iommu_v1_remove(struct platform_device *pdev)
 	clk_disable_unprepare(data->bclk);
 	devm_free_irq(&pdev->dev, data->irq, data);
 	component_master_del(&pdev->dev, &mtk_iommu_v1_com_ops);
+
+	for (i = 0; i < MTK_LARB_NR_MAX; i++)
+		put_device(data->larb_imu[i].dev);
 }
 
 static int __maybe_unused mtk_iommu_v1_suspend(struct device *dev)
-- 
2.49.1

[PATCH v2 10/14] iommu/mediatek-v1: add missing larb count sanity check

[Johan Hovold]

Add the missing larb count sanity check to avoid writing beyond a fixed
sized array in case of a malformed devicetree.

Acked-by: Robin Murphy <robin.murphy@arm.com>
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/iommu/mtk_iommu_v1.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/iommu/mtk_iommu_v1.c b/drivers/iommu/mtk_iommu_v1.c
index 44b965a2db92..55d6615a41a9 100644
--- a/drivers/iommu/mtk_iommu_v1.c
+++ b/drivers/iommu/mtk_iommu_v1.c
@@ -643,6 +643,9 @@ static int mtk_iommu_v1_probe(struct platform_device *pdev)
 	if (larb_nr < 0)
 		return larb_nr;
 
+	if (larb_nr > MTK_LARB_NR_MAX)
+		return -EINVAL;
+
 	for (i = 0; i < larb_nr; i++) {
 		struct device_node *larbnode;
 		struct platform_device *plarbdev;
-- 
2.49.1

[PATCH v2 11/14] iommu/omap: fix device leaks on probe_device()

[Johan Hovold]

Make sure to drop the references taken to the iommu platform devices
when looking up their driver data during probe_device().

Note that the arch data device pointer added by commit 604629bcb505
("iommu/omap: add support for late attachment of iommu devices") has
never been used. Remove it to underline that the references are not
needed.

Fixes: 9d5018deec86 ("iommu/omap: Add support to program multiple iommus")
Fixes: 7d6827748d54 ("iommu/omap: Fix iommu archdata name for DT-based devices")
Cc: stable@vger.kernel.org	# 3.18
Cc: Suman Anna <s-anna@ti.com>
Acked-by: Robin Murphy <robin.murphy@arm.com>
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/iommu/omap-iommu.c | 2 +-
 drivers/iommu/omap-iommu.h | 2 --
 2 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/iommu/omap-iommu.c b/drivers/iommu/omap-iommu.c
index 6fb93927bdb9..b87ce129fb1f 100644
--- a/drivers/iommu/omap-iommu.c
+++ b/drivers/iommu/omap-iommu.c
@@ -1675,6 +1675,7 @@ static struct iommu_device *omap_iommu_probe_device(struct device *dev)
 		}
 
 		oiommu = platform_get_drvdata(pdev);
+		put_device(&pdev->dev);
 		if (!oiommu) {
 			of_node_put(np);
 			kfree(arch_data);
@@ -1682,7 +1683,6 @@ static struct iommu_device *omap_iommu_probe_device(struct device *dev)
 		}
 
 		tmp->iommu_dev = oiommu;
-		tmp->dev = &pdev->dev;
 
 		of_node_put(np);
 	}
diff --git a/drivers/iommu/omap-iommu.h b/drivers/iommu/omap-iommu.h
index 27697109ec79..50b39be61abc 100644
--- a/drivers/iommu/omap-iommu.h
+++ b/drivers/iommu/omap-iommu.h
@@ -88,7 +88,6 @@ struct omap_iommu {
 /**
  * struct omap_iommu_arch_data - omap iommu private data
  * @iommu_dev: handle of the OMAP iommu device
- * @dev: handle of the iommu device
  *
  * This is an omap iommu private data object, which binds an iommu user
  * to its iommu device. This object should be placed at the iommu user's
@@ -97,7 +96,6 @@ struct omap_iommu {
  */
 struct omap_iommu_arch_data {
 	struct omap_iommu *iommu_dev;
-	struct device *dev;
 };
 
 struct cr_regs {
-- 
2.49.1

[PATCH v2 12/14] iommu/omap: simplify probe_device() error handling

[Johan Hovold]

Simplify the probe_device() error handling by dropping the iommu OF node
reference sooner.

Acked-by: Robin Murphy <robin.murphy@arm.com>
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/iommu/omap-iommu.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/iommu/omap-iommu.c b/drivers/iommu/omap-iommu.c
index b87ce129fb1f..2e161d174b5a 100644
--- a/drivers/iommu/omap-iommu.c
+++ b/drivers/iommu/omap-iommu.c
@@ -1668,8 +1668,8 @@ static struct iommu_device *omap_iommu_probe_device(struct device *dev)
 		}
 
 		pdev = of_find_device_by_node(np);
+		of_node_put(np);
 		if (!pdev) {
-			of_node_put(np);
 			kfree(arch_data);
 			return ERR_PTR(-ENODEV);
 		}
@@ -1677,14 +1677,11 @@ static struct iommu_device *omap_iommu_probe_device(struct device *dev)
 		oiommu = platform_get_drvdata(pdev);
 		put_device(&pdev->dev);
 		if (!oiommu) {
-			of_node_put(np);
 			kfree(arch_data);
 			return ERR_PTR(-EINVAL);
 		}
 
 		tmp->iommu_dev = oiommu;
-
-		of_node_put(np);
 	}
 
 	dev_iommu_priv_set(dev, arch_data);
-- 
2.49.1

[PATCH v2 13/14] iommu/sun50i: fix device leak on of_xlate()

[Johan Hovold]

Make sure to drop the reference taken to the iommu platform device when
looking up its driver data during of_xlate().

Fixes: 4100b8c229b3 ("iommu: Add Allwinner H6 IOMMU driver")
Cc: stable@vger.kernel.org	# 5.8
Cc: Maxime Ripard <mripard@kernel.org>
Acked-by: Robin Murphy <robin.murphy@arm.com>
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/iommu/sun50i-iommu.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/iommu/sun50i-iommu.c b/drivers/iommu/sun50i-iommu.c
index de10b569d9a9..6306570d57db 100644
--- a/drivers/iommu/sun50i-iommu.c
+++ b/drivers/iommu/sun50i-iommu.c
@@ -839,6 +839,8 @@ static int sun50i_iommu_of_xlate(struct device *dev,
 
 	dev_iommu_priv_set(dev, platform_get_drvdata(iommu_pdev));
 
+	put_device(&iommu_pdev->dev);
+
 	return iommu_fwspec_add_ids(dev, &id, 1);
 }
 
-- 
2.49.1

[PATCH v2 14/14] iommu/tegra: fix device leak on probe_device()

[Johan Hovold]

Make sure to drop the reference taken to the iommu platform device when
looking up its driver data during probe_device().

Note that commit 9826e393e4a8 ("iommu/tegra-smmu: Fix missing
put_device() call in tegra_smmu_find") fixed the leak in an error path,
but the reference is still leaking on success.

Fixes: 891846516317 ("memory: Add NVIDIA Tegra memory controller support")
Cc: stable@vger.kernel.org	# 3.19: 9826e393e4a8
Cc: Thierry Reding <treding@nvidia.com>
Cc: Miaoqian Lin <linmq006@gmail.com>
Acked-by: Robin Murphy <robin.murphy@arm.com>
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/iommu/tegra-smmu.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
index 36cdd5fbab07..f6f26a072820 100644
--- a/drivers/iommu/tegra-smmu.c
+++ b/drivers/iommu/tegra-smmu.c
@@ -830,10 +830,9 @@ static struct tegra_smmu *tegra_smmu_find(struct device_node *np)
 		return NULL;
 
 	mc = platform_get_drvdata(pdev);
-	if (!mc) {
-		put_device(&pdev->dev);
+	put_device(&pdev->dev);
+	if (!mc)
 		return NULL;
-	}
 
 	return mc->smmu;
 }
-- 
2.49.1

Re: [PATCH v2 03/14] iommu/exynos: fix device leak on of_xlate()

[Marek Szyprowski]

On 07.10.2025 11:43, Johan Hovold wrote:
> Make sure to drop the reference taken to the iommu platform device when
> looking up its driver data during of_xlate().
>
> Note that commit 1a26044954a6 ("iommu/exynos: add missing put_device()
> call in exynos_iommu_of_xlate()") fixed the leak in a couple of error
> paths, but the reference is still leaking on success.
>
> Fixes: aa759fd376fb ("iommu/exynos: Add callback for initializing devices from device tree")
> Cc: stable@vger.kernel.org	# 4.2: 1a26044954a6
> Cc: Marek Szyprowski <m.szyprowski@samsung.com>
> Cc: Yu Kuai <yukuai3@huawei.com>
> Acked-by: Robin Murphy <robin.murphy@arm.com>
> Signed-off-by: Johan Hovold <johan@kernel.org>
Acked-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>   drivers/iommu/exynos-iommu.c | 9 +++------
>   1 file changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
> index b6edd178fe25..ce9e935cb84c 100644
> --- a/drivers/iommu/exynos-iommu.c
> +++ b/drivers/iommu/exynos-iommu.c
> @@ -1446,17 +1446,14 @@ static int exynos_iommu_of_xlate(struct device *dev,
>   		return -ENODEV;
>   
>   	data = platform_get_drvdata(sysmmu);
> -	if (!data) {
> -		put_device(&sysmmu->dev);
> +	put_device(&sysmmu->dev);
> +	if (!data)
>   		return -ENODEV;
> -	}
>   
>   	if (!owner) {
>   		owner = kzalloc(sizeof(*owner), GFP_KERNEL);
> -		if (!owner) {
> -			put_device(&sysmmu->dev);
> +		if (!owner)
>   			return -ENOMEM;
> -		}
>   
>   		INIT_LIST_HEAD(&owner->controllers);
>   		mutex_init(&owner->rpm_lock);

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

Re: [PATCH v2 14/14] iommu/tegra: fix device leak on probe_device()

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 1684 bytes --]

On Tue, Oct 07, 2025 at 11:43:27AM +0200, Johan Hovold wrote:
> Make sure to drop the reference taken to the iommu platform device when
> looking up its driver data during probe_device().
> 
> Note that commit 9826e393e4a8 ("iommu/tegra-smmu: Fix missing
> put_device() call in tegra_smmu_find") fixed the leak in an error path,
> but the reference is still leaking on success.
> 
> Fixes: 891846516317 ("memory: Add NVIDIA Tegra memory controller support")
> Cc: stable@vger.kernel.org	# 3.19: 9826e393e4a8
> Cc: Thierry Reding <treding@nvidia.com>
> Cc: Miaoqian Lin <linmq006@gmail.com>
> Acked-by: Robin Murphy <robin.murphy@arm.com>
> Signed-off-by: Johan Hovold <johan@kernel.org>
> ---
>  drivers/iommu/tegra-smmu.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
> index 36cdd5fbab07..f6f26a072820 100644
> --- a/drivers/iommu/tegra-smmu.c
> +++ b/drivers/iommu/tegra-smmu.c
> @@ -830,10 +830,9 @@ static struct tegra_smmu *tegra_smmu_find(struct device_node *np)
>  		return NULL;
>  
>  	mc = platform_get_drvdata(pdev);
> -	if (!mc) {
> -		put_device(&pdev->dev);
> +	put_device(&pdev->dev);
> +	if (!mc)
>  		return NULL;
> -	}
>  
>  	return mc->smmu;

pdev->dev is what's backing mc, so if we use put_device() here, then the
MC could go away at any time, right?

So the goal here was to make sure that the MC stays around during the
entire lifetime of the IOMMU attachment. We don't currently release that
reference, ever, so there is a leak, but wouldn't it be more appropriate
to release it in a .release_device implementation?

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2 14/14] iommu/tegra: fix device leak on probe_device()

[Johan Hovold]

[-- Attachment #1: Type: text/plain, Size: 884 bytes --]

On Thu, Oct 09, 2025 at 09:56:18AM +0200, Thierry Reding wrote:
> On Tue, Oct 07, 2025 at 11:43:27AM +0200, Johan Hovold wrote:

> > @@ -830,10 +830,9 @@ static struct tegra_smmu *tegra_smmu_find(struct device_node *np)
> >  		return NULL;
> >  
> >  	mc = platform_get_drvdata(pdev);
> > -	if (!mc) {
> > -		put_device(&pdev->dev);
> > +	put_device(&pdev->dev);
> > +	if (!mc)
> >  		return NULL;
> > -	}
> >  
> >  	return mc->smmu;
> 
> pdev->dev is what's backing mc, so if we use put_device() here, then the
> MC could go away at any time, right?

Holding a reference to a device does not prevent its driver data from
going away so there is no point in keeping the reference.

But from what I can tell, you don't need to worry about that anyway
since it's the memory controller driver that registers the iommu (and
the driver can't be unbound).

Johan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v2 14/14] iommu/tegra: fix device leak on probe_device()

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 1283 bytes --]

On Thu, Oct 09, 2025 at 10:27:55AM +0200, Johan Hovold wrote:
> On Thu, Oct 09, 2025 at 09:56:18AM +0200, Thierry Reding wrote:
> > On Tue, Oct 07, 2025 at 11:43:27AM +0200, Johan Hovold wrote:
> 
> > > @@ -830,10 +830,9 @@ static struct tegra_smmu *tegra_smmu_find(struct device_node *np)
> > >  		return NULL;
> > >  
> > >  	mc = platform_get_drvdata(pdev);
> > > -	if (!mc) {
> > > -		put_device(&pdev->dev);
> > > +	put_device(&pdev->dev);
> > > +	if (!mc)
> > >  		return NULL;
> > > -	}
> > >  
> > >  	return mc->smmu;
> > 
> > pdev->dev is what's backing mc, so if we use put_device() here, then the
> > MC could go away at any time, right?
> 
> Holding a reference to a device does not prevent its driver data from
> going away so there is no point in keeping the reference.
> 
> But from what I can tell, you don't need to worry about that anyway
> since it's the memory controller driver that registers the iommu (and
> the driver can't be unbound).

That's true. It'd be nice to at least conceptually do the right thing
here, but not sure it's worth it. As you said, driver data going away
would need special handling and it's not even clear what that would
mean in terms of the clients...

Acked-by: Thierry Reding <treding@nvidia.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2 05/14] iommu/mediatek: fix device leak on of_xlate()

[Yong Wu (吴勇)]

On Tue, 2025-10-07 at 11:43 +0200, Johan Hovold wrote:
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> 
> 
> Make sure to drop the reference taken to the iommu platform device
> when
> looking up its driver data during of_xlate().
> 
> Fixes: 0df4fabe208d ("iommu/mediatek: Add mt8173 IOMMU driver")
> Cc: stable@vger.kernel.org      # 4.6
> Cc: Yong Wu <yong.wu@mediatek.com>
> Acked-by: Robin Murphy <robin.murphy@arm.com>
> Signed-off-by: Johan Hovold <johan@kernel.org>

Reviewed-by: Yong Wu <yong.wu@mediatek.com>

Re: [PATCH v2 08/14] iommu/mediatek-v1: fix device leak on probe_device()

[Yong Wu (吴勇)]

On Tue, 2025-10-07 at 11:43 +0200, Johan Hovold wrote:
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> 
> 
> Make sure to drop the reference taken to the iommu platform device
> when
> looking up its driver data during probe_device().
> 
> Fixes: b17336c55d89 ("iommu/mediatek: add support for mtk iommu
> generation one HW")
> Cc: stable@vger.kernel.org      # 4.8
> Cc: Honghui Zhang <honghui.zhang@mediatek.com>
> Acked-by: Robin Murphy <robin.murphy@arm.com>
> Signed-off-by: Johan Hovold <johan@kernel.org>

Reviewed-by: Yong Wu <yong.wu@mediatek.com>

Re: [PATCH v2 10/14] iommu/mediatek-v1: add missing larb count sanity check

[Yong Wu (吴勇)]

On Tue, 2025-10-07 at 11:43 +0200, Johan Hovold wrote:
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> 
> 
> Add the missing larb count sanity check to avoid writing beyond a
> fixed
> sized array in case of a malformed devicetree.
> 
> Acked-by: Robin Murphy <robin.murphy@arm.com>
> Signed-off-by: Johan Hovold <johan@kernel.org>

Reviewed-by: Yong Wu <yong.wu@mediatek.com>

Re: [PATCH v2 06/14] iommu/mediatek: fix device leaks on probe()

[Yong Wu (吴勇)]

On Tue, 2025-10-07 at 11:43 +0200, Johan Hovold wrote:
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> 
> 
> Make sure to drop the references taken to the larb devices during
> probe on probe failure (e.g. probe deferral) and on driver unbind.
> 
> Note that commit 26593928564c ("iommu/mediatek: Add error path for
> loop
> of mm_dts_parse") fixed the leaks in a couple of error paths, but the
> references are still leaking on success and late failures.
> 
> Fixes: 0df4fabe208d ("iommu/mediatek: Add mt8173 IOMMU driver")
> Cc: stable@vger.kernel.org      # 4.6
> Cc: Yong Wu <yong.wu@mediatek.com>
> Acked-by: Robin Murphy <robin.murphy@arm.com>
> Signed-off-by: Johan Hovold <johan@kernel.org>
> ---
>  drivers/iommu/mtk_iommu.c | 24 ++++++++++++++++++------
>  1 file changed, 18 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
> index 8d8e85186188..20a5ba80f983 100644
> --- a/drivers/iommu/mtk_iommu.c
> +++ b/drivers/iommu/mtk_iommu.c
> @@ -1216,13 +1216,17 @@ static int mtk_iommu_mm_dts_parse(struct
> device *dev, struct component_match **m
>                 platform_device_put(plarbdev);
>         }
> 
> -       if (!frst_avail_smicomm_node)
> -               return -EINVAL;
> +       if (!frst_avail_smicomm_node) {
> +               ret = -EINVAL;
> +               goto err_larbdev_put;

There already is a "platform_device_put(plarbdev);" at the end of "for"
loop, then no need put_device for it outside the "for" loop or outside
this function?

Thanks.

> +       }
> 
>         pcommdev = of_find_device_by_node(frst_avail_smicomm_node);
>         of_node_put(frst_avail_smicomm_node);
> -       if (!pcommdev)
> -               return -ENODEV;
> +       if (!pcommdev) {
> +               ret = -ENODEV;
> +               goto err_larbdev_put;
> +       }
>         data->smicomm_dev = &pcommdev->dev;
> 
>         link = device_link_add(data->smicomm_dev, dev,
> @@ -1230,7 +1234,8 @@ static int mtk_iommu_mm_dts_parse(struct device
> *dev, struct component_match **m
>         platform_device_put(pcommdev);
>         if (!link) {
>                 dev_err(dev, "Unable to link %s.\n", dev_name(data-
> >smicomm_dev));
> -               return -EINVAL;
> +               ret = -EINVAL;
> +               goto err_larbdev_put;
>         }
>         return 0;
> 
> @@ -1402,8 +1407,12 @@ static int mtk_iommu_probe(struct
> platform_device *pdev)
>         iommu_device_sysfs_remove(&data->iommu);
>  out_list_del:
>         list_del(&data->list);
> -       if (MTK_IOMMU_IS_TYPE(data->plat_data, MTK_IOMMU_TYPE_MM))
> +       if (MTK_IOMMU_IS_TYPE(data->plat_data, MTK_IOMMU_TYPE_MM)) {
>                 device_link_remove(data->smicomm_dev, dev);
> +
> +               for (i = 0; i < MTK_LARB_NR_MAX; i++)
> +                       put_device(data->larb_imu[i].dev);
> +       }
>  out_runtime_disable:
>         pm_runtime_disable(dev);
>         return ret;
> @@ -1423,6 +1432,9 @@ static void mtk_iommu_remove(struct
> platform_device *pdev)
>         if (MTK_IOMMU_IS_TYPE(data->plat_data, MTK_IOMMU_TYPE_MM)) {
>                 device_link_remove(data->smicomm_dev, &pdev->dev);
>                 component_master_del(&pdev->dev, &mtk_iommu_com_ops);
> +
> +               for (i = 0; i < MTK_LARB_NR_MAX; i++)
> +                       put_device(data->larb_imu[i].dev);
>         }
>         pm_runtime_disable(&pdev->dev);
>         for (i = 0; i < data->plat_data->banks_num; i++) {
> --
> 2.49.1
> 

Re: [PATCH v2 06/14] iommu/mediatek: fix device leaks on probe()

[Johan Hovold]

On Sat, Oct 18, 2025 at 06:54:39AM +0000, Yong Wu (吴勇) wrote:
> On Tue, 2025-10-07 at 11:43 +0200, Johan Hovold wrote:

> > Make sure to drop the references taken to the larb devices during
> > probe on probe failure (e.g. probe deferral) and on driver unbind.
> > 
> > Note that commit 26593928564c ("iommu/mediatek: Add error path for
> > loop
> > of mm_dts_parse") fixed the leaks in a couple of error paths, but the
> > references are still leaking on success and late failures.

> > @@ -1216,13 +1216,17 @@ static int mtk_iommu_mm_dts_parse(struct
> > device *dev, struct component_match **m
> >                 platform_device_put(plarbdev);
> >         }
> > 
> > -       if (!frst_avail_smicomm_node)
> > -               return -EINVAL;
> > +       if (!frst_avail_smicomm_node) {
> > +               ret = -EINVAL;
> > +               goto err_larbdev_put;
> 
> There already is a "platform_device_put(plarbdev);" at the end of "for"
> loop, then no need put_device for it outside the "for" loop or outside
> this function?

You're right, thanks for catching that.

But this means that we have an existing potential use-after-free as if,
for example, the driver probe defers we would put the reference to any
previously looked up larbs twice.

I've just sent a v3 which fixes this by dropping the
platform_device_put() after successful lookup as it is expected that the
driver keeps the references while it uses the larb devices:

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

Johan