[PATCH v1 0/1] rpc: fix virNetClient->error leak on teardown
Hi all, this is my first contribution to libvirt upstream. I am with Cyberus Technology, where I work on cloud-hypervisor and its integration with OpenStack and libvirt. The following fixes a leak in virNetClient teardown. virNetClientMarkClose() may save the current error in client->error (via virSaveLastError()). We usually free that in virNetClientCloseLocked(), but we can also end up disposing the client without going through that cleanup. One path where this happens is: virConnectDispose() -> remoteConnectClose() -> doRemoteClose() -> virNetClientClose(priv->client) -> g_clear_pointer(&priv->client, virObjectUnref) In that case, client->error can stay allocated until finalization, and ASan reports a leak. Fix is to also clear client->error in virNetClientDispose() as a final fallback. I included an ASan trace below. Thanks, Leander --- Line numbers in this trace come from our downstream libvirt fork and may differ from upstream, but the call path/function names are the same. [ 276.247727] virtchd[4030]: ==4030==ERROR: LeakSanitizer: detected memory leaks Direct leak of 80 byte(s) in 1 object(s) allocated from: #0 0x7fcd6009e110 in calloc (libasan.so.8+0xfc110) #1 0x7fcd5dcb95bd in g_malloc0 (libglib-2.0.so.0+0x6d5bd) #2 0x7fcd5eb15f31 in virSaveLastError ../src/util/virerror.c:413 #3 0x7fcd5ee6c76c in virNetClientMarkClose ../src/rpc/virnetclient.c:776 #4 0x7fcd5ee6f782 in virNetClientCloseInternal ../src/rpc/virnetclient.c:837 #5 0x7fcd5f0150e4 in doRemoteClose ../src/remote/remote_driver.c:1414 #6 0x7fcd5f0150e4 in remoteConnectClose ../src/remote/remote_driver.c:1442 #7 0x7fcd5f1a90ef in virConnectDispose ../src/datatypes.c:174 #8 0x7fcd5ea58ebb in vir_object_finalize ../src/util/virobject.c:323 #9 0x7fcd5dc05c52 in g_object_unref (libgobject-2.0.so.0+0x1ec52) #10 0x7fcd5ea59fc7 in virObjectUnref ../src/util/virobject.c:377 #11 0x7fcd5ea59fc7 in virObjectUnref ../src/util/virobject.c:370 #12 0x7fcd5ef3a92f in glib_autoptr_clear_virConnect ../src/datatypes.h:548 #13 0x7fcd5ef3a92f in glib_autoptr_cleanup_virConnect ../src/datatypes.h:548 #14 0x7fcd5ef3a92f in virDomainInterfaceDeleteDevice ../src/hypervisor/domain_interface.c:419 #15 0x7fcd594a0cd3 in virCHProcessStopOrKill ../src/ch/ch_process.c:1308 #16 0x7fcd594a8bfa in virCHProcessStop ../src/ch/ch_process.c:1363 #17 0x7fcd5945d8c7 in chDomainDestroyFlags ../src/ch/ch_driver.c:959 #18 0x7fcd5f0f69c0 in virDomainDestroy ../src/libvirt-domain.c:514 #19 0x55c25777376a in remoteDispatchDomainDestroy src/remote/remote_daemon_dispatch_stubs.h:5485 #20 0x55c25777376a in remoteDispatchDomainDestroyHelper src/remote/remote_daemon_dispatch_stubs.h:5464 #21 0x7fcd5ee7c42c in virNetServerProgramDispatchCall ../src/rpc/virnetserverprogram.c:423 #22 0x7fcd5ee7c42c in virNetServerProgramDispatch ../src/rpc/virnetserverprogram.c:299 #23 0x7fcd5ee97c6c in virNetServerProcessMsg ../src/rpc/virnetserver.c:135 #24 0x7fcd5ee98b43 in virNetServerHandleJob ../src/rpc/virnetserver.c:155 #25 0x7fcd5eadbce2 in virThreadPoolWorker ../src/util/virthreadpool.c:164 #26 0x7fcd5eada3c9 in virThreadHelper ../src/util/virthread.c:256 #27 0x7fcd60004cd5 in asan_thread_start(void*) (libasan.so.8+0x62cd5) #28 0x7fcd5d200979 in start_thread (libc.so.6+0x9a979) #29 0x7fcd5d288d2b in __GI___clone3 (libc.so.6+0x122d2b) Objects leaked above: 0x507000064400 (80 bytes) SUMMARY: AddressSanitizer: 80 byte(s) leaked in 1 allocation(s). Leander Kohler (1): rpc: free saved close error in virNetClientDispose src/rpc/virnetclient.c | 2 ++ 1 file changed, 2 insertions(+) -- 2.51.2
virNetClientMarkClose() may cache the current error in client->error via virSaveLastError() when a client is marked for close. That error is normally released in virNetClientCloseLocked(), but some teardown paths can dispose the client object without reaching that cleanup. In that case, client->error remains allocated and ASan reports a leak. Free client->error in virNetClientDispose() as a final cleanup fallback. This was observed during virtchd shutdown in test_disk_is_locked, with the leak originating from: virDomainInterfaceDeleteDevice() -> remoteConnectClose() -> virNetClientMarkClose() -> virSaveLastError() On-behalf-of: SAP leander.kohler@sap.com Signed-off-by: Leander Kohler <leander.kohler@cyberus-technology.de> --- src/rpc/virnetclient.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c index ee729d5e62..4838227f8d 100644 --- a/src/rpc/virnetclient.c +++ b/src/rpc/virnetclient.c @@ -757,6 +757,8 @@ void virNetClientDispose(void *obj) virObjectUnref(client->sasl); #endif + g_clear_pointer(&client->error, virFreeError); + virNetMessageClear(&client->msg); } -- 2.51.2
On 2/27/26 13:39, Leander Kohler wrote:
virNetClientMarkClose() may cache the current error in client->error via virSaveLastError() when a client is marked for close.
That error is normally released in virNetClientCloseLocked(), but some teardown paths can dispose the client object without reaching that cleanup. In that case, client->error remains allocated and ASan reports a leak.
Free client->error in virNetClientDispose() as a final cleanup fallback.
This was observed during virtchd shutdown in test_disk_is_locked, with the leak originating from: virDomainInterfaceDeleteDevice() -> remoteConnectClose() -> virNetClientMarkClose() -> virSaveLastError()
On-behalf-of: SAP leander.kohler@sap.com Signed-off-by: Leander Kohler <leander.kohler@cyberus-technology.de> --- src/rpc/virnetclient.c | 2 ++ 1 file changed, 2 insertions(+)
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> and merged. Congratulations on your first libvirt contribution! Michal
participants (2)
-
Leander Kohler -
Michal Prívozník