[PATCH] RPCS: enumerateInstance(Name)s and getInstance with wrong hypervisor segfaults
# HG changeset patch # User Heidi Eckhart <heidieck@linux.vnet.ibm.com> # Date 1201864846 -3600 # Node ID 6b1bf8231d82f51f25ac3d954b63962bfc7484c9 # Parent cb0e8b858f4243a7086f627d65a07075b9f31ebc RPCS: enumerateInstance(Name)s and getInstance with wrong hypervisor segfaults wbemein http://localhost/root/virt:CIM_ResourcePoolConfigurationService on a KVM system with no Xen segfaults. The same happens to wbemgi 'http://localhost:5988/root/virt:Xen_ResourcePoolConfigurationService.SystemCreationClassName="Xen_HostSystem",SystemName="localhost.localdomain",CreationClassName="Xen_ResourcePoolConfigurationService",Name="RPCS"' This patch also makes RPCS aware of the supported hypervisor type. Now only instances of the machine's supported hypervisor are returned. This approach is used all over the libvirt-cim providers, but was broken here. Signed-off-by: Heidi Eckhart <heidieck@linux.vnet.ibm.com> diff -r cb0e8b858f42 -r 6b1bf8231d82 src/Virt_ResourcePoolConfigurationService.c --- a/src/Virt_ResourcePoolConfigurationService.c Fri Feb 01 11:17:15 2008 +0100 +++ b/src/Virt_ResourcePoolConfigurationService.c Fri Feb 01 12:20:46 2008 +0100 @@ -100,15 +100,20 @@ CMPIStatus rpcs_instance(const CMPIObjec { CMPIInstance *inst; CMPIInstance *host; - CMPIStatus s; + CMPIStatus s = {CMPI_RC_OK, NULL}; CMPIData prop; + virConnectPtr conn = NULL; + + conn = connect_by_classname(_BROKER, CLASSNAME(reference), &s); + if (conn == NULL) + goto out; s = get_host_cs(broker, reference, &host); if (s.rc != CMPI_RC_OK) goto out; inst = get_typed_instance(broker, - CLASSNAME(reference), + pfx_from_conn(conn), "ResourcePoolConfigurationService", NAMESPACE(reference)); if (inst == NULL) { @@ -145,7 +150,10 @@ CMPIStatus rpcs_instance(const CMPIObjec (CMPIValue *)&prop.value.string, CMPI_string); *_inst = inst; + out: + virConnectClose(conn); + return s; } @@ -155,13 +163,17 @@ static CMPIStatus GetInstance(CMPIInstan const CMPIObjectPath *reference, const char **properties) { - CMPIInstance *inst; + CMPIInstance *inst = NULL; CMPIStatus s; const char *prop = NULL; s = rpcs_instance(reference, &inst, _BROKER); - if (s.rc != CMPI_RC_OK) + if (s.rc != CMPI_RC_OK || inst == NULL) { + cu_statusf(_BROKER, &s, + CMPI_RC_ERR_NOT_FOUND, + "No such instance"); return s; + } prop = cu_compare_ref(reference, inst); if (prop != NULL) { @@ -180,11 +192,11 @@ static CMPIStatus EnumInstanceNames(CMPI const CMPIResult *results, const CMPIObjectPath *reference) { - CMPIInstance *inst; - CMPIStatus s; - - s = rpcs_instance(reference, &inst, _BROKER); - if (s.rc == CMPI_RC_OK) + CMPIInstance *inst = NULL; + CMPIStatus s; + + s = rpcs_instance(reference, &inst, _BROKER); + if (s.rc == CMPI_RC_OK && inst != NULL) cu_return_instance_name(results, inst); return s; @@ -197,11 +209,11 @@ static CMPIStatus EnumInstances(CMPIInst const char **properties) { - CMPIInstance *inst; - CMPIStatus s; - - s = rpcs_instance(reference, &inst, _BROKER); - if (s.rc == CMPI_RC_OK) + CMPIInstance *inst = NULL; + CMPIStatus s; + + s = rpcs_instance(reference, &inst, _BROKER); + if (s.rc == CMPI_RC_OK && inst != NULL) CMReturnInstance(results, inst); return s;
Heidi Eckhart wrote:
# HG changeset patch # User Heidi Eckhart <heidieck@linux.vnet.ibm.com> # Date 1201864846 -3600 # Node ID 6b1bf8231d82f51f25ac3d954b63962bfc7484c9 # Parent cb0e8b858f4243a7086f627d65a07075b9f31ebc RPCS: enumerateInstance(Name)s and getInstance with wrong hypervisor segfaults
wbemein http://localhost/root/virt:CIM_ResourcePoolConfigurationService on a KVM system with no Xen segfaults.
The same happens to wbemgi 'http://localhost:5988/root/virt:Xen_ResourcePoolConfigurationService.SystemCreationClassName="Xen_HostSystem",SystemName="localhost.localdomain",CreationClassName="Xen_ResourcePoolConfigurationService",Name="RPCS"'
This patch also makes RPCS aware of the supported hypervisor type. Now only instances of the machine's supported hypervisor are returned. This approach is used all over the libvirt-cim providers, but was broken here.
Signed-off-by: Heidi Eckhart <heidieck@linux.vnet.ibm.com>
diff -r cb0e8b858f42 -r 6b1bf8231d82 src/Virt_ResourcePoolConfigurationService.c --- a/src/Virt_ResourcePoolConfigurationService.c Fri Feb 01 11:17:15 2008 +0100 +++ b/src/Virt_ResourcePoolConfigurationService.c Fri Feb 01 12:20:46 2008 +0100 @@ -100,15 +100,20 @@ CMPIStatus rpcs_instance(const CMPIObjec { CMPIInstance *inst; CMPIInstance *host; - CMPIStatus s; + CMPIStatus s = {CMPI_RC_OK, NULL}; CMPIData prop; + virConnectPtr conn = NULL; + + conn = connect_by_classname(_BROKER, CLASSNAME(reference), &s); + if (conn == NULL) + goto out;
I would also set the status here. This would mean we don't need to check for a NULL instance everywhere rpcs_instance() is called. Virt_HostedService calls rpcs_instance(), but currently doesn't check for a NULL. So if don't set the status here, HostedService will need to be updated to check for a NULL instance. -- Kaitlin Rupert IBM Linux Technology Center kaitlin@linux.vnet.ibm.com
Kaitlin Rupert wrote:
Heidi Eckhart wrote:
diff -r cb0e8b858f42 -r 6b1bf8231d82 src/Virt_ResourcePoolConfigurationService.c --- a/src/Virt_ResourcePoolConfigurationService.c Fri Feb 01 11:17:15 2008 +0100 +++ b/src/Virt_ResourcePoolConfigurationService.c Fri Feb 01 12:20:46 2008 +0100 @@ -100,15 +100,20 @@ CMPIStatus rpcs_instance(const CMPIObjec { CMPIInstance *inst; CMPIInstance *host; - CMPIStatus s; + CMPIStatus s = {CMPI_RC_OK, NULL}; CMPIData prop; + virConnectPtr conn = NULL; + + conn = connect_by_classname(_BROKER, CLASSNAME(reference), &s); + if (conn == NULL) + goto out;
I would also set the status here. This would mean we don't need to check for a NULL instance everywhere rpcs_instance() is called. Well, this function is used for enumeration and get requests. Setting an error code for conn==NULL would break the CIM model for enumeration request. An enumeration against CIM_RPCS on a Xen system with no KVM, would get the error for KVM and valid instances for Xen. But this KVM error would then cause the CIMOM to skip all returned and valid Xen instances. In the case of a get, setting this error code indeed is required. But the idea behind your comment is good. I will reorganize the functions a bit and will resend the patch.
Virt_HostedService calls rpcs_instance(), but currently doesn't check for a NULL. So if don't set the status here, HostedService will need to be updated to check for a NULL instance.
Thanks for this hint :). I will fix this. -- Regards Heidi Eckhart Software Engineer IBM Linux Technology Center - Open Hypervisor
participants (2)
-
Heidi Eckhart -
Kaitlin Rupert