]> git.ipfire.org Git - thirdparty/libvirt.git/commit
remoteClientCloseFunc: Don't mangle connection object refcount
authorMichal Privoznik <mprivozn@redhat.com>
Thu, 3 Sep 2015 10:11:53 +0000 (12:11 +0200)
committerMichal Privoznik <mprivozn@redhat.com>
Thu, 3 Sep 2015 15:46:09 +0000 (17:46 +0200)
commite7ebcd5c57396a8fafeb4b0e69c353283a15af88
tree18e5d7e7153515405d6115d14a92b7258daa2373
parent605b12068392d29beb44a8ab7d6ec176d6b05237
remoteClientCloseFunc: Don't mangle connection object refcount

Well, in 8ad126e6 we tried to fix a memory corruption problem.
However, the fix was not as good as it could be. I mean, the
commit has one line more than it should. I've noticed this output
just recently:

  # ./run valgrind --leak-check=full --show-reachable=yes ./tools/virsh domblklist gentoo
  ==17019== Memcheck, a memory error detector
  ==17019== Copyright (C) 2002-2013, and GNU GPL'd, by Julian Seward et al.
  ==17019== Using Valgrind-3.10.1 and LibVEX; rerun with -h for copyright info
  ==17019== Command: /home/zippy/work/libvirt/libvirt.git/tools/.libs/virsh domblklist gentoo
  ==17019==
  Target     Source
  ------------------------------------------------
  fda        /var/lib/libvirt/images/fd.img
  vda        /var/lib/libvirt/images/gentoo.qcow2
  hdc        /home/zippy/tmp/install-amd64-minimal-20150402.iso

  ==17019== Thread 2:
  ==17019== Invalid read of size 4
  ==17019==    at 0x4EFF5B4: virObjectUnref (virobject.c:258)
  ==17019==    by 0x5038CFF: remoteClientCloseFunc (remote_driver.c:552)
  ==17019==    by 0x5069D57: virNetClientCloseLocked (virnetclient.c:685)
  ==17019==    by 0x506C848: virNetClientIncomingEvent (virnetclient.c:1852)
  ==17019==    by 0x5082136: virNetSocketEventHandle (virnetsocket.c:1913)
  ==17019==    by 0x4ECD64E: virEventPollDispatchHandles (vireventpoll.c:509)
  ==17019==    by 0x4ECDE02: virEventPollRunOnce (vireventpoll.c:658)
  ==17019==    by 0x4ECBF00: virEventRunDefaultImpl (virevent.c:308)
  ==17019==    by 0x130386: vshEventLoop (vsh.c:1864)
  ==17019==    by 0x4F1EB07: virThreadHelper (virthread.c:206)
  ==17019==    by 0xA8462D3: start_thread (in /lib64/libpthread-2.20.so)
  ==17019==    by 0xAB441FC: clone (in /lib64/libc-2.20.so)
  ==17019==  Address 0x139023f4 is 4 bytes inside a block of size 240 free'd
  ==17019==    at 0x4C2B1F0: free (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
  ==17019==    by 0x4EA8949: virFree (viralloc.c:582)
  ==17019==    by 0x4EFF6D0: virObjectUnref (virobject.c:273)
  ==17019==    by 0x4FE74D6: virConnectClose (libvirt.c:1390)
  ==17019==    by 0x13342A: virshDeinit (virsh.c:406)
  ==17019==    by 0x134A37: main (virsh.c:950)

The problem is, when registering remoteClientCloseFunc(), it's
conn->closeCallback which is ref'd. But in the function itself
it's conn->closeCallback->conn what is unref'd. This is causing
imbalance in reference counting. Moreover, there's no need for
the remote driver to increase/decrease conn refcount since it's
not used anywhere. It's just merely passed to client registered
callback. And for that purpose it's correctly ref'd in
virConnectRegisterCloseCallback() and then unref'd in
virConnectUnregisterCloseCallback().

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
(cherry picked from commit e68930077034f786e219bdb015f8880dbc5a246f)
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
src/remote/remote_driver.c