lib: Check calls to xmlXPathEvalExpression for nodesetval == NULL.

libxml2 xmlXPathEvalExpression can return an unusual nodeset:

  $1 = {type = XPATH_NODESET, nodesetval = 0x0, boolval = 0, floatval = 0,
    stringval = 0x0, user = 0x0, index = 0, user2 = 0x0, index2 = 0}

Note that the nodeset is non-NULL, but the nodesetval is NULL.

Check every call site and fix those that don't deal with this
correctly.
This commit is contained in:
Richard W.M. Jones
2013-03-01 14:44:51 +00:00
parent 3adb0d6d01
commit 7f5bedd53b
4 changed files with 162 additions and 146 deletions

View File

@@ -786,6 +786,8 @@ do_xpath (const char *query)
switch (xpathObj->type) {
case XPATH_NODESET:
nodes = xpathObj->nodesetval;
if (nodes == NULL)
break;
saveCtx = xmlSaveToFd (STDOUT_FILENO, NULL, XML_SAVE_NO_DECL);
if (saveCtx == NULL) {

View File

@@ -528,21 +528,24 @@ parse_capabilities (guestfs_h *g, const char *capabilities_xml,
nodes = xpathObj->nodesetval;
seen_qemu = seen_kvm = 0;
for (i = 0; i < (size_t) nodes->nodeNr; ++i) {
CLEANUP_FREE char *type = NULL;
if (seen_qemu && seen_kvm)
break;
if (nodes != NULL) {
for (i = 0; i < (size_t) nodes->nodeNr; ++i) {
CLEANUP_FREE char *type = NULL;
assert (nodes->nodeTab[i]);
assert (nodes->nodeTab[i]->type == XML_ATTRIBUTE_NODE);
attr = (xmlAttrPtr) nodes->nodeTab[i];
type = (char *) xmlNodeListGetString (doc, attr->children, 1);
if (seen_qemu && seen_kvm)
break;
if (STREQ (type, "qemu"))
seen_qemu++;
else if (STREQ (type, "kvm"))
seen_kvm++;
assert (nodes->nodeTab[i]);
assert (nodes->nodeTab[i]->type == XML_ATTRIBUTE_NODE);
attr = (xmlAttrPtr) nodes->nodeTab[i];
type = (char *) xmlNodeListGetString (doc, attr->children, 1);
if (STREQ (type, "qemu"))
seen_qemu++;
else if (STREQ (type, "kvm"))
seen_kvm++;
}
}
/* This was RHBZ#886915: in that case the default libvirt URI

View File

@@ -360,6 +360,9 @@ libvirt_selinux_label (guestfs_h *g, xmlDocPtr doc,
}
nodes = xpathObj->nodesetval;
if (nodes == NULL)
return 0;
nr_nodes = nodes->nodeNr;
if (nr_nodes == 0)
@@ -427,93 +430,97 @@ for_each_disk (guestfs_h *g,
}
nodes = xpathObj->nodesetval;
nr_nodes = nodes->nodeNr;
for (i = 0; i < nr_nodes; ++i) {
CLEANUP_FREE char *type = NULL, *filename = NULL, *format = NULL;
CLEANUP_XMLXPATHFREEOBJECT xmlXPathObjectPtr xptype = NULL;
CLEANUP_XMLXPATHFREEOBJECT xmlXPathObjectPtr xpformat = NULL;
CLEANUP_XMLXPATHFREEOBJECT xmlXPathObjectPtr xpreadonly = NULL;
CLEANUP_XMLXPATHFREEOBJECT xmlXPathObjectPtr xpfilename = NULL;
xmlAttrPtr attr;
int readonly;
int t;
if (nodes != NULL) {
nr_nodes = nodes->nodeNr;
for (i = 0; i < nr_nodes; ++i) {
CLEANUP_FREE char *type = NULL, *filename = NULL, *format = NULL;
CLEANUP_XMLXPATHFREEOBJECT xmlXPathObjectPtr xptype = NULL;
CLEANUP_XMLXPATHFREEOBJECT xmlXPathObjectPtr xpformat = NULL;
CLEANUP_XMLXPATHFREEOBJECT xmlXPathObjectPtr xpreadonly = NULL;
CLEANUP_XMLXPATHFREEOBJECT xmlXPathObjectPtr xpfilename = NULL;
xmlAttrPtr attr;
int readonly;
int t;
/* Change the context to the current <disk> node.
* DV advises to reset this before each search since older versions of
* libxml2 might overwrite it.
*/
xpathCtx->node = nodes->nodeTab[i];
/* Filename can be in <source dev=..> or <source file=..> attribute.
* Check the <disk type=..> attribute first to find out which one.
*/
xptype = xmlXPathEvalExpression (BAD_CAST "./@type", xpathCtx);
if (xptype == NULL ||
xptype->nodesetval == NULL ||
xptype->nodesetval->nodeNr == 0) {
continue; /* no type attribute, skip it */
}
assert (xptype->nodesetval->nodeTab[0]);
assert (xptype->nodesetval->nodeTab[0]->type == XML_ATTRIBUTE_NODE);
attr = (xmlAttrPtr) xptype->nodesetval->nodeTab[0];
type = (char *) xmlNodeListGetString (doc, attr->children, 1);
if (STREQ (type, "file")) { /* type = "file" so look at source/@file */
/* Change the context to the current <disk> node.
* DV advises to reset this before each search since older versions of
* libxml2 might overwrite it.
*/
xpathCtx->node = nodes->nodeTab[i];
xpfilename = xmlXPathEvalExpression (BAD_CAST "./source/@file", xpathCtx);
if (xpfilename == NULL ||
xpfilename->nodesetval == NULL ||
xpfilename->nodesetval->nodeNr == 0) {
continue; /* disk filename not found, skip this */
/* Filename can be in <source dev=..> or <source file=..> attribute.
* Check the <disk type=..> attribute first to find out which one.
*/
xptype = xmlXPathEvalExpression (BAD_CAST "./@type", xpathCtx);
if (xptype == NULL ||
xptype->nodesetval == NULL ||
xptype->nodesetval->nodeNr == 0) {
continue; /* no type attribute, skip it */
}
} else if (STREQ (type, "block")) { /* type = "block", use source/@dev */
assert (xptype->nodesetval->nodeTab[0]);
assert (xptype->nodesetval->nodeTab[0]->type == XML_ATTRIBUTE_NODE);
attr = (xmlAttrPtr) xptype->nodesetval->nodeTab[0];
type = (char *) xmlNodeListGetString (doc, attr->children, 1);
if (STREQ (type, "file")) { /* type = "file" so look at source/@file */
xpathCtx->node = nodes->nodeTab[i];
xpfilename = xmlXPathEvalExpression (BAD_CAST "./source/@file",
xpathCtx);
if (xpfilename == NULL ||
xpfilename->nodesetval == NULL ||
xpfilename->nodesetval->nodeNr == 0) {
continue; /* disk filename not found, skip this */
}
} else if (STREQ (type, "block")) { /* type = "block", use source/@dev */
xpathCtx->node = nodes->nodeTab[i];
xpfilename = xmlXPathEvalExpression (BAD_CAST "./source/@dev",
xpathCtx);
if (xpfilename == NULL ||
xpfilename->nodesetval == NULL ||
xpfilename->nodesetval->nodeNr == 0) {
continue; /* disk filename not found, skip this */
}
} else
continue; /* type <> "file" or "block", skip it */
assert (xpfilename);
assert (xpfilename->nodesetval);
assert (xpfilename->nodesetval->nodeTab[0]);
assert (xpfilename->nodesetval->nodeTab[0]->type == XML_ATTRIBUTE_NODE);
attr = (xmlAttrPtr) xpfilename->nodesetval->nodeTab[0];
filename = (char *) xmlNodeListGetString (doc, attr->children, 1);
/* Get the disk format (may not be set). */
xpathCtx->node = nodes->nodeTab[i];
xpfilename = xmlXPathEvalExpression (BAD_CAST "./source/@dev", xpathCtx);
if (xpfilename == NULL ||
xpfilename->nodesetval == NULL ||
xpfilename->nodesetval->nodeNr == 0) {
continue; /* disk filename not found, skip this */
xpformat = xmlXPathEvalExpression (BAD_CAST "./driver/@type", xpathCtx);
if (xpformat != NULL &&
xpformat->nodesetval &&
xpformat->nodesetval->nodeNr > 0) {
assert (xpformat->nodesetval->nodeTab[0]);
assert (xpformat->nodesetval->nodeTab[0]->type == XML_ATTRIBUTE_NODE);
attr = (xmlAttrPtr) xpformat->nodesetval->nodeTab[0];
format = (char *) xmlNodeListGetString (doc, attr->children, 1);
}
} else
continue; /* type <> "file" or "block", skip it */
assert (xpfilename);
assert (xpfilename->nodesetval);
assert (xpfilename->nodesetval->nodeTab[0]);
assert (xpfilename->nodesetval->nodeTab[0]->type == XML_ATTRIBUTE_NODE);
attr = (xmlAttrPtr) xpfilename->nodesetval->nodeTab[0];
filename = (char *) xmlNodeListGetString (doc, attr->children, 1);
/* Get the <readonly/> flag. */
xpathCtx->node = nodes->nodeTab[i];
xpreadonly = xmlXPathEvalExpression (BAD_CAST "./readonly", xpathCtx);
readonly = 0;
if (xpreadonly != NULL &&
xpreadonly->nodesetval &&
xpreadonly->nodesetval->nodeNr > 0)
readonly = 1;
/* Get the disk format (may not be set). */
xpathCtx->node = nodes->nodeTab[i];
xpformat = xmlXPathEvalExpression (BAD_CAST "./driver/@type", xpathCtx);
if (xpformat != NULL &&
xpformat->nodesetval &&
xpformat->nodesetval->nodeNr > 0) {
assert (xpformat->nodesetval->nodeTab[0]);
assert (xpformat->nodesetval->nodeTab[0]->type == XML_ATTRIBUTE_NODE);
attr = (xmlAttrPtr) xpformat->nodesetval->nodeTab[0];
format = (char *) xmlNodeListGetString (doc, attr->children, 1);
if (f)
t = f (g, filename, format, readonly, data);
else
t = 0;
if (t == -1)
return -1;
nr_added++;
}
/* Get the <readonly/> flag. */
xpathCtx->node = nodes->nodeTab[i];
xpreadonly = xmlXPathEvalExpression (BAD_CAST "./readonly", xpathCtx);
readonly = 0;
if (xpreadonly != NULL &&
xpreadonly->nodesetval &&
xpreadonly->nodesetval->nodeNr > 0)
readonly = 1;
if (f)
t = f (g, filename, format, readonly, data);
else
t = 0;
if (t == -1)
return -1;
nr_added++;
}
if (nr_added == 0) {
@@ -561,26 +568,28 @@ connect_live (guestfs_h *g, virDomainPtr dom)
}
nodes = xpathObj->nodesetval;
for (i = 0; i < nodes->nodeNr; ++i) {
CLEANUP_XMLXPATHFREEOBJECT xmlXPathObjectPtr xppath = NULL;
xmlAttrPtr attr;
if (nodes != NULL) {
for (i = 0; i < nodes->nodeNr; ++i) {
CLEANUP_XMLXPATHFREEOBJECT xmlXPathObjectPtr xppath = NULL;
xmlAttrPtr attr;
/* See note in function above. */
xpathCtx->node = nodes->nodeTab[i];
/* See note in function above. */
xpathCtx->node = nodes->nodeTab[i];
/* The path is in <source path=..> attribute. */
xppath = xmlXPathEvalExpression (BAD_CAST "./source/@path", xpathCtx);
if (xppath == NULL ||
xppath->nodesetval == NULL ||
xppath->nodesetval->nodeNr == 0) {
xmlXPathFreeObject (xppath);
continue; /* no type attribute, skip it */
/* The path is in <source path=..> attribute. */
xppath = xmlXPathEvalExpression (BAD_CAST "./source/@path", xpathCtx);
if (xppath == NULL ||
xppath->nodesetval == NULL ||
xppath->nodesetval->nodeNr == 0) {
xmlXPathFreeObject (xppath);
continue; /* no type attribute, skip it */
}
assert (xppath->nodesetval->nodeTab[0]);
assert (xppath->nodesetval->nodeTab[0]->type == XML_ATTRIBUTE_NODE);
attr = (xmlAttrPtr) xppath->nodesetval->nodeTab[0];
path = (char *) xmlNodeListGetString (doc, attr->children, 1);
break;
}
assert (xppath->nodesetval->nodeTab[0]);
assert (xppath->nodesetval->nodeTab[0]->type == XML_ATTRIBUTE_NODE);
attr = (xmlAttrPtr) xppath->nodesetval->nodeTab[0];
path = (char *) xmlNodeListGetString (doc, attr->children, 1);
break;
}
if (path == NULL) {

View File

@@ -298,51 +298,53 @@ read_osinfo_db_xml (guestfs_h *g, const char *filename)
nodes = xpathObj->nodesetval;
for (i = 0; i < (size_t) nodes->nodeNr; ++i) {
iso_node = nodes->nodeTab[i];
assert (iso_node != NULL);
assert (STREQ ((const char *) iso_node->name, "iso"));
assert (iso_node->type == XML_ELEMENT_NODE);
if (nodes != NULL) {
for (i = 0; i < (size_t) nodes->nodeNr; ++i) {
iso_node = nodes->nodeTab[i];
assert (iso_node != NULL);
assert (STREQ ((const char *) iso_node->name, "iso"));
assert (iso_node->type == XML_ELEMENT_NODE);
media_node = iso_node->parent;
assert (media_node != NULL);
assert (STREQ ((const char *) media_node->name, "media"));
assert (media_node->type == XML_ELEMENT_NODE);
media_node = iso_node->parent;
assert (media_node != NULL);
assert (STREQ ((const char *) media_node->name, "media"));
assert (media_node->type == XML_ELEMENT_NODE);
os_node = media_node->parent;
assert (os_node != NULL);
assert (STREQ ((const char *) os_node->name, "os"));
assert (os_node->type == XML_ELEMENT_NODE);
os_node = media_node->parent;
assert (os_node != NULL);
assert (STREQ ((const char *) os_node->name, "os"));
assert (os_node->type == XML_ELEMENT_NODE);
/* Allocate an osinfo record. */
osinfo_db_size++;
osinfo_db = safe_realloc (g, osinfo_db,
sizeof (struct osinfo) * osinfo_db_size);
osinfo = &osinfo_db[osinfo_db_size-1];
memset (osinfo, 0, sizeof *osinfo);
/* Allocate an osinfo record. */
osinfo_db_size++;
osinfo_db = safe_realloc (g, osinfo_db,
sizeof (struct osinfo) * osinfo_db_size);
osinfo = &osinfo_db[osinfo_db_size-1];
memset (osinfo, 0, sizeof *osinfo);
/* Read XML fields into the new osinfo record. */
if (read_iso_node (g, iso_node, osinfo) == -1 ||
read_media_node (g, xpathCtx, media_node, osinfo) == -1 ||
read_os_node (g, xpathCtx, os_node, osinfo) == -1) {
free_osinfo_db_entry (osinfo);
osinfo_db_size--;
return -1;
}
/* Read XML fields into the new osinfo record. */
if (read_iso_node (g, iso_node, osinfo) == -1 ||
read_media_node (g, xpathCtx, media_node, osinfo) == -1 ||
read_os_node (g, xpathCtx, os_node, osinfo) == -1) {
free_osinfo_db_entry (osinfo);
osinfo_db_size--;
return -1;
}
#if 0
debug (g, "osinfo: %s: %s%s%s%s=> arch %s live %s product %s type %d distro %d version %d.%d",
filename,
osinfo->re_system_id ? "<system-id/> " : "",
osinfo->re_volume_id ? "<volume-id/> " : "",
osinfo->re_publisher_id ? "<publisher-id/> " : "",
osinfo->re_application_id ? "<application-id/> " : "",
osinfo->arch ? osinfo->arch : "(none)",
osinfo->is_live_disk ? "true" : "false",
osinfo->product_name ? osinfo->product_name : "(none)",
(int) osinfo->type, (int) osinfo->distro,
osinfo->major_version, osinfo->minor_version);
debug (g, "osinfo: %s: %s%s%s%s=> arch %s live %s product %s type %d distro %d version %d.%d",
filename,
osinfo->re_system_id ? "<system-id/> " : "",
osinfo->re_volume_id ? "<volume-id/> " : "",
osinfo->re_publisher_id ? "<publisher-id/> " : "",
osinfo->re_application_id ? "<application-id/> " : "",
osinfo->arch ? osinfo->arch : "(none)",
osinfo->is_live_disk ? "true" : "false",
osinfo->product_name ? osinfo->product_name : "(none)",
(int) osinfo->type, (int) osinfo->distro,
osinfo->major_version, osinfo->minor_version);
#endif
}
}
return 0;