July 2013 - Devel - libvirt List Archives

[libvirt] [PATCH] Crash of libvirtd by unprivileged user in virConnectListAllInterfaces
by Eric Blake 03 Jul '13

03 Jul '13

From: "Daniel P. Berrange" <berrange(a)redhat.com> On Thu, Jun 27, 2013 at 03:56:42PM +0100, Daniel P. Berrange wrote: > Hi Security Team, > > I've discovered a way for an unprivileged user with a readonly connection > to libvirtd, to crash the daemon. Ok, the final patch for this is issue will be the simpler variant that Eric suggested The embargo can be considered to be lifted on Monday July 1st, at 0900 UTC The following is the GIT change that DV or myself will apply to libvirt GIT master immediately before the 1.1.0 release: >From 177b4165c531a4b3ba7f6ab6aa41dca9ceb0b8cf Mon Sep 17 00:00:00 2001 From: "Daniel P. Berrange" <berrange(a)redhat.com> Date: Fri, 28 Jun 2013 10:48:37 +0100 Subject: [PATCH] CVE-2013-2218: Fix crash listing network interfaces with filters The virConnectListAllInterfaces method has a double-free of the 'struct netcf_if' object when any of the filtering flags cause an interface to be skipped over. For example when running the command 'virsh iface-list --inactive' This is a regression introduced in release 1.0.6 by commit 7ac2c4fe624f30f2c8270116513fa2ddab07631f Author: Guannan Ren <gren(a)redhat.com> Date: Tue May 21 21:29:38 2013 +0800 interface: list all interfaces with flags == 0 Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com> --- Posting as a courtesy FYI for anyone reading this list but who does not have access to the security list and doesn't want to crawl through git. This commit has been included in 1.1.0 and has been applied to all affected stable branches (just v1.0.6-maint). The rule in determining that a CVE was necessary is the "escalation of privilege" test - any time a read-only client can cause a denial-of-service against a more-privileged read-write client (by crashing libvirtd), there is an escalation. src/interface/interface_backend_netcf.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/interface/interface_backend_netcf.c b/src/interface/interface_backend_netcf.c index a995816..9aa673d 100644 --- a/src/interface/interface_backend_netcf.c +++ b/src/interface/interface_backend_netcf.c @@ -412,6 +412,7 @@ netcfConnectListAllInterfaces(virConnectPtr conn, (MATCH(VIR_CONNECT_LIST_INTERFACES_INACTIVE) && (status & NETCF_IFACE_INACTIVE)))) { ncf_if_free(iface); + iface = NULL; continue; } -- 1.8.1.4

2 1

[libvirt] Bug 980449 - resource leak about event
by Wangyufei (A) 02 Jul '13

02 Jul '13

Hi All, I have filed the following bug for Libvirt event issue : Bug 980449<https://bugzilla.redhat.com/show_bug.cgi?id=980449> - resource leak about event Thanks, WangYufei

1 0

[libvirt] [PATCH v6 0/5] libvirt supports Guest Panicked
by Chen Fan 02 Jul '13

02 Jul '13

Changes: v5-v6: Refactor the patches, and fix the incorrect indentation and name. v4-v5: 1. fix the incorrect indentation and explanation (of some reasons). v3-v4: 1. Supports the dumpcore options of the oncrash element in the XML. 2. Move the previous code to processWatchdogEvent(). v2-v3: 1. split into 3 patches v1-v2: 1. fix the incorrect domain state: paused -> crashed, when crash the guest while libvirt isn't running, then restart libvirtd. Chen Fan (5): libvirt: Define domain crash event types qemu: Refactor the processwatchdogEvent code that can make the driver workpool more generalized qemu: Need to use qemuProcessShutdownOrReboot(), so we defined it at qemu_process.h qemu: Implement 'oncrash' events when guest panicked qemu: Implement 'oncrash' coredump events when guest panicked examples/domain-events/events-c/event-test.c | 10 ++ include/libvirt/libvirt.h.in | 16 ++ src/conf/domain_conf.c | 12 +- src/qemu/qemu_domain.h | 11 +- src/qemu/qemu_driver.c | 222 ++++++++++++++++++++++++--- src/qemu/qemu_monitor.c | 14 +- src/qemu/qemu_monitor.h | 4 + src/qemu/qemu_monitor_json.c | 7 + src/qemu/qemu_process.c | 60 +++++++- src/qemu/qemu_process.h | 3 + tools/virsh-domain-monitor.c | 8 + 11 files changed, 328 insertions(+), 39 deletions(-) -- 1.8.1.4

