]> git.ipfire.org Git - thirdparty/libvirt.git/commit
remote_driver: Don't leak opaque pointer passed to 'virStreamEventAddCallback'
authorPeter Krempa <pkrempa@redhat.com>
Thu, 14 May 2026 15:19:55 +0000 (17:19 +0200)
committerPeter Krempa <pkrempa@redhat.com>
Tue, 19 May 2026 07:42:35 +0000 (09:42 +0200)
commit4e61d97ae1cc4d991716abbfa5e827e41aabea53
treebe0e861f90b878b7b68b05c2ade496ac0d3592b9
parent4817d1003bb9bb070c48ade9b6f699c5b1e93dfa
remote_driver: Don't leak opaque pointer passed to 'virStreamEventAddCallback'

In the implementation of the virStream events in the remote driver, the
freeing of the passed opaque data for the callback was guarded by:

  if (!cbdata->cb && cbdata->ff)
     (cbdata->ff)(cbdata->opaque);

thus if the 'cb' is passed to 'virStreamEventAddCallback' the private
data would never be freed once 'virStreamEventRemoveCallback' is called.

The issue can be reproduced both in client applications which would
register the stream callback and also in the libvirt daemons, when
modular daemons are used. The virStream remote driver client code is in
such deployments also used to tunnel requests from the hypervisor daemon
(virtqemud) to sub-daemons (virstoraged).

In those cases the leak is amplified as the stream event callback is
internally used to do the tunnelling and the daemon dispatch code stores
a reference to the 'virNetClient' object associated with the connection.
As this causes the last reference on the virNetClient object to be still
active, the corresponding connection to the storage daemon isn't closed
either, leaking a FD both in virtqemud and virtstoraged.

Internally the data is stored in 'struct remoteStreamCallbackData' which
is defined only in 'remote_driver.c' and there's no code which would
update the 'cb' field, thus the leak can't be avoided.

Remove the check for 'cb'. My assumption is that this was supposed to
mimic the 'dispatching' field  in 'util/fdstream.c' or similar logic in
other dispatch functions.

The patch also adds debug statements which I've used to trace this.

The leak of a connection in 'virtqemud' has the following backtrace
under valgrind (note that the pointer is considered reachable and thus
this isn't visible in default config):

==3678343== 136 bytes in 1 blocks are still reachable in loss record 2,680 of 2,964
==3678343==    at 0x48FC6CD: calloc (vg_replace_malloc.c:1616)
==3678343==    by 0x4ED8A91: g_malloc0 (in /usr/lib64/libglib-2.0.so.0.8600.5)
==3678343==    by 0x5019CA2: g_type_class_get (in /usr/lib64/libgobject-2.0.so.0.8600.5)
==3678343==    by 0x5001011: g_object_new_with_properties (in /usr/lib64/libgobject-2.0.so.0.8600.5)
==3678343==    by 0x5001EA0: g_object_new (in /usr/lib64/libgobject-2.0.so.0.8600.5)
==3678343==    by 0x499E653: virObjectNew (virobject.c:252)
==3678343==    by 0x499EA0F: virObjectLockableNew (virobject.c:274)
==3678343==    by 0x4A84651: virNetClientStreamNew (virnetclientstream.c:144)
==3678343==    by 0x4AE82AE: remoteStorageVolUpload (remote_client_bodies.h:8242)
==3678343==    by 0x4BED13D: virStorageVolUpload (libvirt-storage.c:1809)
==3678343==    by 0x4043539: remoteDispatchStorageVolUpload (remote_daemon_dispatch_stubs.h:20248)
==3678343==    by 0x4043539: remoteDispatchStorageVolUploadHelper (remote_daemon_dispatch_stubs.h:20218)
==3678343==    by 0x4A89A2C: virNetServerProgramDispatchCall (virnetserverprogram.c:423)
==3678343==    by 0x4A89A2C: virNetServerProgramDispatch (virnetserverprogram.c:299)

Resolves: https://redhat.atlassian.net/browse/RHEL-170773
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
src/remote/remote_driver.c