[libvirt] [PATCH] Fix cd eject segfault
The cdrom eject code was trying to dereference the NULL source of an empty cdrom. Attached patch fixes this. Thanks, Cole commit 5925689b5b94b29a520dcfbc7f4f1cfa0a0a0183 Author: Cole Robinson <crobinso@dhcp-100-19-219.bos.redhat.com> Date: Thu Aug 21 17:56:25 2008 -0400 Prevent cdrom eject from segfaulting when setting new disk source. diff --git a/src/qemu_driver.c b/src/qemu_driver.c index 06fbe55..769f34f 100644 --- a/src/qemu_driver.c +++ b/src/qemu_driver.c @@ -2953,6 +2953,7 @@ static int qemudDomainChangeCDROM(virDomainPtr dom, virDomainDiskDefPtr newdisk) { struct qemud_driver *driver = (struct qemud_driver *)dom->conn->privateData; char *cmd, *reply, *safe_path; + char *newsrc = NULL; if (newdisk->src) { safe_path = qemudEscapeMonitorArg(newdisk->src); @@ -2972,6 +2973,13 @@ static int qemudDomainChangeCDROM(virDomainPtr dom, } VIR_FREE(safe_path); + newsrc = strdup(newdisk->src); + if (!newsrc) { + qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED, + "%s", _("out of memory")); + return -1; + } + } else if (asprintf(&cmd, "eject cdrom") == -1) { qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED, "%s", _("out of memory")); @@ -2982,11 +2990,17 @@ static int qemudDomainChangeCDROM(virDomainPtr dom, qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED, "%s", _("cannot change cdrom media")); VIR_FREE(cmd); + VIR_FREE(newsrc); return -1; } VIR_FREE(reply); VIR_FREE(cmd); - strcpy(olddisk->src, newdisk->src); + + VIR_FREE(olddisk->src); + if (newsrc) { + olddisk->src = newsrc; + newsrc = NULL; + } olddisk->type = newdisk->type; return 0; }
On Thu, Aug 21, 2008 at 11:20:28PM -0400, Cole Robinson wrote:
The cdrom eject code was trying to dereference the NULL source of an empty cdrom. Attached patch fixes this.
Good catch, +1 Rich. -- Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones virt-top is 'top' for virtual machines. Tiny program with many powerful monitoring features, net stats, disk stats, logging, etc. http://et.redhat.com/~rjones/virt-top
On Thu, Aug 21, 2008 at 11:20:28PM -0400, Cole Robinson wrote:
The cdrom eject code was trying to dereference the NULL source of an empty cdrom. Attached patch fixes this. @@ -2972,6 +2973,13 @@ static int qemudDomainChangeCDROM(virDomainPtr dom, } VIR_FREE(safe_path);
+ newsrc = strdup(newdisk->src); + if (!newsrc) { + qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED, + "%s", _("out of memory")); + return -1; + }
Rather than dup'ing the string here and having to deal with OOM...
+ } else if (asprintf(&cmd, "eject cdrom") == -1) { qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED, "%s", _("out of memory")); @@ -2982,11 +2990,17 @@ static int qemudDomainChangeCDROM(virDomainPtr dom, qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED, "%s", _("cannot change cdrom media")); VIR_FREE(cmd); + VIR_FREE(newsrc); return -1; } VIR_FREE(reply); VIR_FREE(cmd); - strcpy(olddisk->src, newdisk->src); + + VIR_FREE(olddisk->src); + if (newsrc) { + olddisk->src = newsrc; + newsrc = NULL; + }
Just do VIR_FREE(olddisk->src); olddisk->src = newdisk->src; newdisk->src = NULL; Daniel -- |: Red Hat, Engineering, London -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 :|
Daniel P. Berrange wrote:
On Thu, Aug 21, 2008 at 11:20:28PM -0400, Cole Robinson wrote:
The cdrom eject code was trying to dereference the NULL source of an empty cdrom. Attached patch fixes this. @@ -2972,6 +2973,13 @@ static int qemudDomainChangeCDROM(virDomainPtr dom, } VIR_FREE(safe_path);
+ newsrc = strdup(newdisk->src); + if (!newsrc) { + qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED, + "%s", _("out of memory")); + return -1; + }
Rather than dup'ing the string here and having to deal with OOM...
+ } else if (asprintf(&cmd, "eject cdrom") == -1) { qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED, "%s", _("out of memory")); @@ -2982,11 +2990,17 @@ static int qemudDomainChangeCDROM(virDomainPtr dom, qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED, "%s", _("cannot change cdrom media")); VIR_FREE(cmd); + VIR_FREE(newsrc); return -1; } VIR_FREE(reply); VIR_FREE(cmd); - strcpy(olddisk->src, newdisk->src); + + VIR_FREE(olddisk->src); + if (newsrc) { + olddisk->src = newsrc; + newsrc = NULL; + }
Just do
VIR_FREE(olddisk->src); olddisk->src = newdisk->src; newdisk->src = NULL;
Daniel
Whoops, yeah. I certainly over complicated that. Updated patch attached. Thanks, Cole diff --git a/src/qemu_driver.c b/src/qemu_driver.c index 06fbe55..21fd468 100644 --- a/src/qemu_driver.c +++ b/src/qemu_driver.c @@ -2986,7 +2986,10 @@ static int qemudDomainChangeCDROM(virDomainPtr dom, } VIR_FREE(reply); VIR_FREE(cmd); - strcpy(olddisk->src, newdisk->src); + + VIR_FREE(olddisk->src); + olddisk->src = newdisk->src; + newdisk->src = NULL; olddisk->type = newdisk->type; return 0; }
participants (3)
-
Cole Robinson -
Daniel P. Berrange -
Richard W.M. Jones