6:34 a.m.
Installed a new Coverity checker (7.6.1) and naturally new issues
were found, plus a set of false positives which for now I'll just
have to ignore since it seems they may be a bug in Coverity, but
that's still to be determined
John Ferlan (8):
libxl: Resolve Coverity RESOURCE_LEAK
vbox: Resolve Coverity RESOURCE_LEAK
qemu: Resolve Coverity IDENTICAL_BRANCHES
qemu: Resolve Coverity FORWARD_NULL
xen: Resolve Coverity FORWARD_NULL
qemu: Resolve Coverity FORWARD_NULL
qemu: Resolve Coverity FORWARD_NULL
qemu: Resolve Coverity FORWARD_NULL
src/libxl/libxl_migration.c | 1 +
src/qemu/qemu_capabilities.c | 4 +++-
src/qemu/qemu_command.c | 4 +++-
src/qemu/qemu_driver.c | 5 +++--
src/qemu/qemu_hotplug.c | 11 +++--------
src/vbox/vbox_common.c | 21 +++++++--------------
src/xen/xm_internal.c | 3 +++
7 files changed, 23 insertions(+), 26 deletions(-)
--
2.1.0
6:34 a.m.
New subject: [libvirt] [PATCH 1/8] libxl: Resolve Coverity RESOURCE_LEAK
The returned socks from virNetSocketNewListenTCP needs to be VIR_FREE'd
as well as seach of the Close/Unref on all the socks[i] that is
already done
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/libxl/libxl_migration.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c
index 1efd98f..10b5bd6 100644
--- a/src/libxl/libxl_migration.c
+++ b/src/libxl/libxl_migration.c
@@ -445,6 +445,7 @@ libxlDomainMigrationPrepare(virConnectPtr dconn,
virNetSocketClose(socks[i]);
virObjectUnref(socks[i]);
}
+ VIR_FREE(socks);
/* Remove virDomainObj from domain list */
if (vm) {
virDomainObjListRemove(driver->domains, vm);
--
2.1.0
6:34 a.m.
New subject: [libvirt] [PATCH 2/8] vbox: Resolve Coverity RESOURCE_LEAK
If the virStringSearch() returns a 0 (zero), then each of the uses
of the call will just jump to cleanup forgetting to free the returned
empty list. Expand the scope a bit of each use and free at cleanup.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/vbox/vbox_common.c | 21 +++++++--------------
1 file changed, 7 insertions(+), 14 deletions(-)
diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c
index 0bb5d29..e3a1739 100644
--- a/src/vbox/vbox_common.c
+++ b/src/vbox/vbox_common.c
@@ -4463,6 +4463,8 @@ vboxSnapshotRedefine(virDomainPtr dom,
int realReadOnlyDisksPathSize = 0;
virVBoxSnapshotConfSnapshotPtr newSnapshotPtr = NULL;
unsigned char snapshotUuid[VIR_UUID_BUFLEN];
+ char **searchResultTab = NULL;
+ ssize_t resultSize = 0;
int it = 0;
int jt = 0;
PRUint32 aMediaSize = 0;
@@ -4862,8 +4864,6 @@ vboxSnapshotRedefine(virDomainPtr dom,
for (it = 0; it < def->dom->ndisks; it++) {
char *location = NULL;
const char *uuidReplacing = NULL;
- char **searchResultTab = NULL;
- ssize_t resultSize = 0;
char *tmp = NULL;
location = def->dom->disks[it]->src->path;
@@ -4885,6 +4885,7 @@ vboxSnapshotRedefine(virDomainPtr dom,
searchResultTab[it],
uuidReplacing);
virStringFreeList(searchResultTab);
+ searchResultTab = NULL;
VIR_FREE(newSnapshotPtr->storageController);
if (!tmp)
goto cleanup;
@@ -5029,8 +5030,6 @@ vboxSnapshotRedefine(virDomainPtr dom,
if (needToChangeStorageController) {
/*We need to append this disk in the storage controller*/
- char **searchResultTab = NULL;
- ssize_t resultSize = 0;
char *tmp = NULL;
resultSize = virStringSearch(snapshotMachineDesc->storageController,
VBOX_UUID_REGEX,
@@ -5045,7 +5044,6 @@ vboxSnapshotRedefine(virDomainPtr dom,
tmp = virStringReplace(snapshotMachineDesc->storageController,
searchResultTab[it],
disk->uuid);
- virStringFreeList(searchResultTab);
VIR_FREE(snapshotMachineDesc->storageController);
if (!tmp)
goto cleanup;
@@ -5077,8 +5075,6 @@ vboxSnapshotRedefine(virDomainPtr dom,
virVBoxSnapshotConfHardDiskPtr disk = NULL;
char *uuid = NULL;
char *format = NULL;
- char **searchResultTab = NULL;
- ssize_t resultSize = 0;
char *tmp = NULL;
vboxIIDUnion iid, parentiid;
@@ -5189,7 +5185,6 @@ vboxSnapshotRedefine(virDomainPtr dom,
tmp = virStringReplace(snapshotMachineDesc->storageController,
searchResultTab[it],
disk->uuid);
- virStringFreeList(searchResultTab);
VIR_FREE(snapshotMachineDesc->storageController);
if (!tmp)
goto cleanup;
@@ -5299,6 +5294,7 @@ vboxSnapshotRedefine(virDomainPtr dom,
VBOX_UTF8_FREE(machineName);
virStringFreeList(realReadOnlyDisksPath);
virStringFreeList(realReadWriteDisksPath);
+ virStringFreeList(searchResultTab);
VIR_FREE(newSnapshotPtr);
VIR_FREE(machineLocationPath);
VIR_FREE(nameTmpUse);
@@ -6730,6 +6726,8 @@ vboxDomainSnapshotDeleteMetadataOnly(virDomainSnapshotPtr snapshot)
char *settingsFilepath = NULL;
virVBoxSnapshotConfMachinePtr snapshotMachineDesc = NULL;
int isCurrent = -1;
+ char **searchResultTab = NULL;
+ ssize_t resultSize = 0;
int it = 0;
PRUnichar *machineNameUtf16 = NULL;
char *machineName = NULL;
@@ -6822,8 +6820,6 @@ vboxDomainSnapshotDeleteMetadataOnly(virDomainSnapshotPtr snapshot)
virVBoxSnapshotConfHardDiskPtr disk = NULL;
char *uuid = NULL;
char *format = NULL;
- char **searchResultTab = NULL;
- ssize_t resultSize = 0;
char *tmp = NULL;
vboxIIDUnion iid, parentiid;
resultCodeUnion resultCode;
@@ -6950,7 +6946,6 @@ vboxDomainSnapshotDeleteMetadataOnly(virDomainSnapshotPtr snapshot)
tmp = virStringReplace(snapshotMachineDesc->storageController,
searchResultTab[it],
disk->uuid);
- virStringFreeList(searchResultTab);
VIR_FREE(snapshotMachineDesc->storageController);
if (!tmp)
goto cleanup;
@@ -6970,8 +6965,6 @@ vboxDomainSnapshotDeleteMetadataOnly(virDomainSnapshotPtr snapshot)
} else {
for (it = 0; it < def->dom->ndisks; it++) {
const char *uuidRO = NULL;
- char **searchResultTab = NULL;
- ssize_t resultSize = 0;
char *tmp = NULL;
uuidRO = virVBoxSnapshotConfHardDiskUuidByLocation(snapshotMachineDesc,
def->dom->disks[it]->src->path);
@@ -6996,7 +6989,6 @@ vboxDomainSnapshotDeleteMetadataOnly(virDomainSnapshotPtr snapshot)
tmp = virStringReplace(snapshotMachineDesc->storageController,
searchResultTab[it],
uuidRO);
- virStringFreeList(searchResultTab);
VIR_FREE(snapshotMachineDesc->storageController);
if (!tmp)
goto cleanup;
@@ -7147,6 +7139,7 @@ vboxDomainSnapshotDeleteMetadataOnly(virDomainSnapshotPtr snapshot)
VBOX_RELEASE(machine);
VBOX_UTF16_FREE(settingsFilePathUtf16);
VBOX_UTF8_FREE(settingsFilepath);
+ virStringFreeList(searchResultTab);
VIR_FREE(snapshotMachineDesc);
VBOX_UTF16_FREE(machineNameUtf16);
VBOX_UTF8_FREE(machineName);
--
2.1.0
6:34 a.m.
New subject: [libvirt] [PATCH 3/8] qemu: Resolve Coverity IDENTICAL_BRANCHES
Coverity complains that in the error paths both the < 0 condition and
the success path after the qemuDomainObjExitMonitor failure will end
up going to cleanup. So just use ignore_value in this error path to
resolve the complaint.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_hotplug.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 613b728..4e9603e 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -3640,15 +3640,13 @@ int qemuDomainDetachControllerDevice(virQEMUDriverPtr driver,
qemuDomainObjEnterMonitor(driver, vm);
if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) {
if (qemuMonitorDelDevice(priv->mon, detach->info.alias)) {
- if (qemuDomainObjExitMonitor(driver, vm) < 0)
- goto cleanup;
+ ignore_value(qemuDomainObjExitMonitor(driver, vm));
goto cleanup;
}
} else {
if (qemuMonitorRemovePCIDevice(priv->mon,
&detach->info.addr.pci) < 0) {
- if (qemuDomainObjExitMonitor(driver, vm) < 0)
- goto cleanup;
+ ignore_value(qemuDomainObjExitMonitor(driver, vm));
goto cleanup;
}
}
@@ -4136,8 +4134,7 @@ int qemuDomainDetachChrDevice(virQEMUDriverPtr driver,
qemuDomainObjEnterMonitor(driver, vm);
if (devstr && qemuMonitorDelDevice(priv->mon, tmpChr->info.alias) <
0) {
- if (qemuDomainObjExitMonitor(driver, vm) < 0)
- goto cleanup;
+ ignore_value(qemuDomainObjExitMonitor(driver, vm));
goto cleanup;
}
if (qemuDomainObjExitMonitor(driver, vm) < 0)
--
2.1.0
6:34 a.m.
New subject: [libvirt] [PATCH 4/8] qemu: Resolve Coverity FORWARD_NULL
Coverity notes that ->ifname is used after the VIR_FREE done in the
code path after the call to virNetDevMacVLanDeleteWithVPortProfile
by a call to virNetDevOpenvswitchRemovePort.
Since the ->ifname will be VIR_FREE()'d eventually in virDomainNetDefFree
just remove the extraneous VIR_FREE here.
When originally added, the Openvswitch code wasn't present and checks
were made for non NULL prior to use.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_hotplug.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 4e9603e..095ed88 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1147,7 +1147,6 @@ int qemuDomainAttachNetDevice(virConnectPtr conn,
virDomainNetGetActualDirectMode(net),
virDomainNetGetActualVirtPortProfile(net),
cfg->stateDir));
- VIR_FREE(net->ifname);
}
vport = virDomainNetGetActualVirtPortProfile(net);
@@ -3107,7 +3106,6 @@ qemuDomainRemoveNetDevice(virQEMUDriverPtr driver,
virDomainNetGetActualDirectMode(net),
virDomainNetGetActualVirtPortProfile(net),
cfg->stateDir));
- VIR_FREE(net->ifname);
}
vport = virDomainNetGetActualVirtPortProfile(net);
--
2.1.0
6:34 a.m.
New subject: [libvirt] [PATCH 5/8] xen: Resolve Coverity FORWARD_NULL
Coverity found that xenXMConfigCacheAddFile has an error path in which
no error message and a -1 was not returned which could have resulted in
a NULL dereference in a VIR_DEBUG statement and of course an erroneous
0 value returned!
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/xen/xm_internal.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/src/xen/xm_internal.c b/src/xen/xm_internal.c
index 64752df..59b1cd4 100644
--- a/src/xen/xm_internal.c
+++ b/src/xen/xm_internal.c
@@ -279,6 +279,9 @@ xenXMConfigCacheAddFile(virConnectPtr conn, const char *filename)
virDomainDefFree(entry->def);
VIR_FREE(entry->filename);
VIR_FREE(entry);
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("xenXMConfigCacheRefresh: virHashAddEntry
name"));
+ return -1;
}
}
VIR_DEBUG("Added config %s %s", entry->def->name, filename);
--
2.1.0
6:34 a.m.
New subject: [libvirt] [PATCH 6/8] qemu: Resolve Coverity FORWARD_NULL
Coverity points out it was possible to have a zero return from
qemuBuildRNGBackendProps thus not filling in 'props' and then
causing a NULL dereference on the next call.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_command.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index c32d8c6..68844fa 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -6464,7 +6464,9 @@ qemuBuildRNGBackendProps(virDomainRNGDefPtr rng,
break;
case VIR_DOMAIN_RNG_BACKEND_LAST:
- break;
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("unknown rng-random backend"));
+ goto cleanup;
}
ret = 0;
--
2.1.0
6:34 a.m.
New subject: [libvirt] [PATCH 7/8] qemu: Resolve Coverity FORWARD_NULL
Coverity complains over the [n]values pairing in virQEMUCapsFreeStringList
and rather than make a bunch if "if values" checks prior to calling, by
just adding the values check inside the free function we avoid the chance
that somehow nvalues is > 0, while values == NULL
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_capabilities.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index d50863f..25c15bf 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -1702,6 +1702,8 @@ virQEMUCapsFreeStringList(size_t len,
char **values)
{
size_t i;
+ if (!values)
+ return;
for (i = 0; i < len; i++)
VIR_FREE(values[i]);
VIR_FREE(values);
@@ -1794,7 +1796,7 @@ virQEMUCapsParseDeviceStrObjectProps(const char *str,
ret = nproplist;
cleanup:
- if (ret < 0 && proplist)
+ if (ret < 0)
virQEMUCapsFreeStringList(nproplist, proplist);
return ret;
}
--
2.1.0
6:34 a.m.
New subject: [libvirt] [PATCH 8/8] qemu: Resolve Coverity FORWARD_NULL
Coverity points out that qemuMonitorGetAllBlockStatsInfo could return a
-1 and thus not fill in 'stats' (leaving it NULL). Then the call to
qemuMonitorBlockStatsUpdateCapacity will dereference it.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_driver.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index a76858a..d173aa1 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -19633,8 +19633,9 @@ qemuDomainGetStatsBlock(virQEMUDriverPtr driver,
qemuDomainObjEnterMonitor(driver, dom);
rc = qemuMonitorGetAllBlockStatsInfo(priv->mon, &stats,
visitBacking);
- ignore_value(qemuMonitorBlockStatsUpdateCapacity(priv->mon, stats,
- visitBacking));
+ if (rc >= 0)
+ ignore_value(qemuMonitorBlockStatsUpdateCapacity(priv->mon, stats,
+ visitBacking));
if (qemuDomainObjExitMonitor(driver, dom) < 0)
goto cleanup;
--
2.1.0
8:02 a.m.
On 05.05.2015 13:34, John Ferlan wrote:
Installed a new Coverity checker (7.6.1) and naturally new issues
were found, plus a set of false positives which for now I'll just
have to ignore since it seems they may be a bug in Coverity, but
that's still to be determined
John Ferlan (8):
libxl: Resolve Coverity RESOURCE_LEAK
vbox: Resolve Coverity RESOURCE_LEAK
qemu: Resolve Coverity IDENTICAL_BRANCHES
qemu: Resolve Coverity FORWARD_NULL
xen: Resolve Coverity FORWARD_NULL
qemu: Resolve Coverity FORWARD_NULL
qemu: Resolve Coverity FORWARD_NULL
qemu: Resolve Coverity FORWARD_NULL
src/libxl/libxl_migration.c | 1 +
src/qemu/qemu_capabilities.c | 4 +++-
src/qemu/qemu_command.c | 4 +++-
src/qemu/qemu_driver.c | 5 +++--
src/qemu/qemu_hotplug.c | 11 +++--------
src/vbox/vbox_common.c | 21 +++++++--------------
src/xen/xm_internal.c | 3 +++
7 files changed, 23 insertions(+), 26 deletions(-)
ACK series.
Michal
3519
days inactive
3519
days old
9 comments
2 participants
participants (2)
-
John Ferlan -
Michal Privoznik