Skip to contentSkip to conversation

iommu/vt-d: Fix dmar_domain leak in iommu_attach_device

PATCH In review Joerg Roedel 3 participants 5 messages Dec 17, 2014 – Jan 5, 2015

Subsystems (2)
lore
Share & tools
lore

Joerg Roedel

author maintainer
#0 lore raw

PATCH 1/2 iommu/vt-d: Fix dmar_domain leak in iommu_attach_device

From: Joerg Roedel <[email protected]>

Since commit 1196c2f a domain is only destroyed in the notifier path if it is hot-unplugged. This caused a domain leakage in iommu_attach_device when a driver was unbound from the device and bound to VFIO. In this case the device is attached to a new domain and unlinked from the old domain. At this point nothing points to the old domain anymore and its memory is leaked.
Fix this by explicitly freeing the old domain in iommu_attach_domain.

Fixes 1196c2f (iommu/vt-d: Fix dmar_domain leak in iommu_attach_device)
Cc <[email protected]> # v3.18
---
 drivers/iommu/intel-iommu.c | 4 ++++
 1 file changed, 4 insertions(+)
drivers/iommu/intel-iommu.c +4 -0
index 1232336..7610121 100644
@@ -4428,6 +4428,10 @@ static int intel_iommu_attach_device(struct iommu_domain *domain,
 				domain_remove_one_dev_info(old_domain, dev);
 			else
 				domain_remove_dev_info(old_domain);
+
+			if (!domain_type_is_vm_or_si(old_domain) &&
+			     list_empty(&old_domain->devices))
+				domain_exit(old_domain);
 		}
 	}
 
Signature · 1 line
1.9.1
author maintainer
#1 lore raw

PATCH 2/2 iommu/vt-d: Remove dead code in device_notifier

From: Joerg Roedel <[email protected]>

This code only runs when action == BUS_NOTIFY_REMOVED_DEVICE, so it can't be BUS_NOTIFY_DEL_DEVICE.

---
 drivers/iommu/intel-iommu.c | 8 --------
 1 file changed, 8 deletions(-)
drivers/iommu/intel-iommu.c +0 -8
index 7610121..40dfbc0 100644
@@ -4029,14 +4029,6 @@ static int device_notifier(struct notifier_block *nb,
 	if (action != BUS_NOTIFY_REMOVED_DEVICE)
 		return 0;
 
-	/*
-	 * If the device is still attached to a device driver we can't
-	 * tear down the domain yet as DMA mappings may still be in use.
-	 * Wait for the BUS_NOTIFY_UNBOUND_DRIVER event to do that.
-	 */
-	if (action == BUS_NOTIFY_DEL_DEVICE && dev->driver != NULL)
-		return 0;
-
 	domain = find_domain(dev);
 	if (!domain)
 		return 0;
Signature · 1 line
1.9.1
Tested-by on v1 2/2
#2 lore raw

Re: [PATCH 2/2] iommu/vt-d: Remove dead code in device_notifier

Quoted 31 lines · Joerg Roedel

From: Joerg Roedel <[email protected]>

This code only runs when action == BUS_NOTIFY_REMOVED_DEVICE, so it can't be BUS_NOTIFY_DEL_DEVICE.

---
 drivers/iommu/intel-iommu.c | 8 --------
 1 file changed, 8 deletions(-)
drivers/iommu/intel-iommu.c +0 -8
index 7610121..40dfbc0 100644
@@ -4029,14 +4029,6 @@ static int device_notifier(struct notifier_block *nb,
 	if (action != BUS_NOTIFY_REMOVED_DEVICE)
 		return 0;
 
-	/*
-	 * If the device is still attached to a device driver we can't
-	 * tear down the domain yet as DMA mappings may still be in use.
-	 * Wait for the BUS_NOTIFY_UNBOUND_DRIVER event to do that.
-	 */
-	if (action == BUS_NOTIFY_DEL_DEVICE && dev->driver != NULL)
-		return 0;
-
 	domain = find_domain(dev);
 	if (!domain)
 		return 0;
Signature · 1 line
1.9.1

Joerg,

Tested this along w/ other patch of the set.

Instrumentation showed that the removed code wouldn't have been executed during VM power on/power off.

Thanks!

Signature · 7 lines
----------------------------------------------------------------------------
Jerry Hoemann            Software Engineer              Hewlett-Packard

3404 E Harmony Rd. MS 36                        phone:  (970) 898-1022
Ft. Collins, CO 80528                           FAX:    (970) 898-0707
                                                email:  [email protected]
----------------------------------------------------------------------------
Tested-by on v1 1/2
#3 lore raw
In reply to Joerg Roedel
Quoted 36 lines · Joerg Roedel

From: Joerg Roedel <[email protected]>

Since commit 1196c2f a domain is only destroyed in the notifier path if it is hot-unplugged. This caused a domain leakage in iommu_attach_device when a driver was unbound from the device and bound to VFIO. In this case the device is attached to a new domain and unlinked from the old domain. At this point nothing points to the old domain anymore and its memory is leaked.
Fix this by explicitly freeing the old domain in iommu_attach_domain.

Fixes 1196c2f (iommu/vt-d: Fix dmar_domain leak in iommu_attach_device)
Cc <[email protected]> # v3.18
---
 drivers/iommu/intel-iommu.c | 4 ++++
 1 file changed, 4 insertions(+)
drivers/iommu/intel-iommu.c +4 -0
index 1232336..7610121 100644
@@ -4428,6 +4428,10 @@ static int intel_iommu_attach_device(struct iommu_domain *domain,
 				domain_remove_one_dev_info(old_domain, dev);
 			else
 				domain_remove_dev_info(old_domain);
+
+			if (!domain_type_is_vm_or_si(old_domain) &&
+			     list_empty(&old_domain->devices))
+				domain_exit(old_domain);
 		}
 	}
 
Signature · 1 line
1.9.1

Joerg,

Before applying this change on a 3.18-rc7 kernel layered on a RHEL 7.0 root disk, I was able to reproduce the memory leak that Alex reported when powering on/off a VM w/ a PCI device assigned to it.

After applying this change, I don't see the memory leak anymore. Instrumentation shows the new code is being invoked during power on of the VM.

Looks good.

Thanks!

Signature · 7 lines
----------------------------------------------------------------------------
Jerry Hoemann            Software Engineer              Hewlett-Packard

3404 E Harmony Rd. MS 36                        phone:  (970) 898-1022
Ft. Collins, CO 80528                           FAX:    (970) 898-0707
                                                email:  [email protected]
----------------------------------------------------------------------------
#4 lore raw

Hi Jerry,

Quoted 9 lines · Jerry Hoemann

Before applying this change on a 3.18-rc7 kernel layered on a RHEL 7.0 root disk, I was able to reproduce the memory leak that Alex reported when powering on/off a VM w/ a PCI device assigned to it.

After applying this change, I don't see the memory leak anymore. Instrumentation shows the new code is being invoked during power on of the VM.

Quoted 5 lines

Looks good.

Thanks!

Thanks for testing, I applied the patches to my iommu/fixes branch for 3.19.

Joerg