[libvirt] [PATCH] libxl: Fix possible invalid read

[Martin Kletzander]

On Thu, Oct 24, 2013 at 10:57:37AM +0100, Daniel P. Berrange wrote:
> On Thu, Oct 24, 2013 at 10:50:02AM +0100, Martin Kletzander wrote:
> > According to the following valgrind output, there seems to be a
> > invalid limit for the iterator (captured on Fedora 19):
> > 
> > ==3945== Invalid read of size 1
> > ==3945==    at 0x1E1FA410: libxlVmStart (libxl_driver.c:475)
> > ==3945==    by 0x1E1FAD9A: libxlDomainCreateWithFlags (libxl_driver.c:2633)
> > ==3945==    by 0x5187D46: virDomainCreate (libvirt.c:9439)
> > ==3945==    by 0x13BAA6: remoteDispatchDomainCreateHelper (remote_dispatch.h:2910)
> > ==3945==    by 0x51DE5B9: virNetServerProgramDispatch (virnetserverprogram.c:435)
> > ==3945==    by 0x51D93E7: virNetServerHandleJob (virnetserver.c:165)
> > ==3945==    by 0x50F5BF4: virThreadPoolWorker (virthreadpool.c:144)
> > ==3945==    by 0x50F5670: virThreadHelper (virthreadpthread.c:161)
> > ==3945==    by 0x8046C52: start_thread (pthread_create.c:308)
> > ==3945==    by 0x8758E1C: clone (clone.S:113)
> > ==3945==  Address 0x23424d81 is 0 bytes after a block of size 1 alloc'd
> > ==3945==    at 0x4A08121: calloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
> > ==3945==    by 0x50B1F8C: virAllocN (viralloc.c:189)
> > ==3945==    by 0x1E1FA3CA: libxlVmStart (libxl_driver.c:468)
> > ==3945==    by 0x1E1FAD9A: libxlDomainCreateWithFlags (libxl_driver.c:2633)
> > ==3945==    by 0x5187D46: virDomainCreate (libvirt.c:9439)
> > ==3945==    by 0x13BAA6: remoteDispatchDomainCreateHelper (remote_dispatch.h:2910)
> > ==3945==    by 0x51DE5B9: virNetServerProgramDispatch (virnetserverprogram.c:435)
> > ==3945==    by 0x51D93E7: virNetServerHandleJob (virnetserver.c:165)
> > ==3945==    by 0x50F5BF4: virThreadPoolWorker (virthreadpool.c:144)
> > ==3945==    by 0x50F5670: virThreadHelper (virthreadpthread.c:161)
> > ==3945==    by 0x8046C52: start_thread (pthread_create.c:308)
> > ==3945==    by 0x8758E1C: clone (clone.S:113)
> > ==3945==
> > 
> > Related: https://bugzilla.redhat.com/show_bug.cgi?id=1013045
> > Signed-off-by: Martin Kletzander <mkletzan at redhat.com>
> >
> > diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> > index e2a6d44..4928695 100644
> > --- a/src/libxl/libxl_driver.c
> > +++ b/src/libxl/libxl_driver.c
> > @@ -454,6 +454,7 @@ libxlDomainSetVcpuAffinities(libxlDriverPrivatePtr driver, virDomainObjPtr vm)
> >      size_t cpumaplen;
> >      int vcpu;
> >      size_t i;
> > +    size_t limit;
> >      int ret = -1;
> > 
> >      if (libxlDoNodeGetInfo(driver, &nodeinfo) < 0)
> > @@ -470,7 +471,8 @@ libxlDomainSetVcpuAffinities(libxlDriverPrivatePtr driver, virDomainObjPtr vm)
> > 
> >          cpumask = (uint8_t*) def->cputune.vcpupin[vcpu]->cpumask;
> > 
> > -        for (i = 0; i < VIR_DOMAIN_CPUMASK_LEN; ++i) {
> > +        limit = MIN(VIR_DOMAIN_CPUMASK_LEN, cpumaplen);
> > +        for (i = 0; i < limit; ++i) {
> >              if (cpumask[i])
> >                  VIR_USE_CPU(cpumap, i);
> >          }
> 
> Hmm, what is strange about that bug is that the crash is in free() wrt
> memory corruption, but valgrind only reports bogus reads. Such a crash
> in free() usually implies a bogus write. The VIR_USE_CPU macro does
> do such a write though, so the patch looks sane - valgrind must have
> just not seen it
> 

It points to another problem in libxl which I don't have set up to use
(and have nowhere to try it), but I can hardly suspect that issue
would corrupt libvirt's memory.

> ACK
> 

Thanks, pushed.

Martin

> Daniel
> -- 
> |: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
> |: http://libvirt.org              -o-             http://virt-manager.org :|
> |: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
> |: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20131024/031eb796/attachment-0001.sig>