[libvirt] [PATCH] 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 | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/daemon/remote.c b/daemon/remote.c index 1610fea..edee981 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, -- 2.8.3

On 04/13/2017 06:44 AM, Wang King wrote:
domain name duplicated in make_nonnull_domain, but not freed when virTypedParamsSerialize return negative. --- daemon/remote.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/daemon/remote.c b/daemon/remote.c index 1610fea..edee981 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; + }
While true this does "fix" this instance, it also doesn't seem to be the only instance [1] of this and isn't handled via "normal" error path using virObjectUnref(dom); [2] [1] example of another instance w/ similar code/leak/issue See also remoteRelayDomainEventJobCompleted I'll assume this one borrowed from remoteRelayDomainEventTunable considering which came first. [2] example of another instance w/ similar code, but no leak See also remoteDispatchConnectGetAllDomainStats FWIW: The sequence I used to search on is: make_nonnull_domain virTypedParamsSerialize While looking through the code during review, I noticed a couple of other possible cleanups. So, if you're feeling adventurous, there's also two instances where a VIR_STRDUP error is followed by a goto error where all the error label does is VIR_FREE the address that failed the VIR_STRDUP and there's no other way to get to the error label... See: remoteRelayDomainEventBlockJob2 remoteRelayDomainEventBlockJob It would seem those two could just have a return -1 instead to follow other similar such error paths: remoteRelayDomainEventDeviceRemovalFailed remoteRelayDomainEventDeviceAdded remoteRelayDomainEventDeviceRemoved remoteRelayDomainEventTrayChange John
remoteDispatchObjectEventSend(callback->client, remoteProgram, REMOTE_PROC_DOMAIN_EVENT_CALLBACK_TUNABLE,
participants (2)
-
John Ferlan
-
Wang King