12:36 p.m.
From: "Eduardo Lima (Etrunko)" <eblima(a)br.ibm.com>
Yet another series of patches fixing errors revealed by Coverity tool. It would
be nice if we had a similar setup so we could run the tool on a regular basis. I
started playing with clang a while ago. It also provides a static analyser, but
I could not complete the setup by then.
https://bugzilla.redhat.com/show_bug.cgi?id=750418
Best regards, Etrunko
Eduardo Lima (Etrunko) (4):
libxkutil: Fix possible NULL dereferences
Fix possible memory leaks
xml_parse_test: Fix invalid dereference
Fix possible use of unitialized variables
libxkutil/cs_util_instance.c | 5 +++++
libxkutil/device_parsing.c | 14 +++++++-------
libxkutil/pool_parsing.c | 5 +++--
libxkutil/xml_parse_test.c | 3 +--
libxkutil/xmlgen.c | 8 +++++---
src/Virt_AppliedFilterList.c | 3 ++-
src/Virt_Device.c | 28 ++++++++++++++++++++++++++++
src/Virt_DevicePool.c | 19 ++++++++++++++++++-
src/Virt_SwitchService.c | 24 ++++++++++++++++++++----
src/Virt_VirtualSystemManagementService.c | 16 ++++++++--------
src/Virt_VirtualSystemSnapshotService.c | 2 +-
11 files changed, 98 insertions(+), 29 deletions(-)
--
1.7.7.5
12:36 p.m.
New subject: [PATCH 1/4] libxkutil: Fix possible NULL dereferences
From: "Eduardo Lima (Etrunko)" <eblima(a)br.ibm.com>
As revealed by recent Coverity scan report provided by Red Hat:
https://bugzilla.redhat.com/show_bug.cgi?id=750418
https://bugzilla.redhat.com/attachment.cgi?id=552325
Error: FORWARD_NULL:
xmlgen.c:100: var_compare_op: Comparing "dev->device" to null implies that
"dev->device" might be null.
xmlgen.c:115: var_deref_model: Passing null variable "(char *)dev->device" to
function "__coverity_strcmp", which dereferences it.
Error: FORWARD_NULL:
device_parsing.c:615: var_compare_op: Comparing "gdev->type" to null implies
that "gdev->type" might be null.
device_parsing.c:677: var_deref_model: Passing null variable "gdev->type" to
function "cleanup_graphics_device", which dereferences it.
device_parsing.c:126: deref_parm_in_call: Function "strcasecmp" dereferences
parameter "dev->type". (The dereference is assumed on the basis of the
'nonnull' parameter attribute.)
Error: NULL_RETURNS:
Virt_DevicePool.c:805: returned_null: Function "get_typed_instance" returns null
(checked 37 out of 44 times).
misc_util.c:348: null_assign: Assigning: "inst" = NULL.
misc_util.c:369: return_null_var: Returning "inst", which is null.
Virt_DevicePool.c:805: var_assigned: Assigning: "inst" = null return value from
"get_typed_instance".
Virt_DevicePool.c:810: dereference: Dereferencing a pointer that might be null
"inst" when calling "mempool_set_total".
Virt_DevicePool.c:686: deref_parm: Directly dereferencing parameter "inst".
Error: NULL_RETURNS:
Virt_DevicePool.c:837: returned_null: Function "get_typed_instance" returns null
(checked 37 out of 44 times).
misc_util.c:348: null_assign: Assigning: "inst" = NULL.
misc_util.c:369: return_null_var: Returning "inst", which is null.
Virt_DevicePool.c:837: var_assigned: Assigning: "inst" = null return value from
"get_typed_instance".
Virt_DevicePool.c:842: dereference: Dereferencing a pointer that might be null
"inst" when calling "procpool_set_total".
Virt_DevicePool.c:743: deref_parm: Directly dereferencing parameter "inst".
Error: NULL_RETURNS:
Virt_Device.c:219: returned_null: Function "get_typed_instance" returns null
(checked 37 out of 44 times).
misc_util.c:348: null_assign: Assigning: "inst" = NULL.
misc_util.c:369: return_null_var: Returning "inst", which is null.
Virt_Device.c:219: var_assigned: Assigning: "inst" = null return value from
"get_typed_instance".
Virt_Device.c:224: dereference: Dereferencing a pointer that might be null
"inst" when calling "graphics_set_attr".
Virt_Device.c:202: deref_parm: Directly dereferencing parameter "instance".
Error: NULL_RETURNS:
Virt_Device.c:133: returned_null: Function "get_typed_instance" returns null
(checked 37 out of 44 times).
misc_util.c:348: null_assign: Assigning: "inst" = NULL.
misc_util.c:369: return_null_var: Returning "inst", which is null.
Virt_Device.c:133: var_assigned: Assigning: "inst" = null return value from
"get_typed_instance".
Virt_Device.c:138: dereference: Dereferencing a pointer that might be null
"inst" when calling "disk_set_name".
Virt_Device.c:117: deref_parm: Directly dereferencing parameter "instance".
Error: NULL_RETURNS:
Virt_Device.c:175: returned_null: Function "get_typed_instance" returns null
(checked 37 out of 44 times).
misc_util.c:348: null_assign: Assigning: "inst" = NULL.
misc_util.c:369: return_null_var: Returning "inst", which is null.
Virt_Device.c:175: var_assigned: Assigning: "inst" = null return value from
"get_typed_instance".
Virt_Device.c:180: dereference: Dereferencing a pointer that might be null
"inst" when calling "mem_set_size".
Virt_Device.c:156: deref_parm: Directly dereferencing parameter "instance".
Error: NULL_RETURNS:
Virt_Device.c:100: returned_null: Function "get_typed_instance" returns null
(checked 37 out of 44 times).
misc_util.c:348: null_assign: Assigning: "inst" = NULL.
misc_util.c:369: return_null_var: Returning "inst", which is null.
Virt_Device.c:100: var_assigned: Assigning: "inst" = null return value from
"get_typed_instance".
Virt_Device.c:105: dereference: Dereferencing a pointer that might be null
"inst" when calling "net_set_type".
Virt_Device.c:61: deref_parm: Directly dereferencing parameter "instance".
Signed-off-byr Eduardo Lima (Etrunko) <eblima(a)br.ibm.com>
---
libxkutil/device_parsing.c | 13 ++++++-------
libxkutil/xmlgen.c | 4 ++--
src/Virt_Device.c | 28 ++++++++++++++++++++++++++++
src/Virt_DevicePool.c | 14 ++++++++++++++
4 files changed, 50 insertions(+), 9 deletions(-)
diff --git a/libxkutil/device_parsing.c b/libxkutil/device_parsing.c
index 7eaa63e..b0eccfc 100644
--- a/libxkutil/device_parsing.c
+++ b/libxkutil/device_parsing.c
@@ -120,15 +120,14 @@ static void cleanup_sdl_device(struct graphics_device *dev)
static void cleanup_graphics_device(struct graphics_device *dev)
{
- if (dev == NULL)
+ if (dev == NULL || dev->type == NULL)
return;
- if (STREQC(dev->type, "sdl")) {
- cleanup_sdl_device(dev);
- }
- else {
- cleanup_vnc_device(dev);
- }
+ if (STREQC(dev->type, "sdl"))
+ cleanup_sdl_device(dev);
+ else
+ cleanup_vnc_device(dev);
+
free(dev->type);
}
diff --git a/libxkutil/xmlgen.c b/libxkutil/xmlgen.c
index d73ffd0..96b4e96 100644
--- a/libxkutil/xmlgen.c
+++ b/libxkutil/xmlgen.c
@@ -112,8 +112,8 @@ static const char *disk_file_xml(xmlNodePtr root, struct disk_device
*dev)
xmlNewProp(tmp, BAD_CAST "cache", BAD_CAST
dev->cache);
}
- if ((XSTREQ(dev->device, "cdrom")) &&
- (XSTREQ(dev->source, ""))) {
+ if (dev->device != NULL && XSTREQ(dev->device, "cdrom")
&&
+ XSTREQ(dev->source, "")) {
/* This is the situation that user defined a cdrom device without
disk in it, so skip generating a line saying "source", for
that
xml defination for libvirt should not have this defined in this
diff --git a/src/Virt_Device.c b/src/Virt_Device.c
index fd11370..96797a4 100644
--- a/src/Virt_Device.c
+++ b/src/Virt_Device.c
@@ -102,6 +102,13 @@ static CMPIInstance *net_instance(const CMPIBroker *broker,
"NetworkPort",
ns);
+ if (inst == NULL) {
+ cu_statusf(broker, &s,
+ CMPI_RC_ERR_FAILED,
+ "Failed to get instance for NetworkPort");
+ return NULL;
+ }
+
if (!net_set_type(inst, dev))
return NULL;
@@ -135,6 +142,13 @@ static CMPIInstance *disk_instance(const CMPIBroker *broker,
"LogicalDisk",
ns);
+ if (inst == NULL) {
+ cu_statusf(broker, &s,
+ CMPI_RC_ERR_FAILED,
+ "Failed to get instance for LogicalDisk");
+ return NULL;
+ }
+
if (!disk_set_name(inst, dev))
return NULL;
@@ -177,6 +191,13 @@ static CMPIInstance *mem_instance(const CMPIBroker *broker,
"Memory",
ns);
+ if (inst == NULL) {
+ cu_statusf(broker, &s,
+ CMPI_RC_ERR_FAILED,
+ "Failed to get instance for Memory");
+ return NULL;
+ }
+
if (!mem_set_size(inst, dev))
return NULL;
@@ -221,6 +242,13 @@ static CMPIInstance *graphics_instance(const CMPIBroker *broker,
"DisplayController",
ns);
+ if (inst == NULL) {
+ cu_statusf(broker, &s,
+ CMPI_RC_ERR_FAILED,
+ "Failed to get instance for DisplayController");
+ return NULL;
+ }
+
if (!graphics_set_attr(inst, dev))
return NULL;
diff --git a/src/Virt_DevicePool.c b/src/Virt_DevicePool.c
index a41a378..ab0baa0 100644
--- a/src/Virt_DevicePool.c
+++ b/src/Virt_DevicePool.c
@@ -807,6 +807,13 @@ static CMPIStatus mempool_instance(virConnectPtr conn,
"MemoryPool",
ns);
+ if (inst == NULL) {
+ cu_statusf(broker, &s,
+ CMPI_RC_ERR_FAILED,
+ "Failed to get instance for MemoryPool");
+ return s;
+ }
+
mempool_set_total(inst, conn);
mempool_set_consumed(inst, conn);
@@ -839,6 +846,13 @@ static CMPIStatus procpool_instance(virConnectPtr conn,
"ProcessorPool",
ns);
+ if (inst == NULL) {
+ cu_statusf(broker, &s,
+ CMPI_RC_ERR_FAILED,
+ "Failed to get instance for ProcessorPool");
+ return s;
+ }
+
procpool_set_total(inst, conn);
set_params(inst, CIM_RES_TYPE_PROC, id, "Processors", NULL, true);
--
1.7.7.5
12:46 p.m.
New subject: [PATCH 1/4] libxkutil: Fix possible NULL dereferences
On 01/13/2012 04:36 PM, Eduardo Lima (Etrunko) wrote:
[snip]
diff --git a/src/Virt_Device.c b/src/Virt_Device.c
index fd11370..96797a4 100644
--- a/src/Virt_Device.c
+++ b/src/Virt_Device.c
@@ -102,6 +102,13 @@ static CMPIInstance *net_instance(const CMPIBroker *broker,
"NetworkPort",
ns);
+ if (inst == NULL) {
+ cu_statusf(broker, &s,
+ CMPI_RC_ERR_FAILED,
+ "Failed to get instance for NetworkPort");
+ return NULL;
+ }
+
if (!net_set_type(inst, dev))
return NULL;
@@ -135,6 +142,13 @@ static CMPIInstance *disk_instance(const CMPIBroker *broker,
"LogicalDisk",
ns);
+ if (inst == NULL) {
+ cu_statusf(broker, &s,
+ CMPI_RC_ERR_FAILED,
+ "Failed to get instance for LogicalDisk");
+ return NULL;
+ }
+
if (!disk_set_name(inst, dev))
return NULL;
@@ -177,6 +191,13 @@ static CMPIInstance *mem_instance(const CMPIBroker *broker,
"Memory",
ns);
+ if (inst == NULL) {
+ cu_statusf(broker, &s,
+ CMPI_RC_ERR_FAILED,
+ "Failed to get instance for Memory");
+ return NULL;
+ }
+
if (!mem_set_size(inst, dev))
return NULL;
@@ -221,6 +242,13 @@ static CMPIInstance *graphics_instance(const CMPIBroker *broker,
"DisplayController",
ns);
+ if (inst == NULL) {
+ cu_statusf(broker, &s,
+ CMPI_RC_ERR_FAILED,
+ "Failed to get instance for DisplayController");
+ return NULL;
+ }
+
if (!graphics_set_attr(inst, dev))
return NULL;
Woops, this patch does not compiles. Sorry about that. I'm sending a new
version of the series.
Cheers,
--
Eduardo de Barros Lima
Software Engineer, Open Virtualization
Linux Technology Center - IBM/Brazil
eblima(a)br.ibm.com
12:36 p.m.
New subject: [PATCH 2/4] Fix possible memory leaks
From: "Eduardo Lima (Etrunko)" <eblima(a)br.ibm.com>
As revealed by recent Coverity scan report provided by Red Hat:
https://bugzilla.redhat.com/show_bug.cgi?id=750418
https://bugzilla.redhat.com/attachment.cgi?id=552325
Error: RESOURCE_LEAK:
pool_parsing.c:140: alloc_fn: Calling allocation function "realloc".
pool_parsing.c:140: var_assign: Assigning: "tmp" = storage returned from
"realloc(dev_paths, 8UL * (ct + 1U))".
pool_parsing.c:145: var_assign: Assigning: "dev_paths" = "tmp".
pool_parsing.c:149: leaked_storage: Variable "tmp" going out of scope leaks the
storage it points to.
pool_parsing.c:169: leaked_storage: Variable "dev_paths" going out of scope
leaks the storage it points to.
Error: RESOURCE_LEAK:
xmlgen.c:891: alloc_fn: Calling allocation function "virt_device_dup".
device_parsing.c:873: alloc_fn: Storage is returned from allocation function
"calloc".
device_parsing.c:873: var_assign: Assigning: "dev" = "calloc(1UL,
152UL)".
device_parsing.c:928: return_alloc: Returning allocated memory "dev".
xmlgen.c:891: var_assign: Assigning: "dev" = storage returned from
"virt_device_dup(_dev)".
xmlgen.c:952: leaked_storage: Variable "dev" going out of scope leaks the
storage it points to.
Error: RESOURCE_LEAK:
device_parsing.c:354: alloc_fn: Calling allocation function "calloc".
device_parsing.c:354: var_assign: Assigning: "vsi_dev" = storage returned from
"calloc(1UL, 56UL)".
device_parsing.c:385: noescape: Variable "vsi_dev" is not freed or pointed-to in
function "memcpy".
device_parsing.c:386: leaked_storage: Variable "vsi_dev" going out of scope
leaks the storage it points to.
Error: RESOURCE_LEAK:
Virt_DevicePool.c:1077: alloc_arg: Calling allocation function
"get_diskpool_config" on "pools".
Virt_DevicePool.c:161: alloc_arg: "get_disk_parent" allocates memory that is
stored into "pools".
Virt_DevicePool.c:74: alloc_fn: Storage is returned from allocation function
"realloc".
Virt_DevicePool.c:74: var_assign: Assigning: "pools" = "realloc(pools,
(count + 1) * 24UL)".
Virt_DevicePool.c:86: var_assign: Assigning: "*_pools" = "pools".
Virt_DevicePool.c:163: var_assign: Assigning: "*_pools" = "pools".
Virt_DevicePool.c:1080: leaked_storage: Variable "pools" going out of scope
leaks the storage it points to.
Error: RESOURCE_LEAK:
Virt_DevicePool.c:404: alloc_arg: Calling allocation function
"get_diskpool_config" on "pools".
Virt_DevicePool.c:161: alloc_arg: "get_disk_parent" allocates memory that is
stored into "pools".
Virt_DevicePool.c:74: alloc_fn: Storage is returned from allocation function
"realloc".
Virt_DevicePool.c:74: var_assign: Assigning: "pools" = "realloc(pools,
(count + 1) * 24UL)".
Virt_DevicePool.c:86: var_assign: Assigning: "*_pools" = "pools".
Virt_DevicePool.c:163: var_assign: Assigning: "*_pools" = "pools".
Virt_DevicePool.c:406: leaked_storage: Variable "pools" going out of scope leaks
the storage it points to.
Error: RESOURCE_LEAK:
Virt_VirtualSystemManagementService.c:415: alloc_fn: Calling allocation function
"realloc".
Virt_VirtualSystemManagementService.c:415: var_assign: Assigning: "tmp_str_arr"
= storage returned from "realloc(domain->os_info.fv.bootlist, bl_size *
8UL)".
Virt_VirtualSystemManagementService.c:432: leaked_storage: Variable
"tmp_str_arr" going out of scope leaks the storage it points to.
Virt_VirtualSystemManagementService.c:440: leaked_storage: Variable
"tmp_str_arr" going out of scope leaks the storage it points to.
Error: RESOURCE_LEAK:
Virt_VirtualSystemManagementService.c:2834: alloc_fn: Calling allocation function
"malloc".
Virt_VirtualSystemManagementService.c:2834: var_assign: Assigning: "__retval" =
storage returned from "malloc(__len)".
Virt_VirtualSystemManagementService.c:2834: noescape: Variable "__retval" is not
freed or pointed-to in function "memcpy".
Virt_VirtualSystemManagementService.c:2834: overwrite_var: Overwriting
"__retval" in call "__retval = (char *)memcpy(__retval,
"ResourceAllocationSettingDataCreatedIndication", __len)" leaks the storage
that "__retval" points to.
Virt_VirtualSystemManagementService.c:2834: var_assign: Assigning: "__retval" =
storage returned from "memcpy(__retval,
"ResourceAllocationSettingDataCreatedIndication", __len)".
Virt_VirtualSystemManagementService.c:2834: var_assign: Assigning: "indication"
= "__retval".
Virt_VirtualSystemManagementService.c:2901: leaked_storage: Variable
"indication" going out of scope leaks the storage it points to.
Error: RESOURCE_LEAK:
Virt_VirtualSystemManagementService.c:2837: alloc_fn: Calling allocation function
"malloc".
Virt_VirtualSystemManagementService.c:2837: var_assign: Assigning: "__retval" =
storage returned from "malloc(__len)".
Virt_VirtualSystemManagementService.c:2837: noescape: Variable "__retval" is not
freed or pointed-to in function "memcpy".
Virt_VirtualSystemManagementService.c:2837: overwrite_var: Overwriting
"__retval" in call "__retval = (char *)memcpy(__retval,
"ResourceAllocationSettingDataDeletedIndication", __len)" leaks the storage
that "__retval" points to.
Virt_VirtualSystemManagementService.c:2837: var_assign: Assigning: "__retval" =
storage returned from "memcpy(__retval,
"ResourceAllocationSettingDataDeletedIndication", __len)".
Virt_VirtualSystemManagementService.c:2837: var_assign: Assigning: "indication"
= "__retval".
Virt_VirtualSystemManagementService.c:2901: leaked_storage: Variable
"indication" going out of scope leaks the storage it points to.
Error: RESOURCE_LEAK:
Virt_VirtualSystemManagementService.c:2840: alloc_fn: Calling allocation function
"malloc".
Virt_VirtualSystemManagementService.c:2840: var_assign: Assigning: "__retval" =
storage returned from "malloc(__len)".
Virt_VirtualSystemManagementService.c:2840: noescape: Variable "__retval" is not
freed or pointed-to in function "memcpy".
Virt_VirtualSystemManagementService.c:2840: overwrite_var: Overwriting
"__retval" in call "__retval = (char *)memcpy(__retval,
"ResourceAllocationSettingDataModifiedIndication", __len)" leaks the
storage that "__retval" points to.
Virt_VirtualSystemManagementService.c:2840: var_assign: Assigning: "__retval" =
storage returned from "memcpy(__retval,
"ResourceAllocationSettingDataModifiedIndication", __len)".
Virt_VirtualSystemManagementService.c:2840: var_assign: Assigning: "indication"
= "__retval".
Virt_VirtualSystemManagementService.c:2901: leaked_storage: Variable
"indication" going out of scope leaks the storage it points to.
Error: RESOURCE_LEAK:
Virt_AppliedFilterList.c:199: alloc_arg: Calling allocation function
"get_domain_list" on "doms".
cs_util_instance.c:52: alloc_fn: Storage is returned from allocation function
"calloc".
cs_util_instance.c:52: var_assign: Assigning: "list" = "calloc(n_names +
n_ids, 8UL)".
cs_util_instance.c:107: var_assign: Assigning: "*_list" = "list".
Virt_AppliedFilterList.c:251: leaked_storage: Variable "doms" going out of scope
leaks the storage it points to.
Error: RESOURCE_LEAK:
misc_util.c:275: alloc_arg: Calling allocation function "get_domain_list" on
"list".
cs_util_instance.c:52: alloc_fn: Storage is returned from allocation function
"calloc".
cs_util_instance.c:52: var_assign: Assigning: "list" = "calloc(n_names +
n_ids, 8UL)".
cs_util_instance.c:107: var_assign: Assigning: "*_list" = "list".
misc_util.c:277: leaked_storage: Variable "list" going out of scope leaks the
storage it points to.
Error: RESOURCE_LEAK:
Virt_SwitchService.c:108: alloc_fn: Calling allocation function "popen".
Virt_SwitchService.c:108: var_assign: Assigning: "stream" = storage returned
from "popen(func, "r")".
Virt_SwitchService.c:118: noescape: Variable "stream" is not freed or pointed-to
in function "fgets".
Virt_SwitchService.c:131: leaked_storage: Variable "stream" going out of scope
leaks the storage it points to.
Virt_SwitchService.c:142: leaked_storage: Variable "stream" going out of scope
leaks the storage it points to.
Error: RESOURCE_LEAK:
Virt_SwitchService.c:123: alloc_fn: Calling allocation function "realloc".
Virt_SwitchService.c:123: var_assign: Assigning: "tmp_list" = storage returned
from "realloc(arr, (i + 1) * 8UL)".
Virt_SwitchService.c:134: var_assign: Assigning: "arr" = "tmp_list".
Virt_SwitchService.c:142: leaked_storage: Variable "arr" going out of scope
leaks the storage it points to.
Virt_SwitchService.c:142: leaked_storage: Variable "tmp_list" going out of scope
leaks the storage it points to.
Error: RESOURCE_LEAK:
Virt_SwitchService.c:236: alloc_fn: Calling allocation function "run_command".
Virt_SwitchService.c:123: alloc_fn: Storage is returned from allocation function
"realloc".
Virt_SwitchService.c:123: var_assign: Assigning: "tmp_list" = "realloc(arr,
(i + 1) * 8UL)".
Virt_SwitchService.c:134: var_assign: Assigning: "arr" = "tmp_list".
Virt_SwitchService.c:151: return_alloc: Returning allocated memory "arr".
Virt_SwitchService.c:236: var_assign: Assigning: "if_list" = storage returned
from "run_command("/sbin/ifconfig -a | /bin/grep eth | /bin/awk
\'{print$1}\'", &count, &s)".
/builddir/build/BUILD/libvirt-cim-0.6.0/src/Virt_SwitchService.c:269: leaked_storage:
Variable "if_list" going out of scope leaks the storage it points to.
Signed-off-by: Eduardo Lima (Etrunko) <eblima(a)br.ibm.com>
---
libxkutil/cs_util_instance.c | 5 +++++
libxkutil/device_parsing.c | 1 +
libxkutil/pool_parsing.c | 5 +++--
libxkutil/xmlgen.c | 4 +++-
src/Virt_AppliedFilterList.c | 3 ++-
src/Virt_DevicePool.c | 5 ++++-
src/Virt_SwitchService.c | 24 ++++++++++++++++++++----
src/Virt_VirtualSystemManagementService.c | 14 +++++++-------
8 files changed, 45 insertions(+), 16 deletions(-)
diff --git a/libxkutil/cs_util_instance.c b/libxkutil/cs_util_instance.c
index d21f0ff..a383147 100644
--- a/libxkutil/cs_util_instance.c
+++ b/libxkutil/cs_util_instance.c
@@ -104,6 +104,11 @@ int get_domain_list(virConnectPtr conn, virDomainPtr **_list)
free(names);
free(ids);
+ if (idx == 0) {
+ free(list);
+ list = NULL;
+ }
+
*_list = list;
return idx;
diff --git a/libxkutil/device_parsing.c b/libxkutil/device_parsing.c
index b0eccfc..a1e8d6c 100644
--- a/libxkutil/device_parsing.c
+++ b/libxkutil/device_parsing.c
@@ -382,6 +382,7 @@ static int parse_vsi_device(xmlNode *dnode, struct net_device *vdevs)
}
memcpy(&(vdevs->vsi), vsi_dev, sizeof(*vsi_dev));
+ free(vsi_dev);
return 1;
err:
diff --git a/libxkutil/pool_parsing.c b/libxkutil/pool_parsing.c
index f73b0fd..e41fc09 100644
--- a/libxkutil/pool_parsing.c
+++ b/libxkutil/pool_parsing.c
@@ -163,10 +163,11 @@ static int parse_disk_source(xmlNode *node, struct disk_pool *pool)
pool->device_paths_ct = ct;
pool->device_paths = dev_paths;
+ return 1;
err:
-
- return 1;
+ free(dev_paths);
+ return 0;
}
char *get_disk_pool_type(uint16_t type)
diff --git a/libxkutil/xmlgen.c b/libxkutil/xmlgen.c
index 96b4e96..7fff4d1 100644
--- a/libxkutil/xmlgen.c
+++ b/libxkutil/xmlgen.c
@@ -928,7 +928,6 @@ char *device_to_xml(struct virt_device *_dev)
dominfo->dev_input = dev;
break;
default:
- cleanup_virt_devices(&dev, 1);
goto out;
}
@@ -942,6 +941,9 @@ char *device_to_xml(struct virt_device *_dev)
out:
CU_DEBUG("Created Device XML:\n%s\n", xml);
+ if (dev != NULL)
+ cleanup_virt_devices(&dev, 1);
+
cleanup_dominfo(&dominfo);
xmlFreeNode(root);
diff --git a/src/Virt_AppliedFilterList.c b/src/Virt_AppliedFilterList.c
index 6567118..bc31c14 100644
--- a/src/Virt_AppliedFilterList.c
+++ b/src/Virt_AppliedFilterList.c
@@ -197,7 +197,7 @@ static CMPIStatus list_to_net(
/* get domains */
dcount = get_domain_list(conn, &doms);
- if (dcount < 0) {
+ if (dcount <= 0) {
cu_statusf(_BROKER, &s,
CMPI_RC_ERR_FAILED,
"Failed to get domain list");
@@ -246,6 +246,7 @@ static CMPIStatus list_to_net(
}
out:
+ free(doms);
virConnectClose(conn);
return s;
diff --git a/src/Virt_DevicePool.c b/src/Virt_DevicePool.c
index ab0baa0..fe5573f 100644
--- a/src/Virt_DevicePool.c
+++ b/src/Virt_DevicePool.c
@@ -402,8 +402,10 @@ static char *_diskpool_member_of(virConnectPtr conn,
char *pool = NULL;
count = get_diskpool_config(conn, &pools);
- if (count == 0)
+ if (count == 0) {
+ free(pools);
return NULL;
+ }
for (i = 0; i < count; i++) {
if (_diskpool_is_member(conn, &pools[i], file)) {
@@ -1091,6 +1093,7 @@ static CMPIStatus diskpool_instance(virConnectPtr conn,
count = get_diskpool_config(conn, &pools);
if ((id == NULL) && (count == 0)) {
CU_DEBUG("No defined DiskPools");
+ free(pools);
return s;
}
diff --git a/src/Virt_SwitchService.c b/src/Virt_SwitchService.c
index 0d57f54..7e59d38 100644
--- a/src/Virt_SwitchService.c
+++ b/src/Virt_SwitchService.c
@@ -128,20 +128,19 @@ static char **run_command(char *func, int *len, CMPIStatus *s) {
cu_statusf(_BROKER, s,
CMPI_RC_ERR_NOT_FOUND,
"Failed to realloc");
- return NULL;
+ goto err;
}
arr = tmp_list;
- string = calloc(len, sizeof(char));
+ string = strndup(buff, len);
if (string == NULL) {
CU_DEBUG("Failed to allocate memory");
cu_statusf(_BROKER, s,
CMPI_RC_ERR_NOT_FOUND,
"Failed to calloc");
- return NULL;
+ goto err;
}
- strncpy(string, buff, len);
arr[i] = string;
i++;
}
@@ -149,6 +148,19 @@ static char **run_command(char *func, int *len, CMPIStatus *s) {
pclose(stream);
*len = i;
return arr;
+
+ err:
+ /* undo everything */
+ if (i > 0) {
+ int count;
+ for (count = 0; count < i; count++)
+ free(arr[count]);
+ }
+
+ free(arr);
+ pclose(stream);
+ return NULL;
+
}
static CMPIStatus set_inst_properties(const CMPIBroker *broker,
@@ -262,6 +274,10 @@ static CMPIStatus get_switchservice(const CMPIObjectPath *reference,
CMSetProperty(inst, "IsVSISupported", (CMPIValue *)&vsi,
CMPI_boolean);
s.rc = CMPI_RC_OK;
+ for (i = 0; i < count; i++)
+ free(if_list[i]);
+
+ free(if_list);
out:
virConnectClose(conn);
*_inst = inst;
diff --git a/src/Virt_VirtualSystemManagementService.c
b/src/Virt_VirtualSystemManagementService.c
index f8c5f24..3a0b423 100644
--- a/src/Virt_VirtualSystemManagementService.c
+++ b/src/Virt_VirtualSystemManagementService.c
@@ -429,6 +429,7 @@ static int bootord_vssd_to_domain(CMPIInstance *inst,
if (CMIsNullValue(boot_elem)) {
CU_DEBUG("Null BootDevice");
+ free(tmp_str_arr);
return 0;
}
@@ -437,6 +438,7 @@ static int bootord_vssd_to_domain(CMPIInstance *inst,
if (str == NULL) {
CU_DEBUG("Could not extract char pointer from "
"CMPIArray");
+ free(tmp_str_arr);
return 0;
}
@@ -2766,13 +2768,11 @@ static CMPIStatus _update_resources_for(const CMPIContext
*context,
}
if (func == &resource_add) {
- indication = strdup(RASD_IND_CREATED);
- }
- else if (func == &resource_del) {
- indication = strdup(RASD_IND_DELETED);
- }
- else {
- indication = strdup(RASD_IND_MODIFIED);
+ indication = RASD_IND_CREATED;
+ } else if (func == &resource_del) {
+ indication = RASD_IND_DELETED;
+ } else {
+ indication = RASD_IND_MODIFIED;
char *dummy_name = NULL;
if (asprintf(&dummy_name, "%s/%s",dominfo->name, devid)
== -1) {
--
1.7.7.5
12:36 p.m.
New subject: [PATCH 3/4] xml_parse_test: Fix invalid dereference
From: "Eduardo Lima (Etrunko)" <eblima(a)br.ibm.com>
As revealed by recent Coverity scan report provided by Red Hat:
https://bugzilla.redhat.com/show_bug.cgi?id=750418
https://bugzilla.redhat.com/attachment.cgi?id=552325
Error: USE_AFTER_FREE:
xml_parse_test.c:239: freed_arg: "free" frees "xml".
xml_parse_test.c:242: pass_freed_arg: Passing freed pointer "xml" as an argument
to function "printf".
Signed-off-by: Eduardo Lima (Etrunko) <eblima(a)br.ibm.com>
---
libxkutil/xml_parse_test.c | 3 +--
1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/libxkutil/xml_parse_test.c b/libxkutil/xml_parse_test.c
index 16ceefb..384593d 100644
--- a/libxkutil/xml_parse_test.c
+++ b/libxkutil/xml_parse_test.c
@@ -236,11 +236,10 @@ static int dominfo_from_file(const char *fname, struct domain **d)
ret = get_dominfo_from_xml(xml, d);
+ printf("XML:\n%s", xml);
free(xml);
fclose(file);
- printf("XML:\n%s", xml);
-
return ret;
}
--
1.7.7.5
12:36 p.m.
New subject: [PATCH 4/4] Fix possible use of unitialized variables
From: "Eduardo Lima (Etrunko)" <eblima(a)br.ibm.com>
As revealed by recent Coverity scan report provided by Red Hat:
https://bugzilla.redhat.com/show_bug.cgi?id=750418
https://bugzilla.redhat.com/attachment.cgi?id=552325
Error: UNINIT:
Virt_VirtualSystemManagementService.c:2798: var_decl: Declaring variable "s"
without initializer.
Virt_VirtualSystemManagementService.c:2901: uninit_use: Using uninitialized value
"s": field "s".msg is uninitialized.
Error: UNINIT:
Virt_VirtualSystemSnapshotService.c:393: var_decl: Declaring variable "s"
without initializer.
Virt_VirtualSystemSnapshotService.c:398: uninit_use_in_call: Using uninitialized value
"s.rc" when calling "new_context".
Virt_VirtualSystemSnapshotService.c:378: read_parm_fld: Reading a parameter field.
Signed-off-by: Eduardo Lima (Etrunko) <eblima(a)br.ibm.com>
---
src/Virt_VirtualSystemManagementService.c | 2 +-
src/Virt_VirtualSystemSnapshotService.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/Virt_VirtualSystemManagementService.c
b/src/Virt_VirtualSystemManagementService.c
index 3a0b423..6f42c42 100644
--- a/src/Virt_VirtualSystemManagementService.c
+++ b/src/Virt_VirtualSystemManagementService.c
@@ -2732,7 +2732,7 @@ static CMPIStatus _update_resources_for(const CMPIContext *context,
CMPIInstance *rasd,
resmod_fn func)
{
- CMPIStatus s;
+ CMPIStatus s = {CMPI_RC_OK, NULL};
struct domain *dominfo = NULL;
uint16_t type;
char *xml = NULL;
diff --git a/src/Virt_VirtualSystemSnapshotService.c
b/src/Virt_VirtualSystemSnapshotService.c
index 898fa57..aae628f 100644
--- a/src/Virt_VirtualSystemSnapshotService.c
+++ b/src/Virt_VirtualSystemSnapshotService.c
@@ -390,7 +390,7 @@ static CMPIStatus start_snapshot_job(const CMPIObjectPath *ref,
CMPIArgs *argsout)
{
struct snap_context *ctx;
- CMPIStatus s;
+ CMPIStatus s = {CMPI_RC_OK, NULL};
CMPIObjectPath *job;
CMPIObjectPath *vssd;
CMPIInstance *inst;
--
1.7.7.5
4684
days inactive
4684
days old
5 comments
1 participants
participants (1)
-
Eduardo Lima (Etrunko)