[libvirt] [PATCH] vbox: fix SEGV during dumpxml of a serial port

[Laine Stump]

commit 77a12987a48 changed the "virDomainChrSourceDef source" inside
virDomainChrDef to "virDomainChrSourceDefPtr source", and started
allocating source inside virDomainChrDefNew(), but vboxDumpSerial()
was allocating a virDomainChrDef with a simple VIR_ALLOC(), so source
was never initialized, leading to a SEGV any time a serial port was
present. The same problem was created in vboxDumpParallel().

This patch changes vboxDumpSerial() and vboxDumpParallel() to use
virDomainChrDefNew() instead of VIR_ALLOC(), and makes a cursory
attempt to "recover" from OOM afterwards (much of the vbox code was
written to assume success, e.g. by having functions return void. Since
I have no way to test a more sweeping change to this code, I chose to
just short-circuit the rest of the function if virDomainChrDefNew()
fails - this is at least one step better than the previous code, which
would otherwise end up trying to dereference a NULL def->serials[i]
and crash libvirtd.

This resolves: https://bugzilla.redhat.com/1536649
---
 src/vbox/vbox_common.c | 30 ++++++++++++++++++++++++++----
 1 file changed, 26 insertions(+), 4 deletions(-)

diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c
index 33aefabe5..c5fa5f08b 100644
--- a/src/vbox/vbox_common.c
+++ b/src/vbox/vbox_common.c
@@ -3887,8 +3887,19 @@ vboxDumpSerial(virDomainDefPtr def, vboxDriverPtr data, IMachine *machine, PRUin
 
     /* Allocate memory for the serial ports which are enabled */
     if ((def->nserials > 0) && (VIR_ALLOC_N(def->serials, def->nserials) >= 0)) {
-        for (i = 0; i < def->nserials; i++)
-            ignore_value(VIR_ALLOC(def->serials[i]));
+        for (i = 0; i < def->nserials; i++) {
+           def->serials[i] = virDomainChrDefNew(NULL);
+           if (!def->serials[i]) {
+               /* there is no provision for returning an error
+                * (although the libvirtd logs will show an OOM error),
+                * but we need to at least prevent dereferencing
+                * def->serials[i] and later (including continuing in
+                * this function), as it will otherwise cause a SEGV.
+                */
+               def->nserials = i;
+               return;
+           }
+        }
     }
 
     /* Now get the details about the serial ports here */
@@ -3975,8 +3986,19 @@ vboxDumpParallel(virDomainDefPtr def, vboxDriverPtr data, IMachine *machine, PRU
 
     /* Allocate memory for the parallel ports which are enabled */
     if ((def->nparallels > 0) && (VIR_ALLOC_N(def->parallels, def->nparallels) >= 0)) {
-        for (i = 0; i < def->nparallels; i++)
-            ignore_value(VIR_ALLOC(def->parallels[i]));
+        for (i = 0; i < def->nparallels; i++) {
+           def->parallels[i] = virDomainChrDefNew(NULL);
+           if (!def->parallels[i]) {
+               /* there is no provision for returning an error
+                * (although the libvirtd logs will show an OOM error),
+                * but we need to at least prevent dereferencing
+                * def->parallels[i] and later (including continuing in
+                * this function), as it will otherwise cause a SEGV.
+                */
+               def->nparallels = i;
+               return;
+           }
+        }
     }
 
     /* Now get the details about the parallel ports here */
-- 
2.14.3