[PATCH v2] iommu/msm: unwind probe state on registration failure

[PATCH v2] iommu/msm: unwind probe state on registration failure

[Weimin Xiong]

msm_iommu_probe() adds its devm-managed IOMMU object to
qcom_iommu_devices before adding the IOMMU sysfs device and registering
it with the IOMMU core.

If iommu_device_sysfs_add() fails, probe returns with the object still on
qcom_iommu_devices. The driver core then releases the devm allocation,
leaving a dangling list entry that later list walks may dereference.

If iommu_device_register() fails, the same dangling list entry remains
and the sysfs device is left registered as well.

Unwind the sysfs device and global list entry in reverse setup order on
the corresponding failure paths.

Fixes: 42df43b36163 ("iommu/msm: Make use of iommu_device_register interface")
Cc: stable@vger.kernel.org
Signed-off-by: Weimin Xiong <xiongwm2026@163.com>
---
Changes in v2:
- Resend the former patch 4/4 independently; patches 1-3 from v1 are
  dropped.
- Explain the dangling list entry and stale sysfs device consequences.
- Add Fixes and stable tags.
- No code changes.

v1: https://lore.kernel.org/r/20260714060930.220277-5-xiongwm2026@163.com

 drivers/iommu/msm_iommu.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/iommu/msm_iommu.c b/drivers/iommu/msm_iommu.c
index 0ad5ff431..d0d926be7 100644
--- a/drivers/iommu/msm_iommu.c
+++ b/drivers/iommu/msm_iommu.c
@@ -784,19 +784,25 @@ static int msm_iommu_probe(struct platform_device *pdev)
 				     "msm-smmu.%pa", &ioaddr);
 	if (ret) {
 		pr_err("Could not add msm-smmu at %pa to sysfs\n", &ioaddr);
-		return ret;
+		goto err_remove_list;
 	}
 
 	ret = iommu_device_register(&iommu->iommu, &msm_iommu_ops, &pdev->dev);
 	if (ret) {
 		pr_err("Could not register msm-smmu at %pa\n", &ioaddr);
-		return ret;
+		goto err_remove_sysfs;
 	}
 
 	pr_info("device mapped at %p, irq %d with %d ctx banks\n",
 		iommu->base, iommu->irq, iommu->ncb);
 
 	return ret;
+
+err_remove_sysfs:
+	iommu_device_sysfs_remove(&iommu->iommu);
+err_remove_list:
+	list_del(&iommu->dev_node);
+	return ret;
 }
 
 static const struct of_device_id msm_iommu_dt_match[] = {

Re: [PATCH v2] iommu/msm: unwind probe state on registration failure

[Mukesh Ojha]

On Thu, Jul 16, 2026 at 09:32:29AM +0800, Weimin Xiong wrote:
> msm_iommu_probe() adds its devm-managed IOMMU object to
> qcom_iommu_devices before adding the IOMMU sysfs device and registering
> it with the IOMMU core.
> 
> If iommu_device_sysfs_add() fails, probe returns with the object still on
> qcom_iommu_devices. The driver core then releases the devm allocation,
> leaving a dangling list entry that later list walks may dereference.
> 
> If iommu_device_register() fails, the same dangling list entry remains
> and the sysfs device is left registered as well.
> 
> Unwind the sysfs device and global list entry in reverse setup order on
> the corresponding failure paths.
> 
> Fixes: 42df43b36163 ("iommu/msm: Make use of iommu_device_register interface")
> Cc: stable@vger.kernel.org
> Signed-off-by: Weimin Xiong <xiongwm2026@163.com>
> ---
> Changes in v2:
> - Resend the former patch 4/4 independently; patches 1-3 from v1 are
>   dropped.
> - Explain the dangling list entry and stale sysfs device consequences.
> - Add Fixes and stable tags.
> - No code changes.
> 
> v1: https://lore.kernel.org/r/20260714060930.220277-5-xiongwm2026@163.com
> 
>  drivers/iommu/msm_iommu.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iommu/msm_iommu.c b/drivers/iommu/msm_iommu.c
> index 0ad5ff431..d0d926be7 100644
> --- a/drivers/iommu/msm_iommu.c
> +++ b/drivers/iommu/msm_iommu.c
> @@ -784,19 +784,25 @@ static int msm_iommu_probe(struct platform_device *pdev)
>  				     "msm-smmu.%pa", &ioaddr);
>  	if (ret) {
>  		pr_err("Could not add msm-smmu at %pa to sysfs\n", &ioaddr);
> -		return ret;
> +		goto err_remove_list;
>  	}
>  
>  	ret = iommu_device_register(&iommu->iommu, &msm_iommu_ops, &pdev->dev);
>  	if (ret) {
>  		pr_err("Could not register msm-smmu at %pa\n", &ioaddr);
> -		return ret;
> +		goto err_remove_sysfs;
>  	}
>  
>  	pr_info("device mapped at %p, irq %d with %d ctx banks\n",
>  		iommu->base, iommu->irq, iommu->ncb);
>  
>  	return ret;
> +
> +err_remove_sysfs:
> +	iommu_device_sysfs_remove(&iommu->iommu);
> +err_remove_list:
> +	list_del(&iommu->dev_node);

