Detected by Coverity. While it is possible on OOM condition, as well as with bad code that passes binary == NULL, it is unlikely to be encountered in the wild. * src/util/command.c (virCommandNewArgList): Don't leak memory. --- src/util/command.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/util/command.c b/src/util/command.c index 288958e..a2f7ff6 100644 --- a/src/util/command.c +++ b/src/util/command.c @@ -142,7 +142,7 @@ virCommandNewArgList(const char *binary, ...) const char *arg; if (!cmd || cmd->has_error) - return NULL; + return cmd; va_start(list, binary); while ((arg = va_arg(list, const char *)) != NULL)

Spotted by Coverity. Only possible on an OOM condition, so unlikely to bite in the wild. * daemon/libvirtd.c (qemudSetLogging): Don't leak memory. --- daemon/libvirtd.c | 4 +++- 1 files changed, 3 insertions(+), 1 deletions(-) diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index aec81cf..728031f 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -2779,8 +2779,10 @@ qemudSetLogging(struct qemud_server *server, virConfPtr conf, goto free_and_fail; if (virAsprintf(&tmp, "%d:file:%s/.libvirt/libvirtd.log", - virLogGetDefaultPriority(), userdir) == -1) + virLogGetDefaultPriority(), userdir) == -1) { + VIR_FREE(userdir); goto out_of_memory; + } } } else { if (virAsprintf(&tmp, "%d:stderr", virLogGetDefaultPriority()) < 0)

Coverity detected that options was being set by strdup but never freed. But why even bother with an options variable? The options parameter never changes! Leak present since commit 44948f5b (0.7.0). This function could probably be rewritten to take better advantage of virCommand, but that is more invasive. * src/storage/storage_backend_fs.c (virStorageBackendFileSystemMount): Avoid wasted strdup, and guarantee proper cleanup on all paths. --- src/storage/storage_backend_fs.c | 15 +-------------- 1 files changed, 1 insertions(+), 14 deletions(-) diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index 3f4d978..207669a 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -317,7 +317,6 @@ virStorageBackendFileSystemIsMounted(virStoragePoolObjPtr pool) { static int virStorageBackendFileSystemMount(virStoragePoolObjPtr pool) { char *src; - char *options = NULL; const char **mntargv; /* 'mount -t auto' doesn't seem to auto determine nfs (or cifs), @@ -328,7 +327,6 @@ virStorageBackendFileSystemMount(virStoragePoolObjPtr pool) { int glusterfs = (pool->def->type == VIR_STORAGE_POOL_NETFS && pool->def->source.format == VIR_STORAGE_POOL_NETFS_GLUSTERFS); - int option_index; int source_index; const char *netfs_auto_argv[] = { @@ -358,7 +356,7 @@ virStorageBackendFileSystemMount(virStoragePoolObjPtr pool) { virStoragePoolFormatFileSystemNetTypeToString(pool->def->source.format), NULL, "-o", - NULL, + "direct-io-mode=1", pool->def->target.path, NULL, }; @@ -369,7 +367,6 @@ virStorageBackendFileSystemMount(virStoragePoolObjPtr pool) { } else if (glusterfs) { mntargv = glusterfs_argv; source_index = 3; - option_index = 5; } else { mntargv = fs_argv; source_index = 3; @@ -405,12 +402,6 @@ virStorageBackendFileSystemMount(virStoragePoolObjPtr pool) { } if (pool->def->type == VIR_STORAGE_POOL_NETFS) { - if (pool->def->source.format == VIR_STORAGE_POOL_NETFS_GLUSTERFS) { - if ((options = strdup("direct-io-mode=1")) == NULL) { - virReportOOMError(); - return -1; - } - } if (virAsprintf(&src, "%s:%s", pool->def->source.host.name, pool->def->source.dir) == -1) { @@ -426,10 +417,6 @@ virStorageBackendFileSystemMount(virStoragePoolObjPtr pool) { } mntargv[source_index] = src; - if (glusterfs) { - mntargv[option_index] = options; - } - if (virRun(mntargv, NULL) < 0) { VIR_FREE(src); return -1;

Coverity couldn't see that priv is clean on failure. But on failure, we might as well guarantee that callers don't try to free uninitialized memory. * src/remote/remote_driver.c (remoteGenericOpen): Even on failure, pass priv back to caller. --- In reading my own mail, I decided that patch 10 can be made simpler. v2: rewrite to avoid sa_assert and be more robust. src/remote/remote_driver.c | 6 ++---- 1 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 14c3d24..8335a1a 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -2545,10 +2545,8 @@ remoteGenericOpen(virConnectPtr conn, virConnectAuthPtr auth, * use the UNIX transport. This handles Xen driver * which doesn't have its own impl of the network APIs. */ struct private_data *priv; - int ret; - ret = remoteOpenSecondaryDriver(conn, auth, flags, &priv); - if (ret == VIR_DRV_OPEN_SUCCESS) - *genericPrivateData = priv; + int ret = remoteOpenSecondaryDriver(conn, auth, flags, &priv); + *genericPrivateData = priv; return ret; } }

Detected by Coverity. Bug introduced in 08106e2044 (unreleased). * src/conf/domain_conf.c (virDomainChannelDefCheckABIStability): Use correct sizeof operand. --- src/conf/domain_conf.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 9975bca..0d9fef4 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -7269,7 +7269,8 @@ static bool virDomainChannelDefCheckABIStability(virDomainChrDefPtr src, } break; case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_GUESTFWD: - if (memcmp(src->target.addr, dst->target.addr, sizeof(src->target.addr)) != 0) { + if (memcmp(src->target.addr, dst->target.addr, + sizeof(*src->target.addr)) != 0) { char *saddr = virSocketFormatAddrFull(src->target.addr, true, ":"); char *daddr = virSocketFormatAddrFull(dst->target.addr, true, ":"); virDomainReportError(VIR_ERR_CONFIG_UNSUPPORTED,