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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <aeec9ee86b63ee892d84ab0232f372bdeccc780f.camel@mediatek.com>
Date: Sat, 18 Oct 2025 06:54:39 +0000
From: Yong Wu (吴勇) <Yong.Wu@...iatek.com>
To: "joro@...tes.org" <joro@...tes.org>, "will@...nel.org" <will@...nel.org>,
	"johan@...nel.org" <johan@...nel.org>
CC: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"j@...nau.net" <j@...nau.net>, "vdumpa@...dia.com" <vdumpa@...dia.com>,
	"robin.murphy@....com" <robin.murphy@....com>, "m.szyprowski@...sung.com"
	<m.szyprowski@...sung.com>, "wens@...e.org" <wens@...e.org>,
	"thierry.reding@...il.com" <thierry.reding@...il.com>,
	"stable@...r.kernel.org" <stable@...r.kernel.org>, "iommu@...ts.linux.dev"
	<iommu@...ts.linux.dev>, "matthias.bgg@...il.com" <matthias.bgg@...il.com>,
	"robin.clark@....qualcomm.com" <robin.clark@....qualcomm.com>,
	"sven@...nel.org" <sven@...nel.org>, AngeloGioacchino Del Regno
	<angelogioacchino.delregno@...labora.com>
Subject: Re: [PATCH v2 06/14] iommu/mediatek: fix device leaks on probe()

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@...r.kernel.org      # 4.6
> Cc: Yong Wu <yong.wu@...iatek.com>
> Acked-by: Robin Murphy <robin.murphy@....com>
> Signed-off-by: Johan Hovold <johan@...nel.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
>