1:30 p.m.
A new version of Coverity found a number of issues:
get_pool_from_xml(): FORWARD_NULL
parse_disk_pool(): RESOURCE_LEAK
- If parse_disk_pool() returned name == NULL, then the strdup() of
name into pool->id would dereference the NULL pointer leading to
a segfault.
Furthermore parse_disk_pool() had a few issues. First the 'type_str'
could be NULL, so that needs to be checked. Second, 'type_str' was
never free()'d (the get_attr_value returns a strdup()'d value).
Realizing all that resulted in a few extra changes to not strdup()
a value that we strdup()'d
Eventually get_pool_from_xml() will return to get_disk_pool() which
returns to diskpool_set_capacity() or _new_volume_template() which
both error out when return value != 0 (although, I did change the
latter to be more explicit and match the former).
Finally, even though the parsing of the element will only ever have
one "name" value - Coverity doesn't know that, so as a benign fix be
sure to not overwrite 'name' if "name" isn't already set.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
libxkutil/pool_parsing.c | 39 ++++++++++++++++++-----------------
src/Virt_SettingsDefineCapabilities.c | 2 +-
2 files changed, 21 insertions(+), 20 deletions(-)
diff --git a/libxkutil/pool_parsing.c b/libxkutil/pool_parsing.c
index 922ff32..80bd4ca 100644
--- a/libxkutil/pool_parsing.c
+++ b/libxkutil/pool_parsing.c
@@ -194,15 +194,17 @@ char *get_disk_pool_type(uint16_t type)
}
-static const char *parse_disk_pool(xmlNodeSet *nsv, struct disk_pool *pool)
+static char *parse_disk_pool(xmlNodeSet *nsv, struct disk_pool *pool)
{
xmlNode **nodes = nsv->nodeTab;
xmlNode *child;
- const char *type_str = NULL;
- const char *name = NULL;
+ char *type_str = NULL;
+ char *name = NULL;
int type = 0;
type_str = get_attr_value(nodes[0], "type");
+ if (type_str == NULL)
+ return NULL;
if (STREQC(type_str, "dir"))
type = DISK_POOL_DIR;
@@ -220,12 +222,15 @@ static const char *parse_disk_pool(xmlNodeSet *nsv, struct disk_pool
*pool)
type = DISK_POOL_SCSI;
else
type = DISK_POOL_UNKNOWN;
+ free(type_str);
pool->pool_type = type;
-
+
for (child = nodes[0]->children; child != NULL; child = child->next) {
- if (XSTREQ(child->name, "name")) {
+ if (XSTREQ(child->name, "name") && name == NULL) {
name = get_node_content(child);
+ if (name == NULL)
+ return NULL;
} else if (XSTREQ(child->name, "target"))
parse_disk_target(child, pool);
else if (XSTREQ(child->name, "source"))
@@ -238,14 +243,18 @@ static const char *parse_disk_pool(xmlNodeSet *nsv, struct disk_pool
*pool)
int get_pool_from_xml(const char *xml, struct virt_pool *pool, int type)
{
int len;
- int ret = 0;
+ int ret = 1;
xmlDoc *xmldoc;
xmlXPathContext *xpathctx;
xmlXPathObject *xpathobj;
const xmlChar *xpathstr = (xmlChar *)"/pool";
- const char *name;
CU_DEBUG("Pool XML : %s", xml);
+
+ /* FIXME: Add support for parsing network pools */
+ if (type == CIM_RES_TYPE_NET)
+ return 0;
+
len = strlen(xml) + 1;
if ((xmldoc = xmlParseMemory(xml, len)) == NULL)
@@ -257,22 +266,14 @@ int get_pool_from_xml(const char *xml, struct virt_pool *pool, int
type)
if ((xpathobj = xmlXPathEvalExpression(xpathstr, xpathctx)) == NULL)
goto err3;
- /* FIXME: Add support for parsing network pools */
- if (type == CIM_RES_TYPE_NET) {
- ret = 0;
- goto err1;
- }
-
memset(pool, 0, sizeof(*pool));
- pool->type = CIM_RES_TYPE_DISK;
- name = parse_disk_pool(xpathobj->nodesetval,
- &(pool)->pool_info.disk);
- if (name == NULL)
+ pool->type = CIM_RES_TYPE_DISK;
+ pool->id = parse_disk_pool(xpathobj->nodesetval,
+ &(pool)->pool_info.disk);
+ if (pool->id != NULL)
ret = 0;
- pool->id = strdup(name);
-
xmlXPathFreeObject(xpathobj);
err3:
xmlXPathFreeContext(xpathctx);
diff --git a/src/Virt_SettingsDefineCapabilities.c
b/src/Virt_SettingsDefineCapabilities.c
index fe16e3f..756e46b 100644
--- a/src/Virt_SettingsDefineCapabilities.c
+++ b/src/Virt_SettingsDefineCapabilities.c
@@ -1376,7 +1376,7 @@ static CMPIStatus _new_volume_template(const CMPIObjectPath *ref,
}
ret = get_disk_pool(poolptr, &pool);
- if (ret == 1) {
+ if (ret != 0) {
virt_set_status(_BROKER, &s,
CMPI_RC_ERR_FAILED,
virStoragePoolGetConnect(poolptr),
--
1.8.4.2