5 14

[libvirt] [PATCHv2] build: don't ship access syms files in tarball
by Eric Blake 02 Jul '13

02 Jul '13

On a mingw VPATH build (such as done by ./autobuild.sh), the tarball created by 'make dist' was including generated files. The VPATH rules were then seeing that the tarball files were up-to-date, and not regenerating files locally, leading to this failure: GEN libvirt.syms cat: libvirt_access.syms: No such file or directory cat: libvirt_access_qemu.syms: No such file or directory cat: libvirt_access_lxc.syms: No such file or directory make: *** [libvirt.syms] Error 1 We already have a category for generated sym files, which are intentionally not part of the tarball; stick the access sym files in that category. Rearrange the declarations a bit to make it harder to repeat the problem in the future, and drop assignments that are now redundant (GENERATED_SYM_FILES are already listed as BUILT_FILES, so we don't have to repeat that for ACCESS_DRIVER_SYM_FILES). * src/Makefile.am (USED_SYM_FILES): Don't include generated files. (GENERATED_SYM_FILES): Access syms files are generated. (libvirt.syms): Include access syms files here. (ACCESS_DRIVER_SYMFILES): Rename... (ACCESS_DRIVER_SYM_FILES): ...for consistency. Signed-off-by: Eric Blake <eblake(a)redhat.com> --- v1 discussion was here: https://www.redhat.com/archives/libvir-list/2013-July/msg00054.html Pushing under the build-breaker rule; now that I'm more confident at why the build was failing, and also pushing to v1.1.0-maint. src/Makefile.am | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/src/Makefile.am b/src/Makefile.am index 285334b..15a2840 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -815,7 +815,7 @@ ACCESS_DRIVER_GENERATED = \ $(srcdir)/access/viraccessapichecklxc.h \ $(srcdir)/access/viraccessapichecklxc.c -ACCESS_DRIVER_SYMFILES = \ +ACCESS_DRIVER_SYM_FILES = \ libvirt_access.syms \ libvirt_access_qemu.syms \ libvirt_access_lxc.syms @@ -927,8 +927,14 @@ libvirt_driver_la_CFLAGS = \ libvirt_driver_la_LIBADD = \ $(GNUTLS_LIBS) $(CURL_LIBS) $(DLOPEN_LIBS) +# All .syms files should be placed in exactly one of these three lists, +# depending on whether they are stored in git and/or used in the build. SYM_FILES = $(USED_SYM_FILES) USED_SYM_FILES = $(srcdir)/libvirt_private.syms +GENERATED_SYM_FILES = \ + $(ACCESS_DRIVER_SYM_FILES) \ + libvirt.syms libvirt.def libvirt_qemu.def libvirt_lxc.def \ + $(NULL) if WITH_TEST noinst_LTLIBRARIES += libvirt_driver_test.la @@ -1474,9 +1480,8 @@ EXTRA_DIST += $(ACCESS_DRIVER_POLKIT_SOURCES) endif -USED_SYM_FILES += $(ACCESS_DRIVER_SYMFILES) -BUILT_SOURCES += $(ACCESS_DRIVER_GENERATED) $(ACCESS_DRIVER_SYMFILES) -CLEANFILES += $(ACCESS_DRIVER_GENERATED) $(ACCESS_DRIVER_SYMFILES) +BUILT_SOURCES += $(ACCESS_DRIVER_GENERATED) +CLEANFILES += $(ACCESS_DRIVER_GENERATED) libvirt_access.syms: $(srcdir)/rpc/gendispatch.pl \ $(REMOTE_PROTOCOL) Makefile.am @@ -1713,12 +1718,9 @@ EXTRA_DIST += \ $(SYM_FILES) \ $(NULL) -GENERATED_SYM_FILES = \ - libvirt.syms libvirt.def libvirt_qemu.def libvirt_lxc.def - BUILT_SOURCES += $(GENERATED_SYM_FILES) -libvirt.syms: libvirt_public.syms $(USED_SYM_FILES) \ +libvirt.syms: libvirt_public.syms $(USED_SYM_FILES) $(ACCESS_DRIVER_SYM_FILES) \ $(top_builddir)/config.status $(AM_V_GEN)rm -f $@-tmp $@ ; \ printf '# WARNING: generated from the following:\n# $^\n\n' >$@-tmp && \ @@ -1726,7 +1728,7 @@ libvirt.syms: libvirt_public.syms $(USED_SYM_FILES) \ printf '\n\n# Private symbols\n\n' >>$@-tmp && \ printf 'LIBVIRT_PRIVATE_$(VERSION) {\n\n' >>$@-tmp && \ printf 'global:\n\n' >>$@-tmp && \ - cat $(USED_SYM_FILES) >>$@-tmp && \ + cat $(USED_SYM_FILES) $(ACCESS_DRIVER_SYM_FILES) >>$@-tmp && \ printf '\n\nlocal:\n*;\n\n};' >>$@-tmp && \ chmod a-w $@-tmp && \ mv $@-tmp libvirt.syms -- 1.8.1.4

1 0

[libvirt] [PATCH] build: avoid build failure without polkit
by Eric Blake 02 Jul '13

02 Jul '13

rpmbuild of mingw-libvirt.spec failed with: GEN libvirt.syms GEN libvirt_qemu.def cat: libvirt_access.syms: No such file or directory cat: libvirt_access_qemu.syms: No such file or directory cat: libvirt_access_lxc.syms: No such file or directory I traced this to unconditionally trying to use the ACL .syms files, even when polkit isn't in use. build: avoid build failure without polkit * src/Makefile.am (USED_SYM_FILES): Mark access driver symbols according to use. --- Even though this fixes a build-breaker (./autobuild.sh), I'm reluctant to push this without review. In particular, I'm worried that I may need a v2 that further conditionalizes whether libvirt_access_qemu.syms is sometimes omitted based on rpm arguments, even if libvirt_access.syms is present. src/Makefile.am | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Makefile.am b/src/Makefile.am index 4cf999d..042bcba 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -1469,12 +1469,13 @@ $(ACCESS_DRIVER_POLKIT_POLICY): $(srcdir)/access/viraccessperm.h \ CLEANFILES += $(ACCESS_DRIVER_POLKIT_POLICY) BUILT_SOURCES += $(ACCESS_DRIVER_POLKIT_POLICY) +USED_SYM_FILES += $(ACCESS_DRIVER_SYMFILES) else EXTRA_DIST += $(ACCESS_DRIVER_POLKIT_SOURCES) +SYM_FILES += $(ACCESS_DRIVER_SYMFILES) endif -USED_SYM_FILES += $(ACCESS_DRIVER_SYMFILES) BUILT_SOURCES += $(ACCESS_DRIVER_GENERATED) $(ACCESS_DRIVER_SYMFILES) CLEANFILES += $(ACCESS_DRIVER_GENERATED) $(ACCESS_DRIVER_SYMFILES) -- 1.8.1.4

2 5

[libvirt] [PATCH] qemu: Restored original console alias
by Michal Privoznik 02 Jul '13

02 Jul '13

Because of some crazy backward compatibility, console device is in some cases just an alias to a serial device. This means, in the process of generating XML description of a domain, all the interesting info is taken from corresponding serial device, if that's the case. Including the device alias. That means, we produce: <console type='pty' tty='/dev/pts/20'> ... <alias name='serial0'/> </console> (notice the assigned alias) Maybe this is okay, maybe its wrong either. Anyway, later, when libvirtd restarts, and we parse the state XML file, we read the wrong alias back. Hence, the internal representation is different to the state it was in prior the libvirtd restart. --- src/qemu/qemu_domain.c | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 8d79066..96d88ec 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -804,6 +804,34 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, dev->data.chr->source.data.nix.listen = true; } + /* For some really crazy back compat in virDomainDefFormatInternal we must + * restore the original console alias. For hvm domains, we are formatting + * a dummy console device (based on a serial device which it refers to) + * instead of the original one. That means the device aliases in memory + * and in the formatted XML are not in sync. While in memory we still have + * 'consoleN', in the formatted XML we have 'serialN'. */ + if (dev->type == VIR_DOMAIN_DEVICE_CHR && + dev->data.chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE && + dev->data.chr->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL && + STREQ(def->os.type, "hvm")) { + int id; + char *alias = dev->data.chr->info.alias; + const char *serial_alias = "serial"; + + if (alias && STRPREFIX(alias, serial_alias)) { + alias += strlen(serial_alias); + + if (virStrToLong_i(alias, NULL, 10, &id) < 0) + goto cleanup; + + VIR_FREE(dev->data.chr->info.alias); + if (virAsprintf(&dev->data.chr->info.alias, "console%d", id) < 0) { + virReportOOMError(); + goto cleanup; + } + } + } + ret = 0; cleanup: -- 1.8.1.5

2 2

[libvirt] [PATCH] domain_conf: Include the correct console alias
by Michal Privoznik 02 Jul '13

02 Jul '13

For some crazy backward compatibility, a console can by just an alias to a serial device. This is detected in the XML formating function which takes the values to format from corresponding serial device. Including the device alias. This results in wrong alias being written into the XML definition: <console type='pty' tty='/dev/pts/5'> ... <alias name='serial0'/> </console> While holding the correct alias still in the memory, it doesn't matter. However, it starts to matter as soon as libvirtd is restarted and the (incorrect) alias is read from status file. --- src/conf/domain_conf.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 011de71..61de836 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -16417,6 +16417,7 @@ virDomainDefFormatInternal(virDomainDefPtr def, memcpy(&console, def->serials[n], sizeof(console)); console.deviceType = VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE; console.targetType = VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL; + memcpy(&console.info, &def->consoles[n]->info, sizeof(console.info)); } else { memcpy(&console, def->consoles[n], sizeof(console)); } @@ -16427,11 +16428,20 @@ virDomainDefFormatInternal(virDomainDefPtr def, def->nconsoles == 0 && def->nserials > 0) { virDomainChrDef console; + char *alias = NULL; memcpy(&console, def->serials[n], sizeof(console)); console.deviceType = VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE; console.targetType = VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL; - if (virDomainChrDefFormat(buf, &console, flags) < 0) + if (console.info.alias) { + if (VIR_STRDUP(alias, "console0") < 0) + goto error; + console.info.alias = alias; + } + if (virDomainChrDefFormat(buf, &console, flags) < 0) { + VIR_FREE(alias); goto error; + } + VIR_FREE(alias); } for (n = 0; n < def->nchannels; n++) -- 1.8.1.5

2 7

[libvirt] [PATCH] build: work around mingw header pollution
by Eric Blake 02 Jul '13

02 Jul '13

On Fedora 18, when cross-compiling to mingw with the mingw*-dbus packages installed, compilation fails with: CC libvirt_net_rpc_server_la-virnetserver.lo In file included from /usr/i686-w64-mingw32/sys-root/mingw/include/dbus-1.0/dbus/dbus-connection.h:32:0, from /usr/i686-w64-mingw32/sys-root/mingw/include/dbus-1.0/dbus/dbus-bus.h:30, from /usr/i686-w64-mingw32/sys-root/mingw/include/dbus-1.0/dbus/dbus.h:31, from ../../src/util/virdbus.h:26, from ../../src/rpc/virnetserver.c:39: /usr/i686-w64-mingw32/sys-root/mingw/include/dbus-1.0/dbus/dbus-message.h:74:58: error: expected ';', ',' or ')' before 'struct' I have reported this as a bug against two packages: - mingw-headers, for polluting the namespace https://bugzilla.redhat.com/show_bug.cgi?id=980270 - dbus, for not dealing with the pollution https://bugzilla.redhat.com/show_bug.cgi?id=980278 At least dbus has agreed that a future version of dbus headers will do s/interface/iface/, regardless of what happens in mingw. But it is also easy to workaround in libvirt in the meantime, without having to wait for either mingw or dbus to upgrade. * src/util/virdbus.h (includes): Undo mingw's pollution so that dbus doesn't fail. Signed-off-by: Eric Blake <eblake(a)redhat.com> --- Pushing under the build-breaker rule. src/util/virdbus.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/util/virdbus.h b/src/util/virdbus.h index a51dba7..302d40d 100644 --- a/src/util/virdbus.h +++ b/src/util/virdbus.h @@ -1,7 +1,7 @@ /* * virdbus.h: helper for using DBus * - * Copyright (C) 2012 Red Hat, Inc. + * Copyright (C) 2012, 2013 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -23,6 +23,7 @@ # define __VIR_DBUS_H__ # ifdef WITH_DBUS +# undef interface /* Work around namespace pollution in mingw's rpc.h */ # include <dbus/dbus.h> # else # define DBusConnection void -- 1.8.1.4

1 0

[libvirt] [PATCH] qemuNodeDeviceDetachFlags: Avoid use of uninitialized variables
by Michal Privoznik 02 Jul '13

02 Jul '13

After abf75aea24 the compiler screams: qemu/qemu_driver.c: In function 'qemuNodeDeviceDetachFlags': qemu/qemu_driver.c:10693:9: error: 'domain' may be used uninitialized in this function [-Werror=maybe-uninitialized] pci = virPCIDeviceNew(domain, bus, slot, function); ^ qemu/qemu_driver.c:10693:9: error: 'bus' may be used uninitialized in this function [-Werror=maybe-uninitialized] qemu/qemu_driver.c:10693:9: error: 'slot' may be used uninitialized in this function [-Werror=maybe-uninitialized] qemu/qemu_driver.c:10693:9: error: 'function' may be used uninitialized in this function [-Werror=maybe-uninitialized] Since the other functions qemuNodeDeviceReAttach and qemuNodeDeviceReset looks exactly the same, I've initialized the variables there as well. However, I am still wondering why those functions don't matter to gcc while the first one does. --- Pushed as build breaker. src/qemu/qemu_driver.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 6a9135a..4856f37 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -10669,7 +10669,7 @@ qemuNodeDeviceDetachFlags(virNodeDevicePtr dev, { virQEMUDriverPtr driver = dev->conn->privateData; virPCIDevicePtr pci = NULL; - unsigned domain, bus, slot, function; + unsigned domain = 0, bus = 0, slot = 0, function = 0; int ret = -1; virNodeDeviceDefPtr def = NULL; char *xml = NULL; @@ -10737,7 +10737,7 @@ qemuNodeDeviceReAttach(virNodeDevicePtr dev) virQEMUDriverPtr driver = dev->conn->privateData; virPCIDevicePtr pci = NULL; virPCIDevicePtr other; - unsigned domain, bus, slot, function; + unsigned domain = 0, bus = 0, slot = 0, function = 0; int ret = -1; virNodeDeviceDefPtr def = NULL; char *xml = NULL; @@ -10799,7 +10799,7 @@ qemuNodeDeviceReset(virNodeDevicePtr dev) { virQEMUDriverPtr driver = dev->conn->privateData; virPCIDevicePtr pci; - unsigned domain, bus, slot, function; + unsigned domain = 0, bus = 0, slot = 0, function = 0; int ret = -1; virNodeDeviceDefPtr def = NULL; char *xml = NULL; -- 1.8.1.5

1 0

[libvirt] [PATCH 0/4] Fix image labeling when saving a guest
by Peter Krempa 02 Jul '13

02 Jul '13

This series cleans up a few places related to selinux labels and fixes issues with selinux when saving a machine with static selinux label and relabeling turned off. Peter Krempa (4): qemu: Improve info message and remove a variable in qemuDomainManagedSave conf: refactor virSecurityLabelDefParseXML security: Introduce method for labeling file descriptors of created files qemu: Always label newly created file on migration (save/managedsave) src/conf/domain_conf.c | 72 ++++++++++++++++++----------------------- src/libvirt_private.syms | 1 + src/qemu/qemu_driver.c | 7 ++-- src/qemu/qemu_migration.c | 4 +-- src/security/security_dac.c | 9 ++++++ src/security/security_driver.h | 4 +++ src/security/security_manager.c | 16 +++++++++ src/security/security_manager.h | 3 ++ src/security/security_nop.c | 1 + src/security/security_selinux.c | 21 ++++++++++++ src/security/security_stack.c | 19 +++++++++++ 11 files changed, 111 insertions(+), 46 deletions(-) -- 1.8.2.1

2 10