[PATCH] qemu: fix memory leak on some paths

From: lu zhipeng <luzhipeng@cestc.cn> virTypedParamsAddString may return -1 but alloc params, so free it. Signed-off-by: lu zhipeng <luzhipeng@cestc.cn> --- src/qemu/qemu_driver.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 94b70872d4..c4501cd705 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4473,11 +4473,13 @@ qemuDomainPinVcpuLive(virDomainObj *vm, goto cleanup; event = virDomainEventTunableNewFromObj(vm, eventParams, eventNparams); - + eventNparams = 0; ret = 0; cleanup: virObjectEventStateQueue(driver->domainEventState, event); + if (eventNparams) + virTypedParamsFree(eventParams, eventNparams); return ret; } @@ -4682,6 +4684,7 @@ qemuDomainPinEmulator(virDomainPtr dom, goto endjob; event = virDomainEventTunableNewFromDom(dom, eventParams, eventNparams); + eventNparams = 0; } if (persistentDef) { @@ -4700,6 +4703,8 @@ qemuDomainPinEmulator(virDomainPtr dom, cleanup: virObjectEventStateQueue(driver->domainEventState, event); virDomainObjEndAPI(&vm); + if (eventNparams) + virTypedParamsFree(eventParams, eventNparams); return ret; } @@ -5079,6 +5084,7 @@ qemuDomainPinIOThread(virDomainPtr dom, goto endjob; event = virDomainEventTunableNewFromDom(dom, eventParams, eventNparams); + eventNparams = 0; } if (persistentDef) { @@ -5106,6 +5112,8 @@ qemuDomainPinIOThread(virDomainPtr dom, cleanup: virObjectEventStateQueue(driver->domainEventState, event); virDomainObjEndAPI(&vm); + if (eventNparams) + virTypedParamsFree(eventParams, eventNparams); return ret; } -- 2.31.1

On Thu, Sep 22, 2022 at 21:00:38 +0800, luzhipeng wrote:
From: lu zhipeng <luzhipeng@cestc.cn>
virTypedParamsAddString may return -1 but alloc params, so free it.
Signed-off-by: lu zhipeng <luzhipeng@cestc.cn> --- src/qemu/qemu_driver.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 94b70872d4..c4501cd705 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4473,11 +4473,13 @@ qemuDomainPinVcpuLive(virDomainObj *vm, goto cleanup;
event = virDomainEventTunableNewFromObj(vm, eventParams, eventNparams);
This call consumes eventParams which should not be used afterwards. Thus instead of
- + eventNparams = 0;
we should set eventParams = NULL here...
ret = 0;
cleanup: virObjectEventStateQueue(driver->domainEventState, event);
... and call virTypedParamsFree unconditionally.
+ if (eventNparams) + virTypedParamsFree(eventParams, eventNparams); return ret; }
Alternatively, virDomainEventTunableNew could be changed to get virTypedParameterPtr *params and set ¶ms = NULL itself. Jirka

On Thu, Sep 22, 2022 at 04:26:06PM +0200, Jiri Denemark wrote:
On Thu, Sep 22, 2022 at 21:00:38 +0800, luzhipeng wrote:
From: lu zhipeng <luzhipeng@cestc.cn>
virTypedParamsAddString may return -1 but alloc params, so free it.
Signed-off-by: lu zhipeng <luzhipeng@cestc.cn> --- src/qemu/qemu_driver.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 94b70872d4..c4501cd705 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4473,11 +4473,13 @@ qemuDomainPinVcpuLive(virDomainObj *vm, goto cleanup;
event = virDomainEventTunableNewFromObj(vm, eventParams, eventNparams);
This call consumes eventParams which should not be used afterwards. Thus instead of
- + eventNparams = 0;
we should set eventParams = NULL here...
ret = 0;
cleanup: virObjectEventStateQueue(driver->domainEventState, event);
... and call virTypedParamsFree unconditionally.
+ if (eventNparams) + virTypedParamsFree(eventParams, eventNparams); return ret; }
Alternatively, virDomainEventTunableNew could be changed to get virTypedParameterPtr *params and set ¶ms = NULL itself.
That's what I wanted to suggest since it actually consumes the params as it would make it more future-proof and easier to use.
Jirka
participants (3)
-
Jiri Denemark
-
luzhipeng
-
Martin Kletzander