Re: [PATCH 2/3] iommu/mediatek-v1: fix IOMMU device binding

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Moving `mtk_iommu_v1_create_mapping()` into `mtk_iommu_v1_probe_finalize()` introduces a data race that can corrupt the shared IOMMU mapping and domain, leading to memory leaks and incorrect domain assignments.
--

commit 506d7ce15633b781671040d746cf03c5adc6f9ca
Author: Roman Vivchar <rva333@protonmail.com>

iommu/mediatek-v1: fix IOMMU device binding

The driver silently fails to bind devices, causing DRM to fall back to PA mode.
This patch provides an of_xlate callback to properly bind devices and use VA
mode.

> diff --git a/drivers/iommu/mtk_iommu_v1.c b/drivers/iommu/mtk_iommu_v1.c
> index ac97dd2868d4b..72355f41c36af 100644
> --- a/drivers/iommu/mtk_iommu_v1.c
> +++ b/drivers/iommu/mtk_iommu_v1.c

[ ... ]

> @@ -519,9 +476,15 @@ static void mtk_iommu_v1_probe_finalize(struct device *dev)
>  	__maybe_unused struct mtk_iommu_v1_data *data = dev_iommu_priv_get(dev);
>  	int err;
>  
> +	err = mtk_iommu_v1_create_mapping(dev);

[Severity: High]
Does moving the call to mtk_iommu_v1_create_mapping() into
mtk_iommu_v1_probe_finalize() introduce a data race?

Previously, this was called from mtk_iommu_v1_probe_device(), which executes
under the iommu_probe_device_lock.  By moving it here, it executes without
synchronization.

If two client devices of the same MTK IOMMU probe concurrently (e.g., via
asynchronous probing or deferred probe), they can race in
mtk_iommu_v1_create_mapping():

drivers/iommu/mtk_iommu_v1.c:mtk_iommu_v1_create_mapping() {
        ...
        data = dev_iommu_priv_get(dev);
        mtk_mapping = data->mapping;
        if (!mtk_mapping) {
                /* MTK iommu support 4GB iova address space. */
                mtk_mapping = arm_iommu_create_mapping(dev, 0, 1ULL << 32);
                if (IS_ERR(mtk_mapping))
                        return PTR_ERR(mtk_mapping);

                data->mapping = mtk_mapping;
        }
        ...
}

Can this lead to a memory leak of the initial mapping and DMA failures for
the racing client device, as it might get attached to a software domain
that does not match the hardware page tables?

> +	if (err) {
> +		dev_err(dev, "Can't create IOMMU mapping - DMA-OPS will not work\n");
> +		return;
> +	}
> +
>  	err = arm_iommu_attach_device(dev, data->mapping);
>  	if (err)
> -		dev_err(dev, "Can't create IOMMU mapping - DMA-OPS will not work\n");
> +		dev_err(dev, "Can't attach to IOMMU mapping - DMA-OPS will not work\n");
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260909-6572-iommu-v1-0-de261da09fb2@protonmail.com?part=2