IMO, this can be avoided if we move the list addition to the end.

-- 
-Mukesh Ojha

[PATCH v3] iommu/msm: Fix dangling list entry on probe failure

[Weimin Xiong]

Move the list_add() call to the end of msm_iommu_probe() instead of
adding cleanup paths, which avoids the dangling list entry problem
entirely (suggested by Mukesh Ojha).

This approach simplifies the code by ensuring the iommu device is
never added to the global list until after all potential failure
points have passed.

Changes in v3:
- Move list_add() to the end instead of adding cleanup labels
  (suggested by Mukesh Ojha)

Changes in v2:
- Resend the former patch 4/4 independently; patches 1-3 from v1
  are dropped.
- Explain the dangling list entry and stale sysfs device consequences.
- Add Fixes and stable tags.
- No code changes.

Fixes: 42df43b36163 ("iommu/msm: Make use of iommu_device_register interface")
Cc: stable@vger.kernel.org
Signed-off-by: Weimin Xiong <xiongwm2026@163.com>
---
diff --git a/drivers/iommu/msm_iommu.c b/drivers/iommu/msm_iommu.c
index 0ad5ff431..9da650a82 100644
--- a/drivers/iommu/msm_iommu.c
+++ b/drivers/iommu/msm_iommu.c
@@ -778,8 +778,6 @@ static int msm_iommu_probe(struct platform_device *pdev)
 		return ret;
 	}
 
