[PATCH v3] dmar: device scope mem leak fix

* [PATCH v3] dmar: device scope mem leak fix
@ 2015-06-02 21:13 elena.ufimtseva
  2015-06-03  7:00 ` Jan Beulich
  0 siblings, 1 reply; 3+ messages in thread
From: elena.ufimtseva @ 2015-06-02 21:13 UTC (permalink / raw)
  To: xen-devel
  Cc: Elena Ufimtseva, kevin.tian, tim, jbeulich, yang.z.zhang,
	boris.ostrovsky

From: Elena Ufimtseva <elena.ufimtseva@oracle.com>

Third attempt to incorporate memory leak fix.
Thanks for comment on v2.

Release memory allocated for scope.devices when disabling
dmar units. Also set device count after memory allocation when
device scope parsing.

Changes in v3:
 - make freeing memory for scope devices and zeroing device counter
 a function and use it;
 - make sure parse_one_rmrr has memory leak fix in this patch;
 - make sure ret values are not lost acpi_parse_one_drhd;

Changes in v2:
 - release memory for devices scope on error paths in acpi_parse_one_drhd
 and acpi_parse_one_atsr and set the count to zero;

Signed-off-by: Elena Ufimtseva <elena.ufimtseva@oracle.com>
---
 xen/drivers/passthrough/vtd/dmar.c | 32 +++++++++++++++++++++++++-------
 1 file changed, 25 insertions(+), 7 deletions(-)

diff --git a/xen/drivers/passthrough/vtd/dmar.c b/xen/drivers/passthrough/vtd/dmar.c
index 2b07be9..a675bf7 100644
--- a/xen/drivers/passthrough/vtd/dmar.c
+++ b/xen/drivers/passthrough/vtd/dmar.c
@@ -81,6 +81,12 @@ static int __init acpi_register_rmrr_unit(struct acpi_rmrr_unit *rmrr)
     return 0;
 }
 
+static void scope_devices_free(struct dmar_scope *scope)
+{
+    scope->devices_cnt = 0;
+    xfree(scope->devices);
+}
+
 static void __init disable_all_dmar_units(void)
 {
     struct acpi_drhd_unit *drhd, *_drhd;
@@ -90,16 +96,19 @@ static void __init disable_all_dmar_units(void)
     list_for_each_entry_safe ( drhd, _drhd, &acpi_drhd_units, list )
     {
         list_del(&drhd->list);
+        scope_devices_free(&drhd->scope);
         xfree(drhd);
     }
     list_for_each_entry_safe ( rmrr, _rmrr, &acpi_rmrr_units, list )
     {
         list_del(&rmrr->list);
+        scope_devices_free(&rmrr->scope);
         xfree(rmrr);
     }
     list_for_each_entry_safe ( atsr, _atsr, &acpi_atsr_units, list )
     {
         list_del(&atsr->list);
+        scope_devices_free(&atsr->scope);
         xfree(atsr);
     }
 }
@@ -318,13 +327,13 @@ static int __init acpi_parse_dev_scope(
     if ( (cnt = scope_device_count(start, end)) < 0 )
         return cnt;
 
-    scope->devices_cnt = cnt;
     if ( cnt > 0 )
     {
         scope->devices = xzalloc_array(u16, cnt);
         if ( !scope->devices )
             return -ENOMEM;
     }
+    scope->devices_cnt = cnt;
 
     while ( start < end )
     {
@@ -427,7 +436,7 @@ static int __init acpi_parse_dev_scope(
 
  out:
     if ( ret )
-        xfree(scope->devices);
+        scope_devices_free(scope);
 
     return ret;
 }
@@ -476,11 +485,6 @@ acpi_parse_one_drhd(struct acpi_dmar_header *header)
     if ( ret )
         goto out;
 
-    dev_scope_start = (void *)(drhd + 1);
-    dev_scope_end = ((void *)drhd) + header->length;
-    ret = acpi_parse_dev_scope(dev_scope_start, dev_scope_end,
-                               &dmaru->scope, DMAR_TYPE, drhd->segment);
-
     if ( dmaru->include_all )
     {
         if ( iommu_verbose )
@@ -495,7 +499,13 @@ acpi_parse_one_drhd(struct acpi_dmar_header *header)
         if ( drhd->segment == 0 )
             include_all = 1;
     }
+    if ( ret )
+        goto out;
 
+    dev_scope_start = (void *)(drhd + 1);
+    dev_scope_end = ((void *)drhd) + header->length;
+    ret = acpi_parse_dev_scope(dev_scope_start, dev_scope_end,
+                               &dmaru->scope, DMAR_TYPE, drhd->segment);
     if ( ret )
         goto out;
     else if ( force_iommu || dmaru->include_all )
@@ -542,6 +552,7 @@ acpi_parse_one_drhd(struct acpi_dmar_header *header)
                     "  Workaround BIOS bug: ignore the DRHD due to all "
                     "devices under its scope are not PCI discoverable!\n");
 
+                scope_devices_free(&dmaru->scope);
                 iommu_free(dmaru);
                 xfree(dmaru);
             }
@@ -552,6 +563,7 @@ acpi_parse_one_drhd(struct acpi_dmar_header *header)
                     "its scope are not PCI discoverable! Pls try option "
                     "iommu=force or iommu=workaround_bios_bug if you "
                     "really want VT-d\n");
+                scope_devices_free(&dmaru->scope);
                 ret = -EINVAL;
             }
         }
@@ -565,6 +577,7 @@ out:
         iommu_free(dmaru);
         xfree(dmaru);
     }
+
     return ret;
 }
 
@@ -658,6 +671,7 @@ acpi_parse_one_rmrr(struct acpi_dmar_header *header)
                 "  Ignore the RMRR (%"PRIx64", %"PRIx64") due to "
                 "devices under its scope are not PCI discoverable!\n",
                 rmrru->base_address, rmrru->end_address);
+            scope_devices_free(&rmrru->scope);
             xfree(rmrru);
         }
         else if ( base_addr > end_addr )
@@ -665,6 +679,7 @@ acpi_parse_one_rmrr(struct acpi_dmar_header *header)
             dprintk(XENLOG_WARNING VTDPREFIX,
                 "  The RMRR (%"PRIx64", %"PRIx64") is incorrect!\n",
                 rmrru->base_address, rmrru->end_address);
+            scope_devices_free(&rmrru->scope);
             xfree(rmrru);
             ret = -EFAULT;
         }
@@ -727,7 +742,10 @@ acpi_parse_one_atsr(struct acpi_dmar_header *header)
     }
 
     if ( ret )
+    {
+        scope_devices_free(&atsru->scope);
         xfree(atsru);
+    }
     else
         acpi_register_atsr_unit(atsru);
     return ret;
-- 
2.1.3

^ permalink raw reply related	[flat|nested] 3+ messages in thread