- Devel - libvirt List Archives

[libvirt] [PATCH] Avoid use of free'd memory in auto destroy callback
by Daniel P. Berrange 23 Jan '13

23 Jan '13

From: "Daniel P. Berrange" <berrange(a)redhat.com> The autodestroy callback code has the following function called from a hash iterator qemuDriverCloseCallbackRun(void *payload, const void *name, void *opaque) { ... char *uuidstr = name ... dom = closeDef->cb(data->driver, dom, data->conn); if (dom) virObjectUnlock(dom); virHashRemoveEntry(data->driver->closeCallbacks, uuidstr); } The closeDef->cb function may well cause the current callback to be removed, if it shuts down 'dom'. As such the use of 'uuidstr' in virHashRemoveEntry is accessing free'd memory. We must make a copy of the uuid str before invoking the callback to be safe. Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com> --- src/qemu/qemu_conf.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index b21392e..ee48333 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -510,20 +510,25 @@ qemuDriverCloseCallbackRun(void *payload, { struct qemuDriverCloseCallbackData *data = opaque; qemuDriverCloseDefPtr closeDef = payload; - const char *uuidstr = name; unsigned char uuid[VIR_UUID_BUFLEN]; + char uuidstr[VIR_UUID_STRING_BUFLEN]; virDomainObjPtr dom; VIR_DEBUG("conn=%p, thisconn=%p, uuid=%s, cb=%p", - closeDef->conn, data->conn, uuidstr, closeDef->cb); + closeDef->conn, data->conn, (const char *)name, closeDef->cb); if (data->conn != closeDef->conn || !closeDef->cb) return; - if (virUUIDParse(uuidstr, uuid) < 0) { - VIR_WARN("Failed to parse %s", uuidstr); + if (virUUIDParse(name, uuid) < 0) { + VIR_WARN("Failed to parse %s", (const char *)name); return; } + /* We need to reformat uuidstr, because closeDef->cb + * might cause the current hash entry to be removed, + * which means 'name' will have been free()d + */ + virUUIDFormat(uuid, uuidstr); if (!(dom = virDomainFindByUUID(&data->driver->domains, uuid))) { VIR_DEBUG("No domain object with UUID %s", uuidstr); -- 1.8.1

3 2

[libvirt] [PATCH] Fix leak of securityDriverNames
by Daniel P. Berrange 23 Jan '13

23 Jan '13

