[Libvirt-cim] [PATCH 5/8] Avoid NULL dereferences in various providers
Chip Vincent
cvincent at linux.vnet.ibm.com
Tue Nov 15 14:04:18 UTC 2011
+1
On 11/03/2011 01:48 PM, Eduardo Lima (Etrunko) wrote:
> From: Eduardo Lima (Etrunko)<eblima at br.ibm.com>
>
> As revealed by Coverity scan report:
>
> https://bugzilla.redhat.com/show_bug.cgi?id=728245#c8
> https://bugzilla.redhat.com/attachment.cgi?id=530435
>
> Error: FORWARD_NULL:
> Virt_DevicePool.c:999: var_compare_op: Comparing "netnames" to null implies that
> "netnames" might be null.
> Virt_DevicePool.c:1019: var_deref_op: Dereferencing null variable "netnames".
>
> Error: FORWARD_NULL:
> Virt_ComputerSystemIndication.c:259: var_compare_op: Comparing "ind" to null
> implies that "ind" might be null.
> Virt_ComputerSystemIndication.c:267: var_deref_op: Dereferencing null variable
> "ind".
>
> Error: FORWARD_NULL:
> Virt_ResourcePoolConfigurationService.c:759: var_compare_op: Comparing "op" to
> null implies that "op" might be
> null.
> Virt_ResourcePoolConfigurationService.c:779: var_deref_op: Dereferencing null
> variable "op".
>
> Error: FORWARD_NULL:
> Virt_ResourcePoolConfigurationService.c:1028: var_compare_op: Comparing "res" to
> null implies that "res" might be
> null.
> Virt_ResourcePoolConfigurationService.c:1035: var_deref_op: Dereferencing null
> variable "res".
>
> Error: FORWARD_NULL:
> Virt_VSMigrationService.c:766: var_compare_op: Comparing "ref" to null implies
> that "ref" might be null.
> Virt_VSMigrationService.c:796: var_deref_op: Dereferencing null variable "ref".
>
> Error: FORWARD_NULL:
> Virt_VirtualSystemManagementService.c:1274: var_compare_op: Comparing "op" to
> null implies that "op" might be
> null.
> Virt_VirtualSystemManagementService.c:1292: var_deref_op: Dereferencing null
> variable "op".
>
> Error: FORWARD_NULL:
> Virt_VirtualSystemSnapshotService.c:353: var_compare_op: Comparing "ctx" to null
> implies that "ctx" might be null.
> Virt_VirtualSystemSnapshotService.c:376: var_deref_model: Passing null variable
> "ctx" to function "snap_job_free",
> which dereferences it.
> Virt_VirtualSystemSnapshotService.c:66: deref_parm: Directly dereferencing
> parameter "ctx".
>
> Error: REVERSE_INULL:
> Virt_ResourcePoolConfigurationService.c:173: deref_ptr: Directly dereferencing
> pointer "path_list".
> Virt_ResourcePoolConfigurationService.c:174: check_after_deref: Dereferencing
> "path_list" before a null check.
>
> Signed-off-by: Eduardo Lima (Etrunko)<eblima at br.ibm.com>
> ---
> src/Virt_ComputerSystemIndication.c | 1 +
> src/Virt_DevicePool.c | 8 +++++---
> src/Virt_ResourcePoolConfigurationService.c | 6 +++---
> src/Virt_VSMigrationService.c | 9 +++++++--
> src/Virt_VirtualSystemManagementService.c | 2 +-
> src/Virt_VirtualSystemSnapshotService.c | 3 +++
> 6 files changed, 20 insertions(+), 9 deletions(-)
>
> diff --git a/src/Virt_ComputerSystemIndication.c b/src/Virt_ComputerSystemIndication.c
> index c057a0c..337f20a 100644
> --- a/src/Virt_ComputerSystemIndication.c
> +++ b/src/Virt_ComputerSystemIndication.c
> @@ -262,6 +262,7 @@ static bool _do_indication(const CMPIBroker *broker,
> prefix,
> ind_type_name);
> ret = false;
> + goto out;
> }
>
> ind_op = CMGetObjectPath(ind,&s);
> diff --git a/src/Virt_DevicePool.c b/src/Virt_DevicePool.c
> index 7112a9d..a41a378 100644
> --- a/src/Virt_DevicePool.c
> +++ b/src/Virt_DevicePool.c
> @@ -1015,9 +1015,11 @@ static CMPIStatus netpool_instance(virConnectPtr conn,
> }
>
> out:
> - for (i = 0; i< nets; i++)
> - free(netnames[i]);
> - free(netnames);
> + if (netnames != NULL) {
> + for (i = 0; i< nets; i++)
> + free(netnames[i]);
> + free(netnames);
> + }
>
> return s;
> }
> diff --git a/src/Virt_ResourcePoolConfigurationService.c b/src/Virt_ResourcePoolConfigurationService.c
> index 3908c9c..7e76032 100644
> --- a/src/Virt_ResourcePoolConfigurationService.c
> +++ b/src/Virt_ResourcePoolConfigurationService.c
> @@ -171,7 +171,7 @@ static char *get_dev_paths(CMPIInstance *inst,
> return "Unable to get DevicePaths array count";
>
> *path_list = calloc(ct, sizeof(char *));
> - if (path_list == NULL)
> + if (*path_list == NULL)
> return "Failed to alloc space for device paths";
>
> *count = ct;
> @@ -775,7 +775,7 @@ static const char *rasd_to_res(CMPIInstance *inst,
> msg = "This function does not support this resource type";
>
> out:
> - if (msg)
> + if (msg&& op)
> CU_DEBUG("rasd_to_res(%s): %s", CLASSNAME(op), msg);
>
> return msg;
> @@ -1025,7 +1025,7 @@ static CMPIStatus delete_resource_in_pool(CMPIMethodMI *self,
> goto out;
>
> res = CMGetObjectPath(resource,&s);
> - if ((res == NULL)&& (s.rc != CMPI_RC_OK)) {
> + if ((res == NULL) || (s.rc != CMPI_RC_OK)) {
> cu_statusf(_BROKER,&s,
> CMPI_RC_ERR_FAILED,
> "Unable to get ObjectPath of Resource instance");
> diff --git a/src/Virt_VSMigrationService.c b/src/Virt_VSMigrationService.c
> index ee79a96..414feda 100644
> --- a/src/Virt_VSMigrationService.c
> +++ b/src/Virt_VSMigrationService.c
> @@ -761,10 +761,15 @@ static bool raise_indication(const CMPIContext *context,
>
> /* This is a workaround for Pegasus, it loses its objectpath by
> CMGetObjectPath. So set it back. */
> - inst->ft->setObjectPath((CMPIInstance *)inst, ref);
> + if (ref != NULL)
> + inst->ft->setObjectPath((CMPIInstance *)inst, ref);
>
> if ((ref == NULL) || (s.rc != CMPI_RC_OK)) {
> CU_DEBUG("Failed to get job reference");
> + cu_statusf(_BROKER,&s,
> + CMPI_RC_ERR_FAILED,
> + "Failed to get job reference");
> + goto out;
> } else {
> s = get_host_system_properties(&host,
> &ccname,
> @@ -798,7 +803,7 @@ static bool raise_indication(const CMPIContext *context,
> s = stdi_raise_indication(_BROKER, context, type, ns, ind);
>
> free(type);
> -
> + out:
> return s.rc == CMPI_RC_OK;
> }
>
> diff --git a/src/Virt_VirtualSystemManagementService.c b/src/Virt_VirtualSystemManagementService.c
> index 5edc65f..fa1e266 100644
> --- a/src/Virt_VirtualSystemManagementService.c
> +++ b/src/Virt_VirtualSystemManagementService.c
> @@ -1526,7 +1526,7 @@ static const char *rasd_to_vdev(CMPIInstance *inst,
> else
> msg = _sysvirt_rasd_to_vdev(inst, dev, type, ns);
> out:
> - if (msg)
> + if (msg&& op)
> CU_DEBUG("rasd_to_vdev(%s): %s", CLASSNAME(op), msg);
>
> return msg;
> diff --git a/src/Virt_VirtualSystemSnapshotService.c b/src/Virt_VirtualSystemSnapshotService.c
> index e7789cb..898fa57 100644
> --- a/src/Virt_VirtualSystemSnapshotService.c
> +++ b/src/Virt_VirtualSystemSnapshotService.c
> @@ -63,6 +63,9 @@ struct snap_context {
>
> static void snap_job_free(struct snap_context *ctx)
> {
> + if (ctx == NULL)
> + return;
> +
> free(ctx->domain);
> free(ctx->save_path);
> free(ctx->ref_ns);
--
Chip Vincent
Open Virtualization
IBM Linux Technology Center
cvincent at linux.vnet.ibm.com
More information about the Libvirt-cim
mailing list