[libvirt] [PATCH] vbox: fix SEGV during dumpxml of a serial port
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