[PATCH 0/3] iommu/vsi: Fix the domain locking and a probe leak

[PATCH 0/3] iommu/vsi: Fix the domain locking and a probe leak

[Arun Iyengar via B4 Relay]

Patch 1 stops identity attach and resume from locking memory past the
static identity domain. Patch 2 takes the old domain's lock to unlink
the IOMMU from its list. Patch 3 fixes a clock prepare leak in probe.
Patches 1 and 2 were found by Sashiko on my vsi-iommu-pm-ref v1 [1] and
hantro-robust v1.

Based on [1]. No conflict with vsi-iommu-disable v1 [2].

Tested on an Orange Pi 5 (RK3588S) with a v7.3-based kernel: the AV1
decoder's IOMMU group switched to identity and back to DMA, then an AV1
decode; 5 unbind/bind cycles of the decoder; VP8 and AV1 fluster with no
new failures; no splats. Patch 2's race and patch 3's error path were
not reproduced.

Developed with Claude (Anthropic), which wrote the code and the commit
messages. I reviewed every change.

[1] https://lore.kernel.org/r/20261113-vsi-iommu-pm-ref-v1-0-fd917eebdf18@gmail.com
[2] https://lore.kernel.org/r/20261113-vsi-iommu-disable-v1-1-94f7f44540b3@gmail.com

Signed-off-by: Arun Iyengar <aiyengar1990@gmail.com>
---
Arun Iyengar (3):
      iommu/vsi: Don't use the identity domain as a paging domain
      iommu/vsi: Unlink the IOMMU from its old domain under that domain's lock
      iommu/vsi: Unprepare the clocks when getting the IRQ fails

 drivers/iommu/vsi-iommu.c | 54 ++++++++++++++++++++++++++++-------------------
 1 file changed, 32 insertions(+), 22 deletions(-)
---
base-commit: 2cf4be57b301ed2d7af82e70cff5dfd96894d8de
change-id: 20261113-iommu-vsi-fixes-dc26821295cd
prerequisite-patch-id: 8b7b6e1a9abc4998ef53e7bc992d739bc1b53c94
prerequisite-patch-id: 27aee835025b1b64907a2e0e4e42e3eff120e86b

Best regards,
--  
Arun Iyengar <aiyengar1990@gmail.com>

[PATCH 1/3] iommu/vsi: Don't use the identity domain as a paging domain

[Arun Iyengar via B4 Relay]

From: Arun Iyengar <aiyengar1990@gmail.com>

vsi_identity_domain is a bare struct iommu_domain, but identity attach
and resume pass it to to_vsi_domain() and take the lock of that
vsi_iommu_domain, which is memory past the static domain. Identity
attach also unlinks the IOMMU from its old domain's list without that
domain's lock.

Take the old paging domain's lock to unlink the IOMMU, and only the
IOMMU lock otherwise. Resume touches neither the page tables nor the
domain's list, so it needs only the IOMMU lock.

Fixes: 917ace84b770 ("iommu: Add verisilicon IOMMU driver")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/patchset/20261113-vsi-iommu-pm-ref-v1-0-fd917eebdf18@gmail.com?part=1
Cc: stable@vger.kernel.org
Signed-off-by: Arun Iyengar <aiyengar1990@gmail.com>
---
 drivers/iommu/vsi-iommu.c | 45 ++++++++++++++++++++++++++-------------------
 1 file changed, 26 insertions(+), 19 deletions(-)

diff --git a/drivers/iommu/vsi-iommu.c b/drivers/iommu/vsi-iommu.c
index 3dec58224953..5156e8a2e7fc 100644
--- a/drivers/iommu/vsi-iommu.c
+++ b/drivers/iommu/vsi-iommu.c
@@ -497,31 +497,42 @@ static void vsi_iommu_disable(struct vsi_iommu *iommu)
 	iommu->enable = false;
 }
 
+/* Take the IOMMU off the list of its paging domain, if it has one. */
+static void vsi_iommu_list_del(struct vsi_iommu *iommu)
+{
+	struct vsi_iommu_domain *vsi_domain;
+	unsigned long flags;
+
+	if (!iommu->domain || iommu->domain == &vsi_identity_domain)
+		return;
+
+	vsi_domain = to_vsi_domain(iommu->domain);
+	spin_lock_irqsave(&vsi_domain->lock, flags);
+	list_del_init(&iommu->node);
+	spin_unlock_irqrestore(&vsi_domain->lock, flags);
+}
+
 static int vsi_iommu_identity_attach(struct iommu_domain *domain,
 				     struct device *dev, struct iommu_domain *old)
 {
 	struct vsi_iommu *iommu = dev_iommu_priv_get(dev);
-	struct vsi_iommu_domain *vsi_domain = to_vsi_domain(domain);
 	unsigned long flags;
 	int ret;
 
+	if (iommu->domain == domain)
+		return 0;
+
 	ret = pm_runtime_resume_and_get(iommu->dev);
 	if (ret < 0)
 		return ret;
 
-	spin_lock_irqsave(&vsi_domain->lock, flags);
-	spin_lock(&iommu->lock);
-	if (iommu->domain == domain)
-		goto unlock;
+	vsi_iommu_list_del(iommu);
 
+	spin_lock_irqsave(&iommu->lock, flags);
 	vsi_iommu_disable(iommu);
-	list_del_init(&iommu->node);
-
 	iommu->domain = domain;
+	spin_unlock_irqrestore(&iommu->lock, flags);
 
-unlock:
-	spin_unlock(&iommu->lock);
-	spin_unlock_irqrestore(&vsi_domain->lock, flags);
 	pm_runtime_put_autosuspend(iommu->dev);
 	return 0;
 }
