On 09/30/2011 07:39 PM, Marc-André Lureau wrote:
Do not crash if virStreamFinish is called after error.
==11000== Invalid read of size 4
==11000== at 0x373A8099A0: pthread_mutex_lock (pthread_mutex_lock.c:51)
==11000== by 0x4C7CADE: virMutexLock (threads-pthread.c:85)
==11000== by 0x4D57C31: virNetClientStreamRaiseError (virnetclientstream.c:203)
==11000== by 0x4D385E4: remoteStreamFinish (remote_driver.c:3541)
==11000== by 0x4D182F9: virStreamFinish (libvirt.c:14157)
==11000== by 0x40FDC4: cmdScreenshot (virsh.c:3075)
==11000== by 0x42BA40: vshCommandRun (virsh.c:14922)
==11000== by 0x42ECCA: main (virsh.c:16381)
==11000== Address 0x59b86c0 is 16 bytes inside a block of size 216 free'd
==11000== at 0x4A06928: free (vg_replace_malloc.c:427)
==11000== by 0x4C69E2B: virFree (memory.c:310)
==11000== by 0x4D57B56: virNetClientStreamFree (virnetclientstream.c:184)
==11000== by 0x4D3DB7A: remoteDomainScreenshot (remote_client_bodies.h:1812)
==11000== by 0x4CFD245: virDomainScreenshot (libvirt.c:2903)
==11000== by 0x40FB73: cmdScreenshot (virsh.c:3029)
==11000== by 0x42BA40: vshCommandRun (virsh.c:14922)
==11000== by 0x42ECCA: main (virsh.c:16381)
---
src/rpc/gendispatch.pl | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
Took me a while to read the code, including the generated
src/remote/remote_client_bodies.h before and after the patch, to
convince myself, but this does indeed look like the correct fix.
cmdScreenshot definitely triggers this, but several other commands were
fixed in the process, such as peer2peer migration, storage volume
download, remote console support, and so on.
diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl
index 039d785..b7ac3c8 100755
--- a/src/rpc/gendispatch.pl
+++ b/src/rpc/gendispatch.pl
@@ -1480,6 +1480,8 @@ elsif ($opt_k) {
if ($call->{streamflag} ne "none") {
print " virNetClientRemoveStream(priv->client, netst);\n";
print " virNetClientStreamFree(netst);\n";
+ print " st->driver = NULL;\n";
+ print " st->privateData = NULL;\n";
The second line is not strictly necessary; the first is sufficient to
prevent the crash. But it also doesn't hurt, and mirrors the fact that
both members of st were just set away from NULL earlier on in each of
the same functions that now have the new cleanup.
ACK and pushed as-is.
--
Eric Blake eblake@xxxxxxxxxx +1-801-349-2682
Libvirt virtualization library http://libvirt.org
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list