remote: avoid null dereference on error

Clang found three instances of uninitialized use of nparams in
the cleanup path.  Unfortunately, one is a false positive: clang
couldn't see that ret->params.params_val is guaranteed to be
NULL unless allocated within a function, and that nparams is
guaranteed to be assigned prior to the allocation; hoisting the
assignment to nparams to be earlier in the function shuts up
that false positive.  But two of the reports also happened to
highlight a real bug - the error path can dereference NULL.

Regression introduced in commit 158ba873.

* daemon/remote.c (remoteDispatchDomainGetMemoryParameters)
(remoteDispatchDomainGetBlkioParameters): Don't clear fields if
array was not allocated.
(remoteDispatchDomainGetSchedulerParameters): Initialize nparams
earlier.
This commit is contained in:
Eric Blake 2011-05-03 11:24:23 -06:00
parent d0a8f99c75
commit 85cb292681

View File

@ -886,7 +886,8 @@ remoteDispatchDomainGetSchedulerParameters(struct qemud_server *server ATTRIBUTE
{ {
virDomainPtr dom = NULL; virDomainPtr dom = NULL;
virSchedParameterPtr params = NULL; virSchedParameterPtr params = NULL;
int i, nparams; int i;
int nparams = args->nparams;
int rv = -1; int rv = -1;
if (!conn) { if (!conn) {
@ -894,8 +895,6 @@ remoteDispatchDomainGetSchedulerParameters(struct qemud_server *server ATTRIBUTE
goto cleanup; goto cleanup;
} }
nparams = args->nparams;
if (nparams > REMOTE_DOMAIN_SCHEDULER_PARAMETERS_MAX) { if (nparams > REMOTE_DOMAIN_SCHEDULER_PARAMETERS_MAX) {
virNetError(VIR_ERR_INTERNAL_ERROR, "%s", _("nparams too large")); virNetError(VIR_ERR_INTERNAL_ERROR, "%s", _("nparams too large"));
goto cleanup; goto cleanup;
@ -2970,7 +2969,8 @@ remoteDispatchDomainGetMemoryParameters(struct qemud_server *server
{ {
virDomainPtr dom = NULL; virDomainPtr dom = NULL;
virMemoryParameterPtr params = NULL; virMemoryParameterPtr params = NULL;
int i, nparams; int i;
int nparams = args->nparams;
unsigned int flags; unsigned int flags;
int rv = -1; int rv = -1;
@ -2979,7 +2979,6 @@ remoteDispatchDomainGetMemoryParameters(struct qemud_server *server
goto cleanup; goto cleanup;
} }
nparams = args->nparams;
flags = args->flags; flags = args->flags;
if (nparams > REMOTE_DOMAIN_MEMORY_PARAMETERS_MAX) { if (nparams > REMOTE_DOMAIN_MEMORY_PARAMETERS_MAX) {
@ -3060,9 +3059,11 @@ success:
cleanup: cleanup:
if (rv < 0) { if (rv < 0) {
remoteDispatchError(rerr); remoteDispatchError(rerr);
for (i = 0; i < nparams; i++) if (ret->params.params_val) {
VIR_FREE(ret->params.params_val[i].field); for (i = 0; i < nparams; i++)
VIR_FREE(ret->params.params_val); VIR_FREE(ret->params.params_val[i].field);
VIR_FREE(ret->params.params_val);
}
} }
if (dom) if (dom)
virDomainFree(dom); virDomainFree(dom);
@ -3186,7 +3187,8 @@ remoteDispatchDomainGetBlkioParameters(struct qemud_server *server
{ {
virDomainPtr dom = NULL; virDomainPtr dom = NULL;
virBlkioParameterPtr params = NULL; virBlkioParameterPtr params = NULL;
int i, nparams; int i;
int nparams = args->nparams;
unsigned int flags; unsigned int flags;
int rv = -1; int rv = -1;
@ -3195,7 +3197,6 @@ remoteDispatchDomainGetBlkioParameters(struct qemud_server *server
goto cleanup; goto cleanup;
} }
nparams = args->nparams;
flags = args->flags; flags = args->flags;
if (nparams > REMOTE_DOMAIN_BLKIO_PARAMETERS_MAX) { if (nparams > REMOTE_DOMAIN_BLKIO_PARAMETERS_MAX) {
@ -3276,9 +3277,11 @@ success:
cleanup: cleanup:
if (rv < 0) { if (rv < 0) {
remoteDispatchError(rerr); remoteDispatchError(rerr);
for (i = 0; i < nparams; i++) if (ret->params.params_val) {
VIR_FREE(ret->params.params_val[i].field); for (i = 0; i < nparams; i++)
VIR_FREE(ret->params.params_val); VIR_FREE(ret->params.params_val[i].field);
VIR_FREE(ret->params.params_val);
}
} }
VIR_FREE(params); VIR_FREE(params);
if (dom) if (dom)