December 2008 - Devel - libvirt List Archives

Re: [libvirt] PATCH 0/5: connection cloning support (WIP)
by David Lively 18 Dec '08

18 Dec '08

Hi Daniel - When I apply these patches, I'm seeing segfaults on event delivery when just running the existing synchronous examples/domain-events/events-c/event-test.c (using the remote driver). I've added a little debug. Apparently event->name is being NULLed out sometime after the event is put on the queue: DEBUG: remote_internal.c: remoteDomainEventFired (Event fired 0 3 1 1) DEBUG: remote_internal.c: processCallRecv (Do 4 0) DEBUG: remote_internal.c: processCallRecvLen (Got length, now need 64 total (60 more)) DEBUG: remote_internal.c: processCallRecv (Do 64 4) DEBUG: remote_internal.c: processCallAsyncEvent (Encountered an event while waiting for a response) DEBUG: remote_internal.c: get_nonnull_domain (domain.name: dsl) DEBUG: datatypes.c: virGetDomain (New hash entry 0x804c728) DEBUG: domain_event.c: virDomainEventNew (event: 0x804c770 ->name: dsl) DEBUG: libvirt.c: virDomainFree (domain=0x804c728) DEBUG: datatypes.c: virUnrefDomain (unref domain 0x804c728 dsl 1) DEBUG: datatypes.c: virReleaseDomain (release domain 0x804c728 dsl) DEBUG: datatypes.c: virReleaseDomain (unref connection 0x804b040 4) DEBUG: domain_event.c: virDomainEventQueuePush (event: 0x804c770 ->name: dsl) DEBUG: remote_internal.c: processCallRecv (Do 0 0) DEBUG: remote_internal.c: remoteDomainEventQueueFlush () DEBUG: domain_event.c: virDomainEventDispatchDefaultFunc (event: 0x804c770 ->name: (null)) virGetDomain: name is NULL Segmentation fault I'll continue looking into it. But please let me know if you're familiar with the problem ... Thanks, Dave On Tue, 2008-12-16 at 10:24 -0500, David Lively wrote: > Hi Daniels - > Sorry for the delay. Life's been crazy. But now I'm finally looking > into using this series of patches with my Java event implementation > (which I've now redone via the java.nio.Select mechanism; ready to > submit pending resolution of the RPC concurrency issues). I should have > some feedback for you later today ... > > Thanks, > Dave > > On Tue, 2008-12-09 at 12:08 +0000, Daniel P. Berrange wrote: > > This series is a work-in-progress set of patches implementing connection > > cloning. The idea is that if you want to use a connection form multiple > > threads, you could do > > > > virConnectPtr copy = virConnectClone(conn) > > > > and use 'copy' from the other thread. This avoids the problem of having > > to make all the virError handling stuff thread-local whic is the blocker > > for allowing real mutlti-thread access to a single virConnectPtr object. > > > > I believe this cloning support should be sufficient for the Java bindings > > need to use a thread for its event loop. The idea being that if you wanted > > to use an event loop in a background thread, you'd create a cloned object > > for use event loop triggered callbacks like domain events notifications. > > > > I'd still like to do some experiments later making the single virConnectPtr > > fully thread safe, but that'll take a little more time. I'm hoping this > > cloning will address the Java needs right now... > > > > Daniel

2 5

[libvirt] [SECURITY] PATCH: Fix missing read-only access checks (CVE-2008-5086)
by Daniel P. Berrange 18 Dec '08

18 Dec '08

The following methods in libvirt.c are missing a check against the read-only connection flag: virDomainMigrate virDomainMigratePrepare virDomainMigratePerform virDomainMigrateFinish virDomainMigratePrepare2 virDomainMigrateFinish2 virDomainBlockPeek virDomainMemoryPeek virDomainSetAutostart virNetworkSetAutostart virConnectFindStoragePoolSources virStoragePoolSetAutostart If using PolicyKit auth, the default policy will allow any local user to make a read-only connection to the libvirtd daemon without needing authentication. If not using PolicyKit, the default libvirtd.conf configuration settings will allow an unprivileged user to make a read-only connection to the libvirtd daemon without needing authentication. Thus out of the box unprivileged local users may be able to migrate VMs, set or unset the autostart flag for domains, networks & storage pools, and access privileged data in the VM memory, or disks. All TCP remote connections are read-write, and default settings require full authentication, thus remote access is not impacted by this flaw. Administrators can apply a workaround by editting /etc/libvirt/libvirtd.conf to explicitly set 'unix_sock_ro_perms' parameter to '0700'. Restart the libvirtd daemon after making this change. The first vulnerable release was 0.3.2, where the virDomainMigrate API was added for the Xen driver. Other APIs were added in various subsequent releases depending on the hypervisor driver in question. The attached patch has been committed to CVS, and OS distributors are recommended to apply this patch to all existing releases shipped. It was diff'd against current CVS head, and applies against 0.5.1, and is trivially re-diffable for all earlier releases. This flaw has been assigned the identifier CVE-2008-5086 Daniel diff --git a/src/libvirt.c b/src/libvirt.c --- a/src/libvirt.c +++ b/src/libvirt.c @@ -2296,6 +2296,16 @@ virDomainMigrate (virDomainPtr domain, conn = domain->conn; /* Source connection. */ if (!VIR_IS_CONNECT (dconn)) { virLibConnError (conn, VIR_ERR_INVALID_CONN, __FUNCTION__); + return NULL; + } + + if (domain->conn->flags & VIR_CONNECT_RO) { + virLibDomainError(domain, VIR_ERR_OPERATION_DENIED, __FUNCTION__); + return NULL; + } + if (dconn->flags & VIR_CONNECT_RO) { + /* NB, delibrately report error against source object, not dest here */ + virLibDomainError(domain, VIR_ERR_OPERATION_DENIED, __FUNCTION__); return NULL; } @@ -2426,6 +2436,11 @@ virDomainMigratePrepare (virConnectPtr d return -1; } + if (dconn->flags & VIR_CONNECT_RO) { + virLibConnError(dconn, VIR_ERR_OPERATION_DENIED, __FUNCTION__); + return -1; + } + if (dconn->driver->domainMigratePrepare) return dconn->driver->domainMigratePrepare (dconn, cookie, cookielen, uri_in, uri_out, @@ -2457,6 +2472,11 @@ virDomainMigratePerform (virDomainPtr do } conn = domain->conn; + if (domain->conn->flags & VIR_CONNECT_RO) { + virLibDomainError(domain, VIR_ERR_OPERATION_DENIED, __FUNCTION__); + return -1; + } + if (conn->driver->domainMigratePerform) return conn->driver->domainMigratePerform (domain, cookie, cookielen, uri, @@ -2482,6 +2502,11 @@ virDomainMigrateFinish (virConnectPtr dc if (!VIR_IS_CONNECT (dconn)) { virLibConnError (NULL, VIR_ERR_INVALID_CONN, __FUNCTION__); + return NULL; + } + + if (dconn->flags & VIR_CONNECT_RO) { + virLibConnError(dconn, VIR_ERR_OPERATION_DENIED, __FUNCTION__); return NULL; } @@ -2517,6 +2542,11 @@ virDomainMigratePrepare2 (virConnectPtr return -1; } + if (dconn->flags & VIR_CONNECT_RO) { + virLibConnError(dconn, VIR_ERR_OPERATION_DENIED, __FUNCTION__); + return -1; + } + if (dconn->driver->domainMigratePrepare2) return dconn->driver->domainMigratePrepare2 (dconn, cookie, cookielen, uri_in, uri_out, @@ -2547,6 +2577,11 @@ virDomainMigrateFinish2 (virConnectPtr d return NULL; } + if (dconn->flags & VIR_CONNECT_RO) { + virLibConnError(dconn, VIR_ERR_OPERATION_DENIED, __FUNCTION__); + return NULL; + } + if (dconn->driver->domainMigrateFinish2) return dconn->driver->domainMigrateFinish2 (dconn, dname, cookie, cookielen, @@ -2905,6 +2940,11 @@ virDomainBlockPeek (virDomainPtr dom, } conn = dom->conn; + if (dom->conn->flags & VIR_CONNECT_RO) { + virLibDomainError(dom, VIR_ERR_OPERATION_DENIED, __FUNCTION__); + return (-1); + } + if (!path) { virLibDomainError (dom, VIR_ERR_INVALID_ARG, _("path is NULL")); @@ -2980,6 +3020,11 @@ virDomainMemoryPeek (virDomainPtr dom, } conn = dom->conn; + if (dom->conn->flags & VIR_CONNECT_RO) { + virLibDomainError(dom, VIR_ERR_OPERATION_DENIED, __FUNCTION__); + return (-1); + } + /* Flags must be VIR_MEMORY_VIRTUAL at the moment. * * Note on access to physical memory: A VIR_MEMORY_PHYSICAL flag is @@ -3246,6 +3291,11 @@ virDomainSetAutostart(virDomainPtr domai } conn = domain->conn; + + if (domain->conn->flags & VIR_CONNECT_RO) { + virLibDomainError(domain, VIR_ERR_OPERATION_DENIED, __FUNCTION__); + return (-1); + } if (conn->driver->domainSetAutostart) return conn->driver->domainSetAutostart (domain, autostart); @@ -4197,6 +4247,11 @@ virNetworkSetAutostart(virNetworkPtr net return (-1); } + if (network->conn->flags & VIR_CONNECT_RO) { + virLibNetworkError(network, VIR_ERR_OPERATION_DENIED, __FUNCTION__); + return (-1); + } + conn = network->conn; if (conn->networkDriver && conn->networkDriver->networkSetAutostart) @@ -4395,6 +4450,11 @@ virConnectFindStoragePoolSources(virConn return NULL; } + if (conn->flags & VIR_CONNECT_RO) { + virLibConnError(conn, VIR_ERR_OPERATION_DENIED, __FUNCTION__); + return NULL; + } + if (conn->storageDriver && conn->storageDriver->findPoolSources) return conn->storageDriver->findPoolSources(conn, type, srcSpec, flags); @@ -5068,6 +5128,11 @@ virStoragePoolSetAutostart(virStoragePoo return (-1); } + if (pool->conn->flags & VIR_CONNECT_RO) { + virLibStoragePoolError(pool, VIR_ERR_OPERATION_DENIED, __FUNCTION__); + return (-1); + } + conn = pool->conn; if (conn->storageDriver && conn->storageDriver->poolSetAutostart) -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

2 2

[libvirt] [PATCH] don't assume builddir == srcdir
by Guido Günther 18 Dec '08

18 Dec '08

Hi, Makefile.maint assumes this in some places. This doesn't make the world perfect but maybe a bit better? Cheers, -- Guido

3 6

[libvirt] libvirt and 192.168.122.0/24
by H. Peter Anvin 18 Dec '08

18 Dec '08

I'm kind of wondering why libvirt defaults to 192.168.122.0/24 by default. It is a piece of address space which is relatively likely to conflict with address space used in the environment surrounding the machine. Since libvirtd is on by default if installed, this is particularly problematic. I would like to suggest either one of the following as default address space, unless the user has explicitly configured otherwise: 192.0.2.0/24 - reserved as "test and example network" 198.18.0.0/15 - reserved as "benchmark test network" See RFC 3330 and RFC 2544 for the definitions of these networks. Both of them are "should not appear on the public Internet" address blocks, and much less likely than the RFC 1918 address block (10.0.0.0/8, 172.16.0.0/12, and 192.168.0.0/16) to be encountered "in the wild" by a user. -hpa

2 1

[libvirt] [PATCH] [OpenVZ] Segmentation fault
by Anton Protopopov 17 Dec '08

17 Dec '08

2008/12/13 Ivan Vovk <ivovk(a)intermedia.net> > Hello, > > > > after updating to the last official release 0.5.1 my application stopped > working. No errors to the log file though i raised exceptions where it was > possible. > > i checked with virsh xml description used for creating OpenVZ container and > got the following: > > > > virsh # create ovz.xml > Segmentation fault > > Attached patch, fixes that.

2 5

[libvirt] [PATCH] Portability fixes for directory storage backend
by john.levon＠sun.com 17 Dec '08

17 Dec '08

# HG changeset patch # User john.levon(a)sun.com # Date 1229399267 28800 # Node ID 53a18080ea210168c044d7ade8dd8db0881d5c85 # Parent cd2fb266824178c4d63296b587eeedb2360f9f1c Portability fixes for directory storage backend Signed-off-by: John Levon <john.levon(a)sun.com> diff --git a/configure.in b/configure.in --- a/configure.in +++ b/configure.in @@ -75,7 +75,7 @@ AC_CHECK_FUNCS([cfmakeraw regexec uname AC_CHECK_FUNCS([cfmakeraw regexec uname sched_getaffinity getuid getgid]) dnl Availability of various common headers (non-fatal if missing). -AC_CHECK_HEADERS([pwd.h paths.h regex.h sys/syslimits.h sys/utsname.h sys/wait.h winsock2.h sched.h termios.h sys/poll.h]) +AC_CHECK_HEADERS([pwd.h paths.h regex.h sys/syslimits.h sys/utsname.h sys/wait.h winsock2.h sched.h termios.h sys/poll.h endian.h]) dnl Where are the XDR functions? dnl If portablexdr is installed, prefer that. diff --git a/src/storage_backend_fs.c b/src/storage_backend_fs.c --- a/src/storage_backend_fs.c +++ b/src/storage_backend_fs.c @@ -31,10 +31,14 @@ #include <errno.h> #include <fcntl.h> #include <unistd.h> +#include <string.h> + +#ifdef HAVE_ENDIAN_H #include <endian.h> -#include <byteswap.h> -#include <mntent.h> -#include <string.h> +#else +#define __LITTLE_ENDIAN 1234 +#define __BIG_ENDIAN 4321 +#endif #include <libxml/parser.h> #include <libxml/tree.h> @@ -240,6 +244,9 @@ static int virStorageBackendProbeFile(vi } #if WITH_STORAGE_FS + +#include <mntent.h> + struct _virNetfsDiscoverState { const char *host; virStoragePoolSourceList list;

4 4

[libvirt] [PATCH] 4/8 convert virterror to the logging infrastructure
by Daniel Veillard 17 Dec '08

17 Dec '08

Basically having the debug information in one place and the errors in another place is not very helpful, so this plugs the virterror handling into the logging routine. mostly a bit of refactoring, and calling virLogMessage. Just that we pass an extra flag to virLogMessage stating it comes from the error handling level to avoid outputting the error twice on stderr in case where there is no output defined. Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

2 3

[libvirt] [PATCH] let gcc's -Wformat do its job; avoid "make syntax-check" failure
by Jim Meyering 17 Dec '08

17 Dec '08

I first noticed that "make syntax-check" failed. Then I saw that virAsprintf's prototype lacked ATTRIBUTE_FORMAT. This fixes both and updates HACKING. >From 5bae37c505738ed4223625f8e3cc88ad6f4bf68c Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Wed, 17 Dec 2008 08:54:46 +0100 Subject: [PATCH] let gcc's -Wformat do its job; avoid "make syntax-check" failure * src/util.c (virAsprintf): Remove trailing space. * src/util.h (virAsprintf): Use ATTRIBUTE_FORMAT. * HACKING (Printf-style functions): New section. --- HACKING | 16 ++++++++++++++++ src/util.c | 2 +- src/util.h | 3 ++- 3 files changed, 19 insertions(+), 2 deletions(-) diff --git a/HACKING b/HACKING index 3945833..e088da8 100644 --- a/HACKING +++ b/HACKING @@ -247,6 +247,22 @@ are some special reasons why you cannot include these files explicitly. +Printf-style functions +====================== + +Whenever you add a new printf-style function, i.e., one with a format +string argument and following "..." in its prototype, be sure to use +gcc's printf attribute directive in the prototype. For example, here's +the one for virAsprintf, in util.h: + + int virAsprintf(char **strp, const char *fmt, ...) + ATTRIBUTE_FORMAT(printf, 2, 3); + +This makes it so gcc's -Wformat and -Wformat-security options can do +their jobs and cross-check format strings with the number and types +of arguments. + + Libvirt commiters guidelines ============================ diff --git a/src/util.c b/src/util.c index 12097d4..9eda378 100644 --- a/src/util.c +++ b/src/util.c @@ -1158,7 +1158,7 @@ virParseNumber(const char **str) * * like asprintf but makes sure *strp == NULL on failure */ -int +int virAsprintf(char **strp, const char *fmt, ...) { va_list ap; diff --git a/src/util.h b/src/util.h index 3d603dc..0475bd3 100644 --- a/src/util.h +++ b/src/util.h @@ -112,7 +112,8 @@ int virMacAddrCompare (const char *mac1, const char *mac2); void virSkipSpaces(const char **str); int virParseNumber(const char **str); -int virAsprintf(char **strp, const char *fmt, ...); +int virAsprintf(char **strp, const char *fmt, ...) + ATTRIBUTE_FORMAT(printf, 2, 3); #define VIR_MAC_BUFLEN 6 #define VIR_MAC_PREFIX_BUFLEN 3 -- 1.6.1.rc2.316.geb2f0

2 2

[libvirt] [PATCH] Fix _FILE_OFFSET_BITS re-definition
by john.levon＠sun.com 17 Dec '08

17 Dec '08

# HG changeset patch # User john.levon(a)sun.com # Date 1229399267 28800 # Node ID db36391b739c117f5887388f65f31e6a9d2d361b # Parent 020f8b8e9340287a6ab3d869b359e39b905cd0ff Fix _FILE_OFFSET_BITS re-definition Since config.h contains the _FILE_OFFSET_BITS setting (a little dubious in itself), it must be the first header included, otherwise system headers can define _FILE_OFFSET_BITS differently themselves. Signed-off-by: John Levon <john.levon(a)sun.com> diff --git a/src/node_device_hal.c b/src/node_device_hal.c --- a/src/node_device_hal.c +++ b/src/node_device_hal.c @@ -21,10 +21,11 @@ * Author: David F. Lively <dlively(a)virtualiron.com> */ +#include <config.h> + #include <stdio.h> #include <stdlib.h> -#include <config.h> #include <libhal.h> #include "node_device_conf.h"

4 4

[libvirt] [PATHC] 8/8 extra settings in libvirtd.conf
by Daniel Veillard 17 Dec '08

17 Dec '08

Basically documents the 3 new configuration variables, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

2 2