@@ -537,11 +548,12 @@ static struct iommu_domain vsi_identity_domain = {
 
 static void vsi_iommu_enable(struct vsi_iommu *iommu, struct iommu_domain *domain)
 {
-	struct vsi_iommu_domain *vsi_domain = to_vsi_domain(domain);
+	struct vsi_iommu_domain *vsi_domain;
 
 	if (domain == &vsi_identity_domain)
 		return;
 
+	vsi_domain = to_vsi_domain(domain);
 	writel(vsi_domain->pta_dma, iommu->regs + VSI_MMU_AHB_TLB_ARRAY_BASE_L_BASE);
 	writel(VSI_MMU_OUT_OF_BOUND, iommu->regs + VSI_MMU_CONFIG1_BASE);
 	writel(VSI_MMU_BIT_ENABLE, iommu->regs + VSI_MMU_AHB_EXCEPTION_BASE);
@@ -758,15 +770,10 @@ static int __maybe_unused vsi_iommu_resume(struct device *dev)
 	if (ret)
 		return ret;
 
-	if (iommu->domain) {
-		struct vsi_iommu_domain *vsi_domain = to_vsi_domain(iommu->domain);
-
-		spin_lock_irqsave(&vsi_domain->lock, flags);
-		spin_lock(&iommu->lock);
+	spin_lock_irqsave(&iommu->lock, flags);
+	if (iommu->domain)
 		vsi_iommu_enable(iommu, iommu->domain);
-		spin_unlock(&iommu->lock);
-		spin_unlock_irqrestore(&vsi_domain->lock, flags);
-	}
+	spin_unlock_irqrestore(&iommu->lock, flags);
 
 	return 0;
 }

-- 
2.55.0

[PATCH 2/3] iommu/vsi: Unlink the IOMMU from its old domain under that domain's lock

[Arun Iyengar via B4 Relay]

From: Arun Iyengar <aiyengar1990@gmail.com>

Attaching a paging domain moves iommu->node to the new domain's list
holding only the new domain's lock. A concurrent map or unmap on the old
domain walks the old list in vsi_iommu_flush_tlb() and can follow the
moved node into the new list.

Unlink the IOMMU under the old domain's lock first.

Fixes: 917ace84b770 ("iommu: Add verisilicon IOMMU driver")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://lore.kernel.org/all/sashiko-outbox-206639@kernel.org/
Cc: stable@vger.kernel.org
Signed-off-by: Arun Iyengar <aiyengar1990@gmail.com>
---
 drivers/iommu/vsi-iommu.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/vsi-iommu.c b/drivers/iommu/vsi-iommu.c
index 5156e8a2e7fc..8a0f253f2017 100644
--- a/drivers/iommu/vsi-iommu.c
+++ b/drivers/iommu/vsi-iommu.c
@@ -575,12 +575,13 @@ static int vsi_iommu_attach_device(struct iommu_domain *domain,
 	if (ret < 0)
 		return ret;
 
+	vsi_iommu_list_del(iommu);
+
 	spin_lock_irqsave(&vsi_domain->lock, flags);
 	spin_lock(&iommu->lock);
 
 	vsi_iommu_enable(iommu, domain);
 
-	list_del_init(&iommu->node);
 	list_add_tail(&iommu->node, &vsi_domain->iommus);
 
 	iommu->domain = domain;

-- 
2.55.0

[PATCH 3/3] iommu/vsi: Unprepare the clocks when getting the IRQ fails

[Arun Iyengar via B4 Relay]

From: Arun Iyengar <aiyengar1990@gmail.com>

vsi_iommu_probe() returns without clk_bulk_unprepare() when
platform_get_irq() fails.

Fixes: 917ace84b770 ("iommu: Add verisilicon IOMMU driver")
Signed-off-by: Arun Iyengar <aiyengar1990@gmail.com>
---
 drivers/iommu/vsi-iommu.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/iommu/vsi-iommu.c b/drivers/iommu/vsi-iommu.c
index 8a0f253f2017..579eb1ab5683 100644
--- a/drivers/iommu/vsi-iommu.c
+++ b/drivers/iommu/vsi-iommu.c
@@ -707,8 +707,10 @@ static int vsi_iommu_probe(struct platform_device *pdev)
 		return err;
 
 	iommu->irq = platform_get_irq(pdev, 0);
-	if (iommu->irq < 0)
-		return iommu->irq;
+	if (iommu->irq < 0) {
+		err = iommu->irq;
+		goto err_unprepare_clocks;
+	}
 
 	err = devm_request_irq(iommu->dev, iommu->irq, vsi_iommu_irq,
 			       IRQF_SHARED, dev_name(dev), iommu);

-- 
2.55.0