[libvirt] [PATCH] libxl: Fix possible invalid read
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@redhat.com> --- src/libxl/libxl_driver.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) 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); } -- 1.8.4
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@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 ACK 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 :|
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@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 :|
participants (2)
-
Daniel P. Berrange -
Martin Kletzander