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

["Yong Wu (吴勇)" <Yong.Wu@mediatek.com>]

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
>