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

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

[Joerg Roedel]

From: Joerg Roedel <jroedel@suse.de>

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: <stable@vger.kernel.org> # v3.18
Signed-off-by: Joerg Roedel <jroedel@suse.de>
---
 drivers/iommu/intel-iommu.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 1232336..7610121 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -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);
 		}
 	}
 
-- 
1.9.1

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

[Joerg Roedel]

From: Joerg Roedel <jroedel@suse.de>

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

Signed-off-by: Joerg Roedel <jroedel@suse.de>
---
 drivers/iommu/intel-iommu.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 7610121..40dfbc0 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -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;
-- 
1.9.1

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

[Jerry Hoemann]

On Wed, Dec 17, 2014 at 11:43:36AM +0100, Joerg Roedel wrote:
> From: Joerg Roedel <jroedel@suse.de>
> 
> 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: <stable@vger.kernel.org> # v3.18
> Signed-off-by: Joerg Roedel <jroedel@suse.de>
> ---
>  drivers/iommu/intel-iommu.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
> index 1232336..7610121 100644
> --- a/drivers/iommu/intel-iommu.c
> +++ b/drivers/iommu/intel-iommu.c
> @@ -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);
>  		}
>  	}
>  
> -- 
> 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!

Tested-by: Jerry Hoemann <jerry.hoemann@hp.com>
-- 

----------------------------------------------------------------------------
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:  jerry.hoemann@hp.com
----------------------------------------------------------------------------

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

[Jerry Hoemann]

On Wed, Dec 17, 2014 at 11:43:37AM +0100, Joerg Roedel wrote:
> From: Joerg Roedel <jroedel@suse.de>
> 
> This code only runs when action == BUS_NOTIFY_REMOVED_DEVICE,
> so it can't be BUS_NOTIFY_DEL_DEVICE.
> 
> Signed-off-by: Joerg Roedel <jroedel@suse.de>
> ---
>  drivers/iommu/intel-iommu.c | 8 --------
>  1 file changed, 8 deletions(-)
> 
> diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
> index 7610121..40dfbc0 100644
> --- a/drivers/iommu/intel-iommu.c
> +++ b/drivers/iommu/intel-iommu.c
> @@ -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;
> -- 
> 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!

Tested-by: Jerry Hoemann <jerry.hoemann@hp.com>


-- 

----------------------------------------------------------------------------
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:  jerry.hoemann@hp.com
----------------------------------------------------------------------------

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

[Joerg Roedel]

Hi Jerry,

On Wed, Dec 17, 2014 at 02:37:47PM -0700, Jerry Hoemann wrote:
> 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!
> 
> Tested-by: Jerry Hoemann <jerry.hoemann@hp.com>

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


	Joerg