Re: [libvirt] PATCH: 3/4: Fix some memory allocation bugs

["Daniel P. Berrange" <berrange@xxxxxxxxxx>]

The capabilities.c file was not checking for NULL pointers when cleaning
up after some failed allocations, and so deferencing a NULL.

The qparams.c file was not calling virRaiseError upon failure so there
wasn't any indication of why it failed

The qemu_conf.c file was not free'ing the sound card data in several
places

 capabilities.c |   15 ++++++++++++++-
 qemu_conf.c    |   10 +++++++++-
 qparams.c      |   27 +++++++++++++++++++++++----
 3 files changed, 46 insertions(+), 6 deletions(-)

Dan.

diff -r 9f962ac84b09 src/capabilities.c
--- a/src/capabilities.c	Wed May 21 19:42:55 2008 -0400
+++ b/src/capabilities.c	Wed May 21 22:22:47 2008 -0400
@@ -44,7 +44,7 @@
     virCapsPtr caps;
 
     if (VIR_ALLOC(caps) < 0)
-        goto no_memory;
+        return NULL;
 
     if ((caps->host.arch = strdup(arch)) == NULL)
         goto no_memory;
@@ -61,6 +61,9 @@
 static void
 virCapabilitiesFreeHostNUMACell(virCapsHostNUMACellPtr cell)
 {
+    if (cell == NULL)
+        return;
+
     VIR_FREE(cell->cpus);
     VIR_FREE(cell);
 }
@@ -69,6 +72,9 @@
 virCapabilitiesFreeGuestDomain(virCapsGuestDomainPtr dom)
 {
     int i;
+    if (dom == NULL)
+        return;
+
     VIR_FREE(dom->info.emulator);
     VIR_FREE(dom->info.loader);
     for (i = 0 ; i < dom->info.nmachines ; i++)
@@ -82,6 +88,8 @@
 static void
 virCapabilitiesFreeGuestFeature(virCapsGuestFeaturePtr feature)
 {
+    if (feature == NULL)
+        return;
     VIR_FREE(feature->name);
     VIR_FREE(feature);
 }
@@ -90,6 +98,9 @@
 virCapabilitiesFreeGuest(virCapsGuestPtr guest)
 {
     int i;
+    if (guest == NULL)
+        return;
+
     VIR_FREE(guest->ostype);
 
     VIR_FREE(guest->arch.name);
@@ -120,6 +131,8 @@
 void
 virCapabilitiesFree(virCapsPtr caps) {
     int i;
+    if (caps == NULL)
+        return;
 
     for (i = 0 ; i < caps->nguests ; i++)
         virCapabilitiesFreeGuest(caps->guests[i]);
diff -r 7c1231eebae9 src/qemu_conf.c
--- a/src/qemu_conf.c	Thu May 22 12:31:13 2008 -0400
+++ b/src/qemu_conf.c	Thu May 22 12:31:46 2008 -0400
@@ -215,6 +215,7 @@
     struct qemud_vm_input_def *input = def->inputs;
     struct qemud_vm_chr_def *serial = def->serials;
     struct qemud_vm_chr_def *parallel = def->parallels;
+    struct qemud_vm_sound_def *sound = def->sounds;
 
     while (disk) {
         struct qemud_vm_disk_def *prev = disk;
@@ -239,6 +240,11 @@
     while (parallel) {
         struct qemud_vm_chr_def *prev = parallel;
         parallel = parallel->next;
+        free(prev);
+    }
+    while (sound) {
+        struct qemud_vm_sound_def *prev = sound;
+        sound = sound->next;
         free(prev);
     }
     xmlFree(def->keymap);
@@ -2187,8 +2193,10 @@
                 }
                 check = check->next;
             }
-            if (collision)
+            if (collision) {
+                free(sound);
                 continue;
+            }
 
             def->nsounds++;
             sound->next = NULL;
diff -r 9f962ac84b09 src/qparams.c
--- a/src/qparams.c	Wed May 21 19:42:55 2008 -0400
+++ b/src/qparams.c	Wed May 21 22:22:47 2008 -0400
@@ -30,6 +30,14 @@
 #include "memory.h"
 #include "qparams.h"
 
+static void
+qparam_report_oom(void)
+{
+    const char *virerr = __virErrorMsg(VIR_ERR_NO_MEMORY, NULL);
+    __virRaiseError(NULL, NULL, NULL, VIR_FROM_NONE, VIR_ERR_NO_MEMORY, VIR_ERR_ERROR,
+                    virerr, NULL, NULL, -1, -1, virerr, NULL);
+}
+
 struct qparam_set *
 new_qparam_set (int init_alloc, ...)
 {
@@ -39,12 +47,15 @@
 
     if (init_alloc <= 0) init_alloc = 1;
 
-    if (VIR_ALLOC(ps) < 0)
+    if (VIR_ALLOC(ps) < 0) {
+        qparam_report_oom();
         return NULL;
+    }
     ps->n = 0;
     ps->alloc = init_alloc;
     if (VIR_ALLOC_N(ps->p, ps->alloc) < 0) {
         VIR_FREE (ps);
+        qparam_report_oom();
         return NULL;
     }
 
@@ -88,7 +99,7 @@
 {
     if (ps->n >= ps->alloc) {
         if (VIR_REALLOC_N(ps->p, ps->alloc * 2) < 0) {
-            perror ("realloc");
+            qparam_report_oom();
             return -1;
         }
         ps->alloc *= 2;
@@ -104,12 +115,15 @@
     char *pname, *pvalue;
 
     pname = strdup (name);
-    if (!pname)
+    if (!pname) {
+        qparam_report_oom();
         return -1;
+    }
 
     pvalue = strdup (value);
     if (!pvalue) {
         VIR_FREE (pname);
+        qparam_report_oom();
         return -1;
     }
 
@@ -143,6 +157,7 @@
     }
 
     if (virBufferError(&buf)) {
+        qparam_report_oom();
         return NULL;
     }
 
@@ -169,7 +184,10 @@
     const char *end, *eq;
 
     ps = new_qparam_set (0, NULL);
-    if (!ps) return NULL;
+    if (!ps) {
+        qparam_report_oom();
+        return NULL;
+    }
 
     if (!query || query[0] == '\0') return ps;
 
@@ -240,6 +258,7 @@
     return ps;
 
  out_of_memory:
+    qparam_report_oom();
     free_qparam_set (ps);
     return NULL;
 }

-- 
|: Red Hat, Engineering, Boston   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list