[libvirt] [PATCH] libxl: avoid crashing if calling `virsh numatune' on inactive domain

by, in libxlDomainGetNumaParameters(), calling libxl_bitmap_init() as soon as possible, which avoids getting to 'cleanup:', where libxl_bitmap_dispose() happens, without having initialized the nodemap, and hence crashing after some invalid free()-s: # ./daemon/libvirtd -v *** Error in `/home/xen/libvirt.git/daemon/.libs/lt-libvirtd': munmap_chunk(): invalid pointer: 0x00007fdd42592666 *** ======= Backtrace: ========= /lib64/libc.so.6(+0x7bbe7)[0x7fdd3f767be7] /lib64/libxenlight.so.4.3(libxl_bitmap_dispose+0xd)[0x7fdd2c88c045] /home/xen/libvirt.git/daemon/.libs/../../src/.libs/libvirt_driver_libxl.so(+0x12d26)[0x7fdd2caccd26] /home/xen/libvirt.git/src/.libs/libvirt.so.0(virDomainGetNumaParameters+0x15c)[0x7fdd4247898c] /home/xen/libvirt.git/daemon/.libs/lt-libvirtd(+0x1d9a2)[0x7fdd42ecc9a2] /home/xen/libvirt.git/src/.libs/libvirt.so.0(virNetServerProgramDispatch+0x3da)[0x7fdd424e9eaa] /home/xen/libvirt.git/src/.libs/libvirt.so.0(+0x1a6f38)[0x7fdd424e3f38] /home/xen/libvirt.git/src/.libs/libvirt.so.0(+0xa81e5)[0x7fdd423e51e5] /home/xen/libvirt.git/src/.libs/libvirt.so.0(+0xa783e)[0x7fdd423e483e] /lib64/libpthread.so.0(+0x7c53)[0x7fdd3febbc53] /lib64/libc.so.6(clone+0x6d)[0x7fdd3f7e1dbd] Signed-off-by: Dario Faggili <dario.faggioli@citrix.com> Cc: Jim Fehlig <jfehlig@suse.com> Cc: Ian Jackson <Ian.Jackson@eu.citrix.com> --- src/libxl/libxl_driver.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 29aa6c7..d91744f 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -3958,6 +3958,8 @@ libxlDomainGetNumaParameters(virDomainPtr dom, * the filtering on behalf of older clients that can't parse it. */ flags &= ~VIR_TYPED_PARAM_STRING_OKAY; + libxl_bitmap_init(&nodemap); + if (!(vm = libxlDomObjFromDomain(dom))) goto cleanup; @@ -3972,8 +3974,6 @@ libxlDomainGetNumaParameters(virDomainPtr dom, priv = vm->privateData; - libxl_bitmap_init(&nodemap); - if ((*nparams) == 0) { *nparams = LIBXL_NUMA_NPARAM; ret = 0;

Dario Faggioli wrote:
by, in libxlDomainGetNumaParameters(), calling libxl_bitmap_init() as soon as possible, which avoids getting to 'cleanup:', where libxl_bitmap_dispose() happens, without having initialized the nodemap, and hence crashing after some invalid free()-s:
Yikes! ACK to the fix. I've pushed it. Regards, Jim
# ./daemon/libvirtd -v *** Error in `/home/xen/libvirt.git/daemon/.libs/lt-libvirtd': munmap_chunk(): invalid pointer: 0x00007fdd42592666 *** ======= Backtrace: ========= /lib64/libc.so.6(+0x7bbe7)[0x7fdd3f767be7] /lib64/libxenlight.so.4.3(libxl_bitmap_dispose+0xd)[0x7fdd2c88c045] /home/xen/libvirt.git/daemon/.libs/../../src/.libs/libvirt_driver_libxl.so(+0x12d26)[0x7fdd2caccd26] /home/xen/libvirt.git/src/.libs/libvirt.so.0(virDomainGetNumaParameters+0x15c)[0x7fdd4247898c] /home/xen/libvirt.git/daemon/.libs/lt-libvirtd(+0x1d9a2)[0x7fdd42ecc9a2] /home/xen/libvirt.git/src/.libs/libvirt.so.0(virNetServerProgramDispatch+0x3da)[0x7fdd424e9eaa] /home/xen/libvirt.git/src/.libs/libvirt.so.0(+0x1a6f38)[0x7fdd424e3f38] /home/xen/libvirt.git/src/.libs/libvirt.so.0(+0xa81e5)[0x7fdd423e51e5] /home/xen/libvirt.git/src/.libs/libvirt.so.0(+0xa783e)[0x7fdd423e483e] /lib64/libpthread.so.0(+0x7c53)[0x7fdd3febbc53] /lib64/libc.so.6(clone+0x6d)[0x7fdd3f7e1dbd]
Signed-off-by: Dario Faggili <dario.faggioli@citrix.com> Cc: Jim Fehlig <jfehlig@suse.com> Cc: Ian Jackson <Ian.Jackson@eu.citrix.com> --- src/libxl/libxl_driver.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 29aa6c7..d91744f 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -3958,6 +3958,8 @@ libxlDomainGetNumaParameters(virDomainPtr dom, * the filtering on behalf of older clients that can't parse it. */ flags &= ~VIR_TYPED_PARAM_STRING_OKAY;
+ libxl_bitmap_init(&nodemap); + if (!(vm = libxlDomObjFromDomain(dom))) goto cleanup;
@@ -3972,8 +3974,6 @@ libxlDomainGetNumaParameters(virDomainPtr dom,
priv = vm->privateData;
- libxl_bitmap_init(&nodemap); - if ((*nparams) == 0) { *nparams = LIBXL_NUMA_NPARAM; ret = 0;

On 12/20/2013 11:36 AM, Jim Fehlig wrote:
Dario Faggioli wrote:
by, in libxlDomainGetNumaParameters(), calling libxl_bitmap_init() as soon as possible, which avoids getting to 'cleanup:', where libxl_bitmap_dispose() happens, without having initialized the nodemap, and hence crashing after some invalid free()-s:
Yikes! ACK to the fix. I've pushed it.
This has been assigned CVE-6457; we'll get it tagged in libvirt.git and make sure it is backported to relevant branches once I've got more time (may be in 2014). -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On 12/23/2013 11:02 PM, Eric Blake wrote:
On 12/20/2013 11:36 AM, Jim Fehlig wrote:
Dario Faggioli wrote:
by, in libxlDomainGetNumaParameters(), calling libxl_bitmap_init() as soon as possible, which avoids getting to 'cleanup:', where libxl_bitmap_dispose() happens, without having initialized the nodemap, and hence crashing after some invalid free()-s:
Yikes! ACK to the fix. I've pushed it.
This has been assigned CVE-6457; we'll get it tagged in libvirt.git and make sure it is backported to relevant branches once I've got more time (may be in 2014).
Typo, I meant CVE-2013-6457 -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On Tue, Dec 24, 2013 at 12:02 AM, Eric Blake <eblake@redhat.com> wrote:
On 12/20/2013 11:36 AM, Jim Fehlig wrote:
Dario Faggioli wrote:
by, in libxlDomainGetNumaParameters(), calling libxl_bitmap_init() as soon as possible, which avoids getting to 'cleanup:', where libxl_bitmap_dispose() happens, without having initialized the nodemap, and hence crashing after some invalid free()-s:
Yikes! ACK to the fix. I've pushed it.
This has been assigned CVE-6457; we'll get it tagged in libvirt.git and make sure it is backported to relevant branches once I've got more time (may be in 2014).
I'll help you out and get started on this. Family is in town around the holidays so no promises I'll get them all done if its not too trivial. -- Doug Goldstein

On Sat, Dec 28, 2013 at 3:18 PM, Doug Goldstein <cardoe@gentoo.org> wrote:
On Tue, Dec 24, 2013 at 12:02 AM, Eric Blake <eblake@redhat.com> wrote:
On 12/20/2013 11:36 AM, Jim Fehlig wrote:
Dario Faggioli wrote:
by, in libxlDomainGetNumaParameters(), calling libxl_bitmap_init() as soon as possible, which avoids getting to 'cleanup:', where libxl_bitmap_dispose() happens, without having initialized the nodemap, and hence crashing after some invalid free()-s:
Yikes! ACK to the fix. I've pushed it.
This has been assigned CVE-6457; we'll get it tagged in libvirt.git and make sure it is backported to relevant branches once I've got more time (may be in 2014).
I'll help you out and get started on this. Family is in town around the holidays so no promises I'll get them all done if its not too trivial.
-- Doug Goldstein
The fix has been back ported to: v1.1.1-maint v1.1.2-maint v1.1.3-maint v1.1.4-maint v1.2.0-maint This should cover all affected versions per Jim's analysis. Let me know if anything further needs to be done. -- Doug Goldstein
participants (4)
-
Dario Faggioli
-
Doug Goldstein
-
Eric Blake
-
Jim Fehlig