-	list_add(&iommu->dev_node, &qcom_iommu_devices);
-
 	ret = iommu_device_sysfs_add(&iommu->iommu, iommu->dev, NULL,
 				     "msm-smmu.%pa", &ioaddr);
 	if (ret) {
@@ -790,9 +788,12 @@ static int msm_iommu_probe(struct platform_device *pdev)
 	ret = iommu_device_register(&iommu->iommu, &msm_iommu_ops, &pdev->dev);
 	if (ret) {
 		pr_err("Could not register msm-smmu at %pa\n", &ioaddr);
+		iommu_device_sysfs_remove(&iommu->iommu);
 		return ret;
 	}
 
+	list_add(&iommu->dev_node, &qcom_iommu_devices);
+
 	pr_info("device mapped at %p, irq %d with %d ctx banks\n",
 		iommu->base, iommu->irq, iommu->ncb);
 
-- 
2.34.1

Re: [PATCH v3] iommu/msm: Fix dangling list entry on probe failure

[Konrad Dybcio]

On 7/16/26 10:53 AM, Weimin Xiong wrote:
> Move the list_add() call to the end of msm_iommu_probe() instead of
> adding cleanup paths, which avoids the dangling list entry problem
> entirely (suggested by Mukesh Ojha).
> 
> This approach simplifies the code by ensuring the iommu device is
> never added to the global list until after all potential failure
> points have passed.
> 
> Changes in v3:
> - Move list_add() to the end instead of adding cleanup labels
>   (suggested by Mukesh Ojha)

I think this approach introduces a race condition, because e.g.
iommu_ops->of_xlate() depends on the item being present in this list

Konrad

Re: [PATCH v3] iommu/msm: Fix dangling list entry on probe failure

[Robin Murphy]

On 16/07/2026 10:42 am, Konrad Dybcio wrote:
> On 7/16/26 10:53 AM, Weimin Xiong wrote:
>> Move the list_add() call to the end of msm_iommu_probe() instead of
>> adding cleanup paths, which avoids the dangling list entry problem
>> entirely (suggested by Mukesh Ojha).
>>
>> This approach simplifies the code by ensuring the iommu device is
>> never added to the global list until after all potential failure
>> points have passed.
>>
>> Changes in v3:
>> - Move list_add() to the end instead of adding cleanup labels
>>    (suggested by Mukesh Ojha)
> 
> I think this approach introduces a race condition, because e.g.
> iommu_ops->of_xlate() depends on the item being present in this list

Yes, the IOMMU instance must be ready to be fully functional *before* 
calling iommu_device_register(), since that can start probing and 
attaching client devices before it returns.

Thanks,
Robin.

Re: [PATCH v3] iommu/msm: Fix dangling list entry on probe failure

[Mukesh Ojha]

On Thu, Jul 16, 2026 at 01:05:57PM +0100, Robin Murphy wrote:
> On 16/07/2026 10:42 am, Konrad Dybcio wrote:
> > On 7/16/26 10:53 AM, Weimin Xiong wrote:
> > > Move the list_add() call to the end of msm_iommu_probe() instead of
> > > adding cleanup paths, which avoids the dangling list entry problem
> > > entirely (suggested by Mukesh Ojha).
> > > 
> > > This approach simplifies the code by ensuring the iommu device is
> > > never added to the global list until after all potential failure
> > > points have passed.
> > > 
> > > Changes in v3:
> > > - Move list_add() to the end instead of adding cleanup labels
> > >    (suggested by Mukesh Ojha)
> > 
> > I think this approach introduces a race condition, because e.g.
> > iommu_ops->of_xlate() depends on the item being present in this list
> 
> Yes, the IOMMU instance must be ready to be fully functional *before*
> calling iommu_device_register(), since that can start probing and attaching
> client devices before it returns.

Right..I overlooked it.

v2 is fine ..


@Weimin,  JFYI, Always send patch revision as a new thread not as a reply of
previous version.

-- 
-Mukesh Ojha

Re: [PATCH v2] iommu/msm: unwind probe state on registration failure

[Mukesh Ojha]

On Thu, Jul 16, 2026 at 09:32:29AM +0800, Weimin Xiong wrote:
> msm_iommu_probe() adds its devm-managed IOMMU object to
> qcom_iommu_devices before adding the IOMMU sysfs device and registering
> it with the IOMMU core.
> 
> If iommu_device_sysfs_add() fails, probe returns with the object still on
> qcom_iommu_devices. The driver core then releases the devm allocation,
> leaving a dangling list entry that later list walks may dereference.
> 
> If iommu_device_register() fails, the same dangling list entry remains
> and the sysfs device is left registered as well.
> 
> Unwind the sysfs device and global list entry in reverse setup order on
> the corresponding failure paths.
> 
> Fixes: 42df43b36163 ("iommu/msm: Make use of iommu_device_register interface")
> Cc: stable@vger.kernel.org
> Signed-off-by: Weimin Xiong <xiongwm2026@163.com>

Reviewed-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>

-- 
-Mukesh Ojha