[PATCH rc v7 6/6] iommu: Fix UAF in pci_dev_reset_iommu_done() due to concurrent detach

[Nicolin Chen]

In __iommu_group_set_domain_internal(), concurrent domain attachments are
rejected when any device in the group is recovering. This is necessary to
fence concurrent attachments to a multi-device group where devices might
share the same RID due to PCI DMA alias quirks.

However, IOMMU_SET_DOMAIN_MUST_SUCCEED callers (detach/teardown paths such
as __iommu_group_set_core_domain and __iommu_release_dma_ownership) should
not be rejected, as the domain would be free-ed anyway in this nofail path
while group->domain is still pointing to it. So pci_dev_reset_iommu_done()
could trigger a UAF when re-attaching group->domain.

Honor the IOMMU_SET_DOMAIN_MUST_SUCCEED flag, allowing the callers through
the group->recovery_cnt fence, so as to update the group->domain pointer.
Instead add gdev->blocked checks in the device iteration loop to avoid any
concurrent per-device detachment.

Fixes: c279e83953d9 ("iommu: Introduce pci_dev_reset_iommu_prepare/done()")
Cc: stable@xxxxxxxxxxxxxxx
Reported-by: Kevin Tian <kevin.tian@xxxxxxxxx>
Closes: https://lore.kernel.org/all/BN9PR11MB5276D60096EBF15C5753C4BB8C202@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx/
Signed-off-by: Nicolin Chen <nicolinc@xxxxxxxxxx>
---
 drivers/iommu/iommu.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 30ba18e613faa..fa7b04a77e469 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -2470,8 +2470,11 @@ static int __iommu_group_set_domain_internal(struct iommu_group *group,
 	/*
 	 * This is a concurrent attach during device recovery. Reject it until
 	 * pci_dev_reset_iommu_done() attaches the device to group->domain.
+	 *
+	 * Note: still allow MUST_SUCCEED callers (detach/teardown) through to
+	 * avoid UAF on domain release paths.
 	 */
-	if (group->recovery_cnt)
+	if (group->recovery_cnt && !(flags & IOMMU_SET_DOMAIN_MUST_SUCCEED))
 		return -EBUSY;
 
 	/*
@@ -2482,6 +2485,13 @@ static int __iommu_group_set_domain_internal(struct iommu_group *group,
 	 */
 	result = 0;
 	for_each_group_device(group, gdev) {
+		/*
+		 * Device under recovery is attached to group->blocking_domain.
+		 * Don't change that. pci_dev_reset_iommu_done() will re-attach
+		 * its domain to the updated group->domain, after the recovery.
+		 */
+		if (gdev->blocked)
+			continue;
 		ret = __iommu_device_set_domain(group, gdev->dev, new_domain,
 						group->domain, flags);
 		if (ret) {
@@ -2511,6 +2521,8 @@ static int __iommu_group_set_domain_internal(struct iommu_group *group,
 		/* No need to revert the last gdev that failed to set domain */
 		if (gdev == last_gdev)
 			break;
+		if (gdev->blocked)
+			continue;
 		/*
 		 * A NULL domain can happen only for first probe, in which case
 		 * we leave group->domain as NULL and let release clean
-- 
2.43.0