[Libvirt-cim] [PATCH] Fix crash when creating ACL filter lists
Chip Vincent
cvincent at linux.vnet.ibm.com
Wed Jul 20 21:35:56 UTC 2011
# HG changeset patch
# User Chip Vincent <cvincent at us.ibm.com>
# Date 1311193039 14400
# Node ID c4fae0f5cc8f21f0d66077df7693158c10633ece
# Parent 6056961c3c5347d3b8375039767d7bc78fa97eb5
Fix crash when creating ACL filter lists
Specifically, AppliedFilterList and NestedFilterList CreateInstance().
For AppliedFilterList:
The Antecedent and Dependent were backwards and the provider would crash
since no libvirt connection was made before calling get_filter_by_name().
Also added some additional error checking in acl_parsing.c. It appears
libvirt w/ qemu does not support dynamic updating of virtual net devices
in 0.8.1 so CreateInstance() will still fail (but not crash) in that
version. Looking for alternatives to provide dynamic support and will post
in a follow-on patch.
For NestedFilterList:
The code was trying to extract the 'Name' property from the instance reference,
rather than the Antecedent and Dependent properties. Also fixed an issue
when creating the filter XML.
NOTE: I'm still seeing sparatic cores in various providers when doing
'deep' or 'broad' associators calls (not using the -ac). However, there is no
indication that bug is tied to this feature. For example, I cannot get the
ACL providers to crash when using 'wbemcli ai -ac'.
Signed-off-by: Chip Vincent <cvincent at us.ibm.com>
diff --git a/libxkutil/acl_parsing.c b/libxkutil/acl_parsing.c
--- a/libxkutil/acl_parsing.c
+++ b/libxkutil/acl_parsing.c
@@ -429,6 +429,7 @@
int get_filter_from_xml(const char *xml, struct acl_filter **filter)
{
xmlDoc *xmldoc = NULL;
+ int ret = 0;
if (xml == NULL || filter == NULL)
return 0;
@@ -440,15 +441,19 @@
goto err;
*filter = malloc(sizeof(**filter));
+ if (*filter == NULL)
+ goto err;
memset(*filter, 0, sizeof(**filter));
parse_acl_filter(xmldoc->children, *filter);
+ ret = 1;
+
err:
xmlSetGenericErrorFunc(NULL, NULL);
xmlFreeDoc(xmldoc);
- return 1;
+ return ret;
}
int get_filter_by_name(
@@ -464,6 +469,8 @@
return 0;
vfilter = virNWFilterLookupByName(conn, name);
+ if (vfilter == NULL)
+ return 0;
xml = virNWFilterGetXMLDesc(vfilter, 0);
@@ -472,9 +479,7 @@
if (xml == NULL)
return 0;
- get_filter_from_xml(xml, filter);
-
- return 1;
+ return get_filter_from_xml(xml, filter);
#else
return 0;
#endif
@@ -493,6 +498,8 @@
return 0;
vfilter = virNWFilterLookupByUUIDString(conn, uuid);
+ if (vfilter == NULL)
+ return 0;
xml = virNWFilterGetXMLDesc(vfilter, 0);
@@ -581,31 +588,27 @@
int update_filter(virConnectPtr conn, struct acl_filter *filter)
{
- if (delete_filter(conn, filter) == 0 ||
- create_filter(conn, filter) == 0)
- return 0;
-
- return 1;
+ return create_filter(conn, filter);
}
int delete_filter(virConnectPtr conn, struct acl_filter *filter)
{
#if LIBVIR_VERSION_NUMBER > 8000
+ int ret = 0;
virNWFilterPtr vfilter = NULL;
if (filter == NULL)
return 0;
- vfilter = virNWFilterLookupByUUIDString(conn, filter->uuid);
+ vfilter = virNWFilterLookupByName(conn, filter->name);
if (vfilter == NULL)
return 0;
- if (virNWFilterUndefine(vfilter) != 0) {
- virNWFilterFree(vfilter);
- return 0;
- }
+ ret = virNWFilterUndefine(vfilter);
- return 1;
+ virNWFilterFree(vfilter);
+
+ return ret == 0 ? 1 : 0;
#else
return 0;
#endif
diff --git a/libxkutil/xmlgen.c b/libxkutil/xmlgen.c
--- a/libxkutil/xmlgen.c
+++ b/libxkutil/xmlgen.c
@@ -1375,14 +1375,16 @@
if (xmlNewProp(root, BAD_CAST "name", BAD_CAST filter->name) == NULL)
goto out;
- if (filter->chain != NULL)
+ if (filter->chain != NULL) {
if (xmlNewProp(root, BAD_CAST "chain",
BAD_CAST filter->chain) == NULL)
goto out;
+ }
if (filter->uuid != NULL) {
- tmp = xmlNewChild(root, NULL, BAD_CAST "uuid", NULL);
- if (xmlNewProp(tmp, NULL, BAD_CAST filter->uuid) == NULL)
+ tmp = xmlNewChild(root, NULL, BAD_CAST "uuid",
+ BAD_CAST filter->uuid);
+ if (tmp == NULL)
goto out;
}
@@ -1406,7 +1408,7 @@
msg = NULL; /* no errors */
out:
- CU_DEBUG("Filter XML: %s", msg);
+ CU_DEBUG("Filter XML: %s", xml);
xmlFreeNode(root);
diff --git a/src/Virt_AppliedFilterList.c b/src/Virt_AppliedFilterList.c
--- a/src/Virt_AppliedFilterList.c
+++ b/src/Virt_AppliedFilterList.c
@@ -30,6 +30,7 @@
#include "acl_parsing.h"
#include "misc_util.h"
#include "cs_util.h"
+#include "xmlgen.h"
#include "Virt_Device.h"
#include "Virt_FilterList.h"
@@ -111,20 +112,14 @@
VIR_DOMAIN_DEVICE_MODIFY_CONFIG;
int ret = 0;
- /** device_to_xml() is not exported, so this function needs
- * to be moved
- */
-
- /* xml = device_to_xml(dev); */
-
+ xml = device_to_xml(dev);
if (xml == NULL) {
CU_DEBUG("Failed to get XML for device '%s'", dev->id);
goto out;
}
if (virDomainUpdateDeviceFlags(dom, xml, flags) != 0) {
- CU_DEBUG("Failed to dynamically update device:");
- CU_DEBUG("%s", xml);
+ CU_DEBUG("Failed to dynamically update device");
goto out;
}
@@ -430,6 +425,10 @@
virConnectPtr conn = NULL;
virDomainPtr dom = NULL;
+ conn = connect_by_classname(_BROKER, CLASSNAME(reference), &s);
+ if (conn == NULL)
+ goto out;
+
CU_DEBUG("Reference = %s", REF2STR(reference));
if (cu_get_ref_prop(instance, "Antecedent",
@@ -440,18 +439,13 @@
goto out;
}
- if (cu_get_str_path(reference, "Name", &filter_name) != CMPI_RC_OK) {
+ CU_DEBUG("Antecedent = %s", REF2STR(antecedent));
+
+ if (cu_get_str_path(antecedent, "DeviceID",
+ &device_name) != CMPI_RC_OK) {
cu_statusf(_BROKER, &s,
CMPI_RC_ERR_FAILED,
- "Unable to get Antecedent.Name property");
- goto out;
- }
-
- get_filter_by_name(conn, filter_name, &filter);
- if (filter == NULL) {
- cu_statusf(_BROKER, &s,
- CMPI_RC_ERR_FAILED,
- "Antecedent.Name object does not exist");
+ "Unable to get Antecedent.DeviceID property");
goto out;
}
@@ -463,15 +457,23 @@
goto out;
}
- if (cu_get_str_path(reference, "DeviceID",
- &device_name) != CMPI_RC_OK) {
+ CU_DEBUG("Dependent = %s", REF2STR(dependent));
+
+ if (cu_get_str_path(dependent, "Name",
+ &filter_name) != CMPI_RC_OK) {
cu_statusf(_BROKER, &s,
CMPI_RC_ERR_FAILED,
- "Unable to get Dependent.DeviceID property");
+ "Unable to get Dependent.Name property");
goto out;
}
- CU_DEBUG("DeviceID = %s", device_name);
+ get_filter_by_name(conn, filter_name, &filter);
+ if (filter == NULL) {
+ cu_statusf(_BROKER, &s,
+ CMPI_RC_ERR_FAILED,
+ "Antecedent.Name object does not exist");
+ goto out;
+ }
if (parse_fq_devid(device_name, &domain_name, &net_name) == 0) {
CU_DEBUG("Failed to parse devid");
@@ -539,6 +541,10 @@
virConnectPtr conn = NULL;
virDomainPtr dom = NULL;
+ conn = connect_by_classname(_BROKER, CLASSNAME(reference), &s);
+ if (conn == NULL)
+ goto out;
+
CU_DEBUG("Reference = %s", REF2STR(reference));
if (cu_get_ref_path(reference, "Antecedent",
@@ -549,18 +555,11 @@
goto out;
}
- if (cu_get_str_path(reference, "Name", &filter_name) != CMPI_RC_OK) {
+ if (cu_get_str_path(reference, "DeviceID",
+ &device_name) != CMPI_RC_OK) {
cu_statusf(_BROKER, &s,
CMPI_RC_ERR_FAILED,
- "Unable to get Antecedent.Name property");
- goto out;
- }
-
- get_filter_by_name(conn, filter_name, &filter);
- if (filter == NULL) {
- cu_statusf(_BROKER, &s,
- CMPI_RC_ERR_FAILED,
- "Antecedent.Name object does not exist");
+ "Unable to get Antecedent.DeviceID property");
goto out;
}
@@ -572,15 +571,21 @@
goto out;
}
- if (cu_get_str_path(reference, "DeviceID",
- &device_name) != CMPI_RC_OK) {
+ if (cu_get_str_path(reference, "Name",
+ &filter_name) != CMPI_RC_OK) {
cu_statusf(_BROKER, &s,
CMPI_RC_ERR_FAILED,
- "Unable to get Dependent.DeviceID property");
+ "Unable to get Dependent.Name property");
goto out;
}
- CU_DEBUG("DeviceID = %s", device_name);
+ get_filter_by_name(conn, filter_name, &filter);
+ if (filter == NULL) {
+ cu_statusf(_BROKER, &s,
+ CMPI_RC_ERR_FAILED,
+ "Antecedent.Name object does not exist");
+ goto out;
+ }
if (parse_fq_devid(device_name, &domain_name, &net_name) == 0) {
CU_DEBUG("Failed to parse devid");
diff --git a/src/Virt_FilterList.c b/src/Virt_FilterList.c
--- a/src/Virt_FilterList.c
+++ b/src/Virt_FilterList.c
@@ -269,6 +269,8 @@
CMPIInstance *_instance = NULL;
virConnectPtr conn = NULL;
+ CU_DEBUG("Reference = %s", REF2STR(reference));
+
/**Get Name from instance rather than reference since keys
* are set by this provider, not the client.
*/
@@ -335,6 +337,8 @@
struct acl_filter *filter = NULL;
virConnectPtr conn = NULL;
+ CU_DEBUG("Reference = %s", REF2STR(reference));
+
if (cu_get_str_path(reference, "Name", &name) != CMPI_RC_OK) {
cu_statusf(_BROKER, &s,
CMPI_RC_ERR_NOT_FOUND,
diff --git a/src/Virt_NestedFilterList.c b/src/Virt_NestedFilterList.c
--- a/src/Virt_NestedFilterList.c
+++ b/src/Virt_NestedFilterList.c
@@ -323,13 +323,17 @@
goto out;
}
- if (cu_get_str_path(reference, "Name", &parent_name) != CMPI_RC_OK) {
+ CU_DEBUG("Antecedent = %s", REF2STR(antecedent));
+
+ if (cu_get_str_path(antecedent, "Name", &parent_name) != CMPI_RC_OK) {
cu_statusf(_BROKER, &s,
CMPI_RC_ERR_FAILED,
"Unable to get Antecedent.Name property");
goto out;
}
+ CU_DEBUG("Antecedent.Name = %s", parent_name);
+
get_filter_by_name(conn, parent_name, &parent_filter);
if (parent_filter == NULL) {
cu_statusf(_BROKER, &s,
@@ -346,13 +350,17 @@
goto out;
}
- if (cu_get_str_path(reference, "Name", &child_name) != CMPI_RC_OK) {
+ CU_DEBUG("Dependent = %s", REF2STR(dependent));
+
+ if (cu_get_str_path(dependent, "Name", &child_name) != CMPI_RC_OK) {
cu_statusf(_BROKER, &s,
CMPI_RC_ERR_FAILED,
"Unable to get Dependent.Name property");
goto out;
}
+ CU_DEBUG("Dependent.Name = %s", child_name);
+
get_filter_by_name(conn, child_name, &child_filter);
if (child_filter == NULL) {
cu_statusf(_BROKER, &s,
@@ -368,6 +376,9 @@
goto out;
}
+ CU_DEBUG("filter appended, parent_filter->name = %s",
+ parent_filter->name);
+
if (update_filter(conn, parent_filter) == 0) {
cu_statusf(_BROKER, &s,
CMPI_RC_ERR_FAILED,
More information about the Libvirt-cim
mailing list