[libvirt] [PATCH v4 1/2] daemon: Fix domain name leak in error path

Domain name duplicated in make_nonnull_domain, but not freed when virTypedParamsSerialize return negative. --- daemon/remote.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index 1610fea..a8c21fd 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -1066,8 +1066,10 @@ remoteRelayDomainEventTunable(virConnectPtr conn, if (virTypedParamsSerialize(params, nparams, (virTypedParameterRemotePtr *) &data.params.params_val, &data.params.params_len, - VIR_TYPED_PARAM_STRING_OKAY) < 0) + VIR_TYPED_PARAM_STRING_OKAY) < 0) { + VIR_FREE(data.dom.name); return -1; + } remoteDispatchObjectEventSend(callback->client, remoteProgram, REMOTE_PROC_DOMAIN_EVENT_CALLBACK_TUNABLE, @@ -1206,8 +1208,10 @@ remoteRelayDomainEventJobCompleted(virConnectPtr conn, if (virTypedParamsSerialize(params, nparams, (virTypedParameterRemotePtr *) &data.params.params_val, &data.params.params_len, - VIR_TYPED_PARAM_STRING_OKAY) < 0) + VIR_TYPED_PARAM_STRING_OKAY) < 0) { + VIR_FREE(data.dom.name); return -1; + } remoteDispatchObjectEventSend(callback->client, remoteProgram, REMOTE_PROC_DOMAIN_EVENT_CALLBACK_JOB_COMPLETED, -- 2.8.3

Free the dst is unnecessary if the VIR_STRDUP fails, and therefore we need to remove the error label. --- daemon/remote.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index a8c21fd..2709c3c 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -648,7 +648,7 @@ remoteRelayDomainEventBlockJob(virConnectPtr conn, /* build return data */ memset(&data, 0, sizeof(data)); if (VIR_STRDUP(data.path, path) < 0) - goto error; + return -1; data.type = type; data.status = status; make_nonnull_domain(&data.dom, dom); @@ -667,9 +667,6 @@ remoteRelayDomainEventBlockJob(virConnectPtr conn, } return 0; - error: - VIR_FREE(data.path); - return -1; } @@ -1025,7 +1022,7 @@ remoteRelayDomainEventBlockJob2(virConnectPtr conn, memset(&data, 0, sizeof(data)); data.callbackID = callback->callbackID; if (VIR_STRDUP(data.dst, dst) < 0) - goto error; + return -1; data.type = type; data.status = status; make_nonnull_domain(&data.dom, dom); @@ -1035,9 +1032,6 @@ remoteRelayDomainEventBlockJob2(virConnectPtr conn, (xdrproc_t)xdr_remote_domain_event_block_job_2_msg, &data); return 0; - error: - VIR_FREE(data.dst); - return -1; } -- 2.8.3

On Mon, Apr 24, 2017 at 12:05:25PM +0800, Wang King wrote:
Free the dst is unnecessary if the VIR_STRDUP fails, and therefore we need to remove the error label. --- daemon/remote.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-)
Nice cleanup; I'll tweak the commit message a tiny bit and push it in a while.

On Mon, Apr 24, 2017 at 12:05:24PM +0800, Wang King wrote:
Domain name duplicated in make_nonnull_domain, but not freed when virTypedParamsSerialize return negative. --- daemon/remote.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
ACK, I just tweak the commit message and push it in a while.
diff --git a/daemon/remote.c b/daemon/remote.c index 1610fea..a8c21fd 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -1066,8 +1066,10 @@ remoteRelayDomainEventTunable(virConnectPtr conn, if (virTypedParamsSerialize(params, nparams, (virTypedParameterRemotePtr *) &data.params.params_val, &data.params.params_len, - VIR_TYPED_PARAM_STRING_OKAY) < 0) + VIR_TYPED_PARAM_STRING_OKAY) < 0) { + VIR_FREE(data.dom.name); return -1; + }
remoteDispatchObjectEventSend(callback->client, remoteProgram, REMOTE_PROC_DOMAIN_EVENT_CALLBACK_TUNABLE, @@ -1206,8 +1208,10 @@ remoteRelayDomainEventJobCompleted(virConnectPtr conn, if (virTypedParamsSerialize(params, nparams, (virTypedParameterRemotePtr *) &data.params.params_val, &data.params.params_len, - VIR_TYPED_PARAM_STRING_OKAY) < 0) + VIR_TYPED_PARAM_STRING_OKAY) < 0) { + VIR_FREE(data.dom.name); return -1; + }
remoteDispatchObjectEventSend(callback->client, remoteProgram, REMOTE_PROC_DOMAIN_EVENT_CALLBACK_JOB_COMPLETED, -- 2.8.3
participants (2)
-
Martin Kletzander
-
Wang King