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++) {
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
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
linux-stable-mirror@lists.linaro.org