[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

[Libvir] libvirt.c: avoid a double-free upon do_open failure



With a contrived example using more than 20 (the max permitted by
the testing framework) domains, I got a double-free error:

  ==4821== Invalid free() / delete / delete[]
  ==4821==    at 0x4A0560B: free (vg_replace_malloc.c:233)
  ==4821==    by 0x4167F1: virReleaseConnect (hash.c:717)
  ==4821==    by 0x4168E7: virUnrefConnect (hash.c:746)
  ==4821==    by 0x40FCEB: do_open (libvirt.c:621)
  ==4821==    by 0x40FDF1: virConnectOpenAuth (libvirt.c:682)
  ==4821==    by 0x40D5B1: vshInit (virsh.c:4464)
  ==4821==    by 0x40E67C: main (virsh.c:4985)
  ==4821==  Address 0x4C3BA68 is 0 bytes inside a block of size 60 free'd
  ==4821==    at 0x4A0560B: free (vg_replace_malloc.c:233)
  ==4821==    by 0x40FCB3: do_open (libvirt.c:618)
  ==4821==    by 0x40FDF1: virConnectOpenAuth (libvirt.c:682)
  ==4821==    by 0x40D5B1: vshInit (virsh.c:4464)
  ==4821==    by 0x40E67C: main (virsh.c:4985)
  error: failed to connect to the hypervisor
  ...

here's one way to fix it:

diff --git a/src/libvirt.c b/src/libvirt.c
index defadc1..c19565f 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -615,7 +615,6 @@ do_open (const char *name,
     return ret;

 failed:
-    free (ret->name);
     if (ret->driver) ret->driver->close (ret);
     if (uri) xmlFreeURI(uri);
 	virUnrefConnect(ret);

At first, rather than removing the offending
free, I inserted this line just after it:

    ret->name = NULL;

which avoids leaking ->name even if some driver-specific close function
fails to clean up properly.  But IMHO if such a function doesn't clean
up properly then *it* should be fixed, not all callers.


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]