From: "Daniel P. Berrange" <berrange(a)redhat.com> When shutting down, the QEMU driver forgot to free the securityDriverNames string list Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com> --- src/qemu/qemu_driver.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 72907d2..b3d6199 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1134,6 +1134,9 @@ qemuShutdown(void) { VIR_FREE(qemu_driver->saveImageFormat); VIR_FREE(qemu_driver->dumpImageFormat); + for (i = 0 ; qemu_driver->securityDriverNames[i] != NULL ; i++) + VIR_FREE(qemu_driver->securityDriverNames[i]); + VIR_FREE(qemu_driver->securityDriverNames); virSecurityManagerFree(qemu_driver->securityManager); ebtablesContextFree(qemu_driver->ebtables); -- 1.8.1

2 1

[libvirt] ssh not working with libvirt
by varun bhatnagar 23 Jan '13

23 Jan '13

Hi, I am having issue while doing *ssh* to a virtual node through libvirt. The nodes's ip address is 192.168.82.1. When I am giving the command it is giving the following error message: *virsh# connect vbox+ssh://192.168.82.1/session* *Password:* *error: Failed to connect to the hypervisor* *error: End of ifle while reading data: sh: nc: command not found: Input/output error.* Can anyone tell me why am I facing this problem and how to resolve it. Thanks in advance.

2 1

[libvirt] [PATCHv2 0/2] Fix docs for virBitmapParse
by Peter Krempa 23 Jan '13

23 Jan '13

Split out from the other series. Peter Krempa (2): util: Fix docs for virBitmapParse conf: Fix usage of virBitmapParse src/conf/network_conf.c | 2 +- src/util/virbitmap.c | 23 +++++++++++++++-------- 2 files changed, 16 insertions(+), 9 deletions(-) -- 1.8.1.1

2 5

[libvirt] [PATCH] Have a colorful nodedev.rng with vi
by Osier Yang 23 Jan '13

23 Jan '13

Just add the head line to let vi known it's a XML document. --- docs/schemas/nodedev.rng | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/docs/schemas/nodedev.rng b/docs/schemas/nodedev.rng index 7c85815..04d8ee9 100644 --- a/docs/schemas/nodedev.rng +++ b/docs/schemas/nodedev.rng @@ -1,3 +1,4 @@ +<?xml version="1.0"?>  <grammar xmlns="http://relaxng.org/ns/structure/1.0" datatypeLibrary="http://www.w3.org/2001/XMLSchema-datatypes"> -- 1.7.7.6

2 3

[libvirt] [PATCH v2] qemu_agent: Ignore expected EOFs
by Michal Privoznik 23 Jan '13

23 Jan '13

https://bugzilla.redhat.com/show_bug.cgi?id=892079 One of my previous patches (f2a4e5f176c408) tried to fix crashing libvirtd on domain detroy. However, we need to copy pattern from qemuProcessHandleMonitorEOF() instead of decrementing reference counter. The rationale for this is, if qemu process is dying due to domain being destroyed, we obtain EOF on both the monitor and agent sockets. However, if the exit is expected, qemuProcessStop is called, which cleans both agent and monitor sockets up. We want qemuAgentClose() to be called iff the EOF is not expected, so we don't leak an FD and memory. --- diff to v1: -handle race with qemuProcessHandleMonitorEOF correctly src/qemu/qemu_process.c | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 2f08215..013b2ca 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -133,14 +133,30 @@ qemuProcessHandleAgentEOF(qemuAgentPtr agent, virObjectLock(vm); priv = vm->privateData; - if (priv->agent == agent && - !virObjectUnref(priv->agent)) + + if (!priv->agent) { + VIR_DEBUG("Agent freed already"); + goto unlock; + } + + if (priv->beingDestroyed) { + VIR_DEBUG("Domain is being destroyed, agent EOF is expected"); + goto unlock; + } + + if (priv->agent == agent) priv->agent = NULL; virObjectUnlock(vm); qemuDriverUnlock(driver); qemuAgentClose(agent); + return; + +unlock: + virObjectUnlock(vm); + qemuDriverUnlock(driver); + return; } -- 1.8.0.2

2 2

[libvirt] [PATCH 0/6] More Coverity changes
by John Ferlan 23 Jan '13

23 Jan '13

Like a gift that keeps on giving... More Coverity changes. Adjust changes from earlier based on Eric's comments/feedback. nodeinfo.c - use sa_assert() instead of comment: https://www.redhat.com/archives/libvir-list/2013-January/msg01578.html virnetclient.c - remove change - the VIR_FREE change resolves this: https://www.redhat.com/archives/libvir-list/2013-January/msg01581.html Two new Coverity warnings popped up today (lxc_driver.c & lxc_process.c). No rhyme or reason, they just were new with no recent change to blame (very strange). I missed the virnettlscontexttest.c and it's VIR_FREE() warning this morning because it's before all the BAD_SIZEOF. The virbitmaptest.c change was on another branch waiting - I just moved it here. One more major change and some fixups still due on prior changes and the number of Coverity warnings goes below 50 of which 35 are related to the sdt macros (started at 297). John Ferlan (6): nodeinfo: Use sa_assert() instead of Coverity error tag tests: Remove VIR_FREE() on static/stack buffer (der.data) lxc_driver: Need to check for vm before calling virDomainUnlock(vm) lxc_process: Avoid passing NULL iface->iname rpc: Revert Coverity tag message virbitmaptest: Resolve Coverity errors src/lxc/lxc_driver.c | 3 ++- src/lxc/lxc_process.c | 14 ++++++++------ src/nodeinfo.c | 4 ++-- src/rpc/virnetclient.c | 1 - tests/virbitmaptest.c | 24 ++++++++++++++++-------- tests/virnettlscontexttest.c | 3 --- 6 files changed, 28 insertions(+), 21 deletions(-) -- 1.7.11.7

3 14

[libvirt] [PATCH v2] parallels: Resolve some resource leaks
by John Ferlan 23 Jan '13

23 Jan '13

Be sure to VIR_FREE(accel) and moved virDomainVideoDefFree() within no_memory label to be consistent Resolve resource leak in parallelsApplyIfaceParams() when the 'oldnet' is allocated locally. Also virCommandFree(cmd) as necessary. --- This is a followup patch to: https://www.redhat.com/archives/libvir-list/2013-January/msg01565.html Changes in v2: Change label from cleanup to error Add calls to virCommandFree() src/parallels/parallels_driver.c | 40 +++++++++++++++++++++++++--------------- 1 file changed, 25 insertions(+), 15 deletions(-) diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c index 1b47246..2b79939 100644 --- a/src/parallels/parallels_driver.c +++ b/src/parallels/parallels_driver.c @@ -271,17 +271,17 @@ parallelsAddVideoInfo(virDomainDefPtr def, virJSONValuePtr value) if (!(tmp = virJSONValueObjectGetString(value, "size"))) { parallelsParseError(); - goto cleanup; + goto error; } if (virStrToLong_ul(tmp, &endptr, 10, &mem) < 0) { parallelsParseError(); - goto cleanup; + goto error; } if (!STREQ(endptr, "Mb")) { parallelsParseError(); - goto cleanup; + goto error; } if (VIR_ALLOC(video) < 0) @@ -304,8 +304,9 @@ parallelsAddVideoInfo(virDomainDefPtr def, virJSONValuePtr value) no_memory: virReportOOMError(); -cleanup: + VIR_FREE(accel); virDomainVideoDefFree(video); +error: return -1; } @@ -1793,7 +1794,7 @@ static int parallelsApplyIfaceParams(parallelsDomObjPtr pdom, { bool create = false; bool is_changed = false; - virCommandPtr cmd; + virCommandPtr cmd = NULL; char strmac[VIR_MAC_STRING_BUFLEN]; int i; @@ -1808,58 +1809,58 @@ static int parallelsApplyIfaceParams(parallelsDomObjPtr pdom, if (!create && oldnet->type != newnet->type) { virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", _("Changing network type is not supported")); - return -1; + goto error; } if (!STREQ_NULLABLE(oldnet->model, newnet->model)) { virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", _("Changing network device model is not supported")); - return -1; + goto error; } if (!STREQ_NULLABLE(oldnet->data.network.portgroup, newnet->data.network.portgroup)) { virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", _("Changing network portgroup is not supported")); - return -1; + goto error; } if (!virNetDevVPortProfileEqual(oldnet->virtPortProfile, newnet->virtPortProfile)) { virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", _("Changing virtual port profile is not supported")); - return -1; + goto error; } if (newnet->tune.sndbuf_specified) { virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", _("Setting send buffer size is not supported")); - return -1; + goto error; } if (!STREQ_NULLABLE(oldnet->script, newnet->script)) { virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", _("Setting startup script is not supported")); - return -1; + goto error; } if (!STREQ_NULLABLE(oldnet->filter, newnet->filter)) { virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", _("Changing filter params is not supported")); - return -1; + goto error; } if (newnet->bandwidth != NULL) { virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", _("Setting bandwidth params is not supported")); - return -1; + goto error; } for (i = 0; i < sizeof(newnet->vlan); i++) { if (((char *)&newnet->vlan)[i] != 0) { virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", _("Setting vlan params is not supported")); - return -1; + goto error; } } @@ -1901,15 +1902,24 @@ static int parallelsApplyIfaceParams(parallelsDomObjPtr pdom, is_changed = true; } + if (create) + VIR_FREE(oldnet); + if (!create && !is_changed) { /* nothing changed - no need to run prlctl */ + virCommandFree(cmd); return 0; } if (virCommandRun(cmd, NULL)) - return -1; + goto error; return 0; +error: + if (create) + VIR_FREE(oldnet); + virCommandFree(cmd); + return -1; } static int -- 1.7.11.7

2 2

[libvirt] [PATCH] Fix nwfilter driver reload/shutdown handling when unprivileged
by Daniel P. Berrange 23 Jan '13

23 Jan '13

From: "Daniel P. Berrange" <berrange(a)redhat.com> Although the nwfilter driver skips startup when running in a session libvirtd, it did not skip reload or shutdown. This caused errors to be reported when sending SIGHUP to libvirtd, and caused an abort() in libdbus on shutdown due to trying to remove a dbus filter that was never added NB, this was previously part of my large refactoring series https://www.redhat.com/archives/libvir-list/2013-January/msg01411.html but I've pulled it out since I noticed it has SEGV impact. --- src/conf/nwfilter_conf.h | 1 + src/nwfilter/nwfilter_driver.c | 31 +++++++++++++++++++------------ 2 files changed, 20 insertions(+), 12 deletions(-) diff --git a/src/conf/nwfilter_conf.h b/src/conf/nwfilter_conf.h index ca2c070..35f8dde 100644 --- a/src/conf/nwfilter_conf.h +++ b/src/conf/nwfilter_conf.h @@ -556,6 +556,7 @@ typedef struct _virNWFilterDriverState virNWFilterDriverState; typedef virNWFilterDriverState *virNWFilterDriverStatePtr; struct _virNWFilterDriverState { virMutex lock; + bool privileged; virNWFilterObjList nwfilters; diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c index b67b704..19ebb03 100644 --- a/src/nwfilter/nwfilter_driver.c +++ b/src/nwfilter/nwfilter_driver.c @@ -165,7 +165,7 @@ nwfilterDriverInstallDBusMatches(DBusConnection *sysbus ATTRIBUTE_UNUSED) * Initialization function for the QEmu daemon */ static int -nwfilterDriverStartup(bool privileged ATTRIBUTE_UNUSED, +nwfilterDriverStartup(bool privileged, virStateInhibitCallback callback ATTRIBUTE_UNUSED, void *opaque ATTRIBUTE_UNUSED) { @@ -185,6 +185,7 @@ nwfilterDriverStartup(bool privileged ATTRIBUTE_UNUSED, goto err_free_driverstate; driverState->watchingFirewallD = (sysbus != NULL); + driverState->privileged = privileged; if (!privileged) return 0; @@ -275,6 +276,9 @@ nwfilterDriverReload(void) { return -1; } + if (!driverState->privileged) + return 0; + conn = virConnectOpen("qemu:///system"); if (conn) { @@ -333,21 +337,24 @@ nwfilterDriverShutdown(void) { if (!driverState) return -1; - virNWFilterConfLayerShutdown(); - virNWFilterTechDriversShutdown(); - virNWFilterDHCPSnoopShutdown(); - virNWFilterLearnShutdown(); - virNWFilterIPAddrMapShutdown(); + if (driverState->privileged) { + virNWFilterConfLayerShutdown(); + virNWFilterTechDriversShutdown(); + virNWFilterDHCPSnoopShutdown(); + virNWFilterLearnShutdown(); + virNWFilterIPAddrMapShutdown(); - nwfilterDriverLock(driverState); + nwfilterDriverLock(driverState); - nwfilterDriverRemoveDBusMatches(); + nwfilterDriverRemoveDBusMatches(); - /* free inactive nwfilters */ - virNWFilterObjListFree(&driverState->nwfilters); + /* free inactive nwfilters */ + virNWFilterObjListFree(&driverState->nwfilters); + + VIR_FREE(driverState->configDir); + nwfilterDriverUnlock(driverState); + } - VIR_FREE(driverState->configDir); - nwfilterDriverUnlock(driverState); virMutexDestroy(&driverState->lock); VIR_FREE(driverState); -- 1.8.0.2

2 1

[libvirt] [PATCH 0/2] Fix parsing of vCPU counts from domain XMLs.
by Peter Krempa 23 Jan '13

23 Jan '13

These patches fix the parsing and the returned error if parsing of vCPUs failed. If a overflowing number was used for the count, the domain might disappear after the restart of the daemon. Peter Krempa (2): conf: Check if number of vCPUs fits in the storage variable conf: Improve error messages if parsing of vCPU count fails src/conf/domain_conf.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) -- 1.8.1.1

4 8