[Xen-devel] [PATCH v4] dmar: device scope mem leak fix

[elena.ufimtseva@xxxxxxxxxx]

From: Elena Ufimtseva <elena.ufimtseva@xxxxxxxxxx>

Release memory allocated for scope.devices when disabling
dmar units. Also set device count after memory allocation when
device scope parsing.
This is explanation of why the code should be moved imho and
answers Jan question about why I needed to do this.
In acpi_parse_one_drhr move call to acpi_parse_dev_scope after include_all
check so the return value does not get overwritten by calling 
acpi_parse_dev_scope.

Signed-off-by: Elena Ufimtseva <elena.ufimtseva@xxxxxxxxxx>
---
Changes in v4:
 - make scope_devices_free safe to call with NULL scope pointer;
 - since scope_devices_free is safe to call, use it in failure path 
   in acpi_parse_one_drhd;

Changes in v3:
 - make freeing memory for scope devices and zeroing device counter
 as a function;
 - 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;
---
 xen/drivers/passthrough/vtd/dmar.c | 38 ++++++++++++++++++++++++++++++--------
 1 file changed, 30 insertions(+), 8 deletions(-)

diff --git a/xen/drivers/passthrough/vtd/dmar.c 
b/xen/drivers/passthrough/vtd/dmar.c
index 2b07be9..77ef708 100644
--- a/xen/drivers/passthrough/vtd/dmar.c
+++ b/xen/drivers/passthrough/vtd/dmar.c
@@ -81,6 +81,13 @@ static int __init acpi_register_rmrr_unit(struct 
acpi_rmrr_unit *rmrr)
     return 0;
 }
 
+static void scope_devices_free(struct dmar_scope *scope)
+{
+    if ( scope )
+        scope->devices_cnt = 0;
+    xfree(scope->devices);
+}
+
 static void __init disable_all_dmar_units(void)
 {
     struct acpi_drhd_unit *drhd, *_drhd;
@@ -90,16 +97,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 +328,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 +437,7 @@ static int __init acpi_parse_dev_scope(
 
  out:
     if ( ret )
-        xfree(scope->devices);
+        scope_devices_free(scope);
 
     return ret;
 }
@@ -474,12 +484,10 @@ acpi_parse_one_drhd(struct acpi_dmar_header *header)
 
     ret = iommu_alloc(dmaru);
     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);
+    {
+        xfree(dmaru);
+        return ret;
+    }    
 
     if ( dmaru->include_all )
     {
@@ -495,7 +503,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 +556,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);
             }
@@ -562,9 +577,11 @@ acpi_parse_one_drhd(struct acpi_dmar_header *header)
 out:
     if ( ret )
     {
+        scope_devices_free(&dmaru->scope);
         iommu_free(dmaru);
         xfree(dmaru);
     }
+
     return ret;
 }
 
@@ -658,6 +675,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 +683,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 +746,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


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel