linux-kernel - [PATCH RFC] iommu: fix wrong DMA ops for iommu device

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>] [day] [month] [year] [list]

Message-ID: <20241213150415.653821-1-quic_charante@quicinc.com>
Date: Fri, 13 Dec 2024 20:34:15 +0530
From: Charan Teja Kalla <quic_charante@...cinc.com>
To: <joro@...tes.org>, <will@...nel.org>, <robin.murphy@....com>,
        <jgg@...pe.ca>
CC: <iommu@...ts.linux.dev>, <linux-kernel@...r.kernel.org>,
        Charan Teja Kalla
	<quic_charante@...cinc.com>
Subject: [PATCH RFC] iommu: fix wrong DMA ops for iommu device

Race between smmu device probe and iommu device probe is resulting into
wrong DMA ops used for the device.

bus_iommu_probe(P1)                  of_iommu_configure(P2)
(from iommu_device_register)        (from insmod of a driver)
--------------------------         --------------------------
1) probe_iommu_group()
(Fills just dev->iommu_group
under iommu_probe_device_lock)
				2) iommu_probe_device():
				  (acquires the iommu_probe_device_lock,
				   but since dev->iommu_group is already
				   set, it just returns without
				   allocating the domain.)

				3) of_dma_configure_id()->
				   arch_setup_dma_ops() gets called for
				   this device, but returns with the
				   error message:
				   "Failed to set up IOMMU for device;
				    retaining platform DMA ops"

				4) device probe is succeeded where it
				   can now setup iommu mappings using
				   wrong ->dma_ops

5) domain will be allocated later
and fills iommu_group->domain.

Step 3) and 4) denies iommu device from using 'iommu_dma_ops'.

This issue does exists on 6.6 because of commit f188056352bc ("iommu:
Avoid locking/unlocking for iommu_probe_device()") without which 2)
returns only after filling the domain under group->mutex, thus no issue.

With commit b5c58b2fdc42("dma-mapping: direct calls for dma-iommu"),
->iommu_dma_ops is removed altogether and iommu api are directly being
called under "use_dma_iommu(): return dev->dma_iommu". Again,
->dma_iommu flag is set only after domain allocation. So, If
there is a sufficient delay between steps 4) and 5), issue is still
exists but that seems very narrow to me.

I am thinking, setup of the domain before returning from step2) is the
way to fix this issue. Can you please help if you have any suggestions?

Signed-off-by: Charan Teja Kalla <quic_charante@...cinc.com>
---
 drivers/iommu/iommu.c | 26 +++++++++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 3f1029c0825e..61fc8145f378 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -561,19 +561,43 @@ static int __iommu_probe_device(struct device *dev, struct list_head *group_list
 int iommu_probe_device(struct device *dev)
 {
 	const struct iommu_ops *ops;
+	struct iommu_group *group;
 	int ret;
 
 	mutex_lock(&iommu_probe_device_lock);
 	ret = __iommu_probe_device(dev, NULL);
 	mutex_unlock(&iommu_probe_device_lock);
 	if (ret)
-		return ret;
+		goto err_out;
+
+	group = iommu_group_get(dev);
+	if (!group) {
+		ret = -ENODEV;
+		goto err_release;
+	}
+
+	mutex_lock(&group->mutex);
+	if (!group->default_domain) {
+		ret = iommu_setup_default_domain(group, 0);
+		if (ret)
+			goto err_unlock;
+	}
+	mutex_unlock(&group->mutex);
+	iommu_group_put(group);
 
 	ops = dev_iommu_ops(dev);
 	if (ops->probe_finalize)
 		ops->probe_finalize(dev);
 
 	return 0;
+
+err_unlock:
+	mutex_unlock(&group->mutex);
+	iommu_group_put(group);
+err_release:
+	iommu_release_device(dev);
+err_out:
+	return ret;
 }
 
 static void __iommu_group_free_device(struct iommu_group *group,
-- 
2.34.1