4 years agoqemu: Avoid using stale data in virDomainGetBlockInfo
Jiri Denemark [Fri, 20 Dec 2013 13:50:02 +0000]
qemu: Avoid using stale data in virDomainGetBlockInfo


Generally, every API that is going to begin a job should do that before
fetching data from vm->def. However, qemuDomainGetBlockInfo does not
know whether it will have to start a job or not before checking vm->def.
To avoid using disk alias that might have been freed while we were
waiting for a job, we use its copy. In case the disk was removed in the
meantime, we will fail with "cannot find statistics for device '...'"
error message.

(cherry picked from commit b799259583bd65c0b2f5042e6c3ff19637ade881)

4 years agoqemu: Do not access stale data in virDomainBlockStats
Jiri Denemark [Thu, 19 Dec 2013 21:10:04 +0000]
qemu: Do not access stale data in virDomainBlockStats


When virDomainDetachDeviceFlags is called concurrently to
virDomainBlockStats: libvirtd may crash because qemuDomainBlockStats
finds a disk in vm->def before getting a job on a domain and uses the
disk pointer after getting the job. However, the domain in unlocked
while waiting on a job condition and thus data behind the disk pointer
may disappear. This happens when thread 1 runs
virDomainDetachDeviceFlags and enters monitor to actually remove the
disk. Then another thread starts running virDomainBlockStats, finds the
disk in vm->def, and while it's waiting on the job condition (owned by
the first thread), the first thread finishes the disk removal. When the
second thread gets the job, the memory pointed to be the disk pointer is
already gone.

That said, every API that is going to begin a job should do that before
fetching data from vm->def.

(cherry picked from commit db86da5ca2109e4006c286a09b6c75bfe10676ad)

4 years agolibxl: avoid crashing if calling `virsh numatune' on inactive domain
Dario Faggioli [Fri, 20 Dec 2013 15:29:47 +0000]
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: =========

Signed-off-by: Dario Faggili <>
Cc: Jim Fehlig <>
Cc: Ian Jackson <>
(cherry picked from commit f9ee91d35510ccbc6fc42cef8864b291b2d220f4)


4 years agoFix crash in lxcDomainSetMemoryParameters
Martin Kletzander [Mon, 9 Dec 2013 10:15:12 +0000]
Fix crash in lxcDomainSetMemoryParameters

The function doesn't check whether the request is made for active or
inactive domain.  Thus when the domain is not running it still tries
accessing non-existing cgroups (priv->cgroup, which is NULL).

I re-made the function in order for it to work the same way it's qemu
counterpart does.

 1) Define an LXC domain
 2) Do 'virsh memtune <domain> --hard-limit 133T'

 Thread 6 (Thread 0x7fffec8c0700 (LWP 26826)):
 #0  0x00007ffff70edcc4 in virCgroupPathOfController (group=0x0, controller=3,
     key=0x7ffff75734bd "memory.limit_in_bytes", path=0x7fffec8bf718) at util/vircgroup.c:1764
 #1  0x00007ffff70e9206 in virCgroupSetValueStr (group=0x0, controller=3,
     key=0x7ffff75734bd "memory.limit_in_bytes", value=0x7fffe409f360 "1073741824")
     at util/vircgroup.c:669
 #2  0x00007ffff70e98b4 in virCgroupSetValueU64 (group=0x0, controller=3,
     key=0x7ffff75734bd "memory.limit_in_bytes", value=1073741824) at util/vircgroup.c:740
 #3  0x00007ffff70ee518 in virCgroupSetMemory (group=0x0, kb=1048576) at util/vircgroup.c:1904
 #4  0x00007ffff70ee675 in virCgroupSetMemoryHardLimit (group=0x0, kb=1048576)
     at util/vircgroup.c:1944
 #5  0x00005555557d54c8 in lxcDomainSetMemoryParameters (dom=0x7fffe40cc420,
     params=0x7fffe409f100, nparams=1, flags=0) at lxc/lxc_driver.c:774
 #6  0x00007ffff72c20f9 in virDomainSetMemoryParameters (domain=0x7fffe40cc420,
     params=0x7fffe409f100, nparams=1, flags=0) at libvirt.c:4051
 #7  0x000055555561365f in remoteDispatchDomainSetMemoryParameters (server=0x555555eb7e00,
     client=0x555555ec4b10, msg=0x555555eb94e0, rerr=0x7fffec8bfb70, args=0x7fffe40b8510)
     at remote_dispatch.h:7621
 #8  0x00005555556133fd in remoteDispatchDomainSetMemoryParametersHelper (server=0x555555eb7e00,
     client=0x555555ec4b10, msg=0x555555eb94e0, rerr=0x7fffec8bfb70, args=0x7fffe40b8510,
     ret=0x7fffe40b84f0) at remote_dispatch.h:7591
 #9  0x00007ffff73b293f in virNetServerProgramDispatchCall (prog=0x555555ec3ae0,
     server=0x555555eb7e00, client=0x555555ec4b10, msg=0x555555eb94e0)
     at rpc/virnetserverprogram.c:435
 #10 0x00007ffff73b207f in virNetServerProgramDispatch (prog=0x555555ec3ae0,
     server=0x555555eb7e00, client=0x555555ec4b10, msg=0x555555eb94e0)
     at rpc/virnetserverprogram.c:305
 #11 0x00007ffff73a4d2c in virNetServerProcessMsg (srv=0x555555eb7e00, client=0x555555ec4b10,
     prog=0x555555ec3ae0, msg=0x555555eb94e0) at rpc/virnetserver.c:165
 #12 0x00007ffff73a4e8d in virNetServerHandleJob (jobOpaque=0x555555ec3e30, opaque=0x555555eb7e00)
     at rpc/virnetserver.c:186
 #13 0x00007ffff7187f3f in virThreadPoolWorker (opaque=0x555555eb7ac0) at util/virthreadpool.c:144
 #14 0x00007ffff718733a in virThreadHelper (data=0x555555eb7890) at util/virthreadpthread.c:161
 #15 0x00007ffff468ed89 in start_thread (arg=0x7fffec8c0700) at pthread_create.c:308
 #16 0x00007ffff3da26bd in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:113

Signed-off-by: Martin Kletzander <>
(cherry picked from commit 9faf3f2950aed1643ab7564afcb4c693c77f71b5)

4 years agoCVE-2013-6436: fix crash in lxcDomainGetMemoryParameters
Martin Kletzander [Mon, 9 Dec 2013 10:15:11 +0000]
CVE-2013-6436: fix crash in lxcDomainGetMemoryParameters

The function doesn't check whether the request is made for active or
inactive domain.  Thus when the domain is not running it still tries
accessing non-existing cgroups (priv->cgroup, which is NULL).

I re-made the function in order for it to work the same way it's qemu
counterpart does.

 1) Define an LXC domain
 2) Do 'virsh memtune <domain>'

 Thread 6 (Thread 0x7fffec8c0700 (LWP 13387)):
 #0  0x00007ffff70edcc4 in virCgroupPathOfController (group=0x0, controller=3,
     key=0x7ffff75734bd "memory.limit_in_bytes", path=0x7fffec8bf750) at util/vircgroup.c:1764
 #1  0x00007ffff70e958c in virCgroupGetValueStr (group=0x0, controller=3,
     key=0x7ffff75734bd "memory.limit_in_bytes", value=0x7fffec8bf7c0) at util/vircgroup.c:705
 #2  0x00007ffff70e9d29 in virCgroupGetValueU64 (group=0x0, controller=3,
     key=0x7ffff75734bd "memory.limit_in_bytes", value=0x7fffec8bf810) at util/vircgroup.c:804
 #3  0x00007ffff70ee706 in virCgroupGetMemoryHardLimit (group=0x0, kb=0x7fffec8bf8a8)
     at util/vircgroup.c:1962
 #4  0x00005555557d590f in lxcDomainGetMemoryParameters (dom=0x7fffd40024a0,
     params=0x7fffd40027a0, nparams=0x7fffec8bfa24, flags=0) at lxc/lxc_driver.c:826
 #5  0x00007ffff72c28d3 in virDomainGetMemoryParameters (domain=0x7fffd40024a0,
     params=0x7fffd40027a0, nparams=0x7fffec8bfa24, flags=0) at libvirt.c:4137
 #6  0x000055555563714d in remoteDispatchDomainGetMemoryParameters (server=0x555555eb7e00,
     client=0x555555ebaef0, msg=0x555555ebb3e0, rerr=0x7fffec8bfb70, args=0x7fffd40024e0,
     ret=0x7fffd4002420) at remote.c:1895
 #7  0x00005555556052c4 in remoteDispatchDomainGetMemoryParametersHelper (server=0x555555eb7e00,
     client=0x555555ebaef0, msg=0x555555ebb3e0, rerr=0x7fffec8bfb70, args=0x7fffd40024e0,
     ret=0x7fffd4002420) at remote_dispatch.h:4050
 #8  0x00007ffff73b293f in virNetServerProgramDispatchCall (prog=0x555555ec3ae0,
     server=0x555555eb7e00, client=0x555555ebaef0, msg=0x555555ebb3e0)
     at rpc/virnetserverprogram.c:435
 #9  0x00007ffff73b207f in virNetServerProgramDispatch (prog=0x555555ec3ae0,
     server=0x555555eb7e00, client=0x555555ebaef0, msg=0x555555ebb3e0)
     at rpc/virnetserverprogram.c:305
 #10 0x00007ffff73a4d2c in virNetServerProcessMsg (srv=0x555555eb7e00, client=0x555555ebaef0,
     prog=0x555555ec3ae0, msg=0x555555ebb3e0) at rpc/virnetserver.c:165
 #11 0x00007ffff73a4e8d in virNetServerHandleJob (jobOpaque=0x555555ebc7e0, opaque=0x555555eb7e00)
     at rpc/virnetserver.c:186
 #12 0x00007ffff7187f3f in virThreadPoolWorker (opaque=0x555555eb7ac0) at util/virthreadpool.c:144
 #13 0x00007ffff718733a in virThreadHelper (data=0x555555eb7890) at util/virthreadpthread.c:161
 #14 0x00007ffff468ed89 in start_thread (arg=0x7fffec8c0700) at pthread_create.c:308
 #15 0x00007ffff3da26bd in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:113

Signed-off-by: Martin Kletzander <>
(cherry picked from commit f8c1cb90213508c4f32549023b0572ed774e48aa)

4 years agoqemu: Don't access vm->priv on unlocked domain
Michal Privoznik [Wed, 6 Nov 2013 10:46:06 +0000]
qemu: Don't access vm->priv on unlocked domain

Since 86d90b3a (yes, my patch; again) we are supporting NBD storage
migration. However, on error recovery path we got the steps reversed.
The correct order is: return NBD port to the virPortAllocator and then
either unlock the vm or remove it from the driver. Not vice versa.

==11192== Invalid write of size 4
==11192==    at 0x11488559: qemuMigrationPrepareAny (qemu_migration.c:2459)
==11192==    by 0x11488EA6: qemuMigrationPrepareDirect (qemu_migration.c:2652)
==11192==    by 0x114D1509: qemuDomainMigratePrepare3Params (qemu_driver.c:10332)
==11192==    by 0x519075D: virDomainMigratePrepare3Params (libvirt.c:7290)
==11192==    by 0x1502DA: remoteDispatchDomainMigratePrepare3Params (remote.c:4798)
==11192==    by 0x12DECA: remoteDispatchDomainMigratePrepare3ParamsHelper (remote_dispatch.h:5741)
==11192==    by 0x5212127: virNetServerProgramDispatchCall (virnetserverprogram.c:435)
==11192==    by 0x5211C86: virNetServerProgramDispatch (virnetserverprogram.c:305)
==11192==    by 0x520A8FD: virNetServerProcessMsg (virnetserver.c:165)
==11192==    by 0x520A9E1: virNetServerHandleJob (virnetserver.c:186)
==11192==    by 0x50DA78F: virThreadPoolWorker (virthreadpool.c:144)
==11192==    by 0x50DA11C: virThreadHelper (virthreadpthread.c:161)
==11192==  Address 0x1368baa0 is 576 bytes inside a block of size 688 free'd
==11192==    at 0x4A07F5C: free (in /usr/lib64/valgrind/
==11192==    by 0x5079A2F: virFree (viralloc.c:580)
==11192==    by 0x11456C34: qemuDomainObjPrivateFree (qemu_domain.c:267)
==11192==    by 0x50F41B4: virDomainObjDispose (domain_conf.c:2034)
==11192==    by 0x50C2991: virObjectUnref (virobject.c:262)
==11192==    by 0x50F4CFC: virDomainObjListRemove (domain_conf.c:2361)
==11192==    by 0x1145C125: qemuDomainRemoveInactive (qemu_domain.c:2087)
==11192==    by 0x11488520: qemuMigrationPrepareAny (qemu_migration.c:2456)
==11192==    by 0x11488EA6: qemuMigrationPrepareDirect (qemu_migration.c:2652)
==11192==    by 0x114D1509: qemuDomainMigratePrepare3Params (qemu_driver.c:10332)
==11192==    by 0x519075D: virDomainMigratePrepare3Params (libvirt.c:7290)
==11192==    by 0x1502DA: remoteDispatchDomainMigratePrepare3Params (remote.c:4798)

Signed-off-by: Michal Privoznik <>
(cherry picked from commit 1f2f879ed17a784559f67b0fa2408d2436b731fd)

4 years agoDisable nwfilter driver when running unprivileged
Ján Tomko [Tue, 12 Nov 2013 12:18:54 +0000]
Disable nwfilter driver when running unprivileged

When opening a new connection to the driver, nwfilterOpen
only succeeds if the driverState has been allocated.

Move the privilege check in driver initialization before
the state allocation to disable the driver.

This changes the nwfilter-define error from:
error: cannot create config directory (null): Bad address
this function is not supported by the connection driver:
(cherry picked from commit b7829f959b33c6e32422222a9ed745c0da7dc696)

4 years agoBlock all use of in setuid programs
Daniel P. Berrange [Thu, 10 Oct 2013 16:45:14 +0000]
Block all use of in setuid programs

Avoid people introducing security flaws in their apps by
forbidding the use of in setuid programs, with
a check in virInitialize.

Signed-off-by: Daniel P. Berrange <>
(cherry picked from commit 9cd6a57db6ea6762fbf85c59c379a27fa6e7fd2e)

4 years agoRemove (nearly) all use of getuid()/getgid()
Daniel P. Berrange [Wed, 9 Oct 2013 11:13:45 +0000]
Remove (nearly) all use of getuid()/getgid()

Most of the usage of getuid()/getgid() is in cases where we are
considering what privileges we have. As such the code should be
using the effective IDs, not real IDs.

Signed-off-by: Daniel P. Berrange <>
(cherry picked from commit 9b0af09240618184fea5884952941217e65b824f)

4 years agoAdd stub getegid impl for platforms lacking it
Daniel P. Berrange [Thu, 17 Oct 2013 13:51:32 +0000]
Add stub getegid impl for platforms lacking it

We already have stubs for getuid, geteuid, getgid but
not for getegid. Something in gnulib already does a
check for it during configure, so we already have the
HAVE_GETEGID macro defined.

Signed-off-by: Daniel P. Berrange <>
(cherry picked from commit c566fa1ad007a280bdf5132f7f91010459036ff6)

4 years agoDon't allow remote driver daemon autostart when running setuid
Daniel P. Berrange [Wed, 9 Oct 2013 10:47:13 +0000]
Don't allow remote driver daemon autostart when running setuid

We don't want setuid programs automatically spawning libvirtd,
so disable any use of autostart when setuid.

Signed-off-by: Daniel P. Berrange <>
(cherry picked from commit 171bb129115d49c567b643acaf20b363b124b8cf)

4 years agoOnly allow the UNIX transport in remote driver when setuid
Daniel P. Berrange [Wed, 9 Oct 2013 10:44:50 +0000]
Only allow the UNIX transport in remote driver when setuid

We don't know enough about quality of external libraries used
for non-UNIX transports, nor do we want to spawn external
commands when setuid. Restrict to the bare minimum which is
UNIX transport for local usage. Users shouldn't need to be
running setuid if connecting to remote hypervisors in any

Signed-off-by: Daniel P. Berrange <>
(cherry picked from commit e22b0232c7b94aefaef87c52c4d626fa532fcce3)

4 years agoBlock all use of getenv with syntax-check
Daniel P. Berrange [Wed, 9 Oct 2013 10:19:27 +0000]
Block all use of getenv with syntax-check

The use of getenv is typically insecure, and we want people
to use our wrappers, to force them to think about setuid

Signed-off-by: Daniel P. Berrange <>
(cherry picked from commit 71b21f12bece1127b28b404f11f57b4c2d48983a)

4 years agoRemove all direct use of getenv
Daniel P. Berrange [Wed, 9 Oct 2013 10:18:15 +0000]
Remove all direct use of getenv

Unconditional use of getenv is not secure in setuid env.
While not all libvirt code runs in a setuid env (since
much of it only exists inside libvirtd) this is not always
clear to developers. So make all the code paranoid, even
if it only ever runs inside libvirtd.

Signed-off-by: Daniel P. Berrange <>
(cherry picked from commit 1e4a02bdfe6307f93763fa2c9681f280c564aee5)

4 years agoMake virCommand env handling robust in setuid env
Daniel P. Berrange [Wed, 9 Oct 2013 10:03:02 +0000]
Make virCommand env handling robust in setuid env

When running setuid, we must be careful about what env vars
we allow commands to inherit from us. Replace the
virCommandAddEnvPass function with two new ones which do


And make virCommandAddEnvPassCommon use the appropriate

Signed-off-by: Daniel P. Berrange <>
(cherry picked from commit 9b8f307c6ad002a17a0510513883d06395636793)


4 years agoInitialize threading & error layer in LXC controller
Daniel P. Berrange [Tue, 8 Oct 2013 13:35:01 +0000]
Initialize threading & error layer in LXC controller

In Fedora 20, libvirt_lxc crashes immediately at startup with a

 #0  0x00007f0cddb653ec in free () from /lib64/
 #1  0x00007f0ce0e16f4a in virFree (ptrptr=ptrptr@entry=0x7f0ce1830058) at util/viralloc.c:580
 #2  0x00007f0ce0e2764b in virResetError (err=0x7f0ce1830030) at util/virerror.c:354
 #3  0x00007f0ce0e27a5a in virResetLastError () at util/virerror.c:387
 #4  0x00007f0ce0e28858 in virEventRegisterDefaultImpl () at util/virevent.c:233
 #5  0x00007f0ce0db47c6 in main (argc=11, argv=0x7fff4596c328) at lxc/lxc_controller.c:2352

Normally virInitialize calls virErrorInitialize and
virThreadInitialize, but we don't link to
in libvirt_lxc, and nor did we ever call the error
or thread initializers.

I have absolutely no idea how this has ever worked, let alone
what caused it to stop working in Fedora 20.

In addition not all code paths from virLogSetFromEnv will
ensure virLogInitialize is called correctly, which is another
possible crash scenario.

Signed-off-by: Daniel P. Berrange <>
(cherry picked from commit 97973ebb7a64a3be6710ddd38d124307991ad7cb)

4 years agoFix flaw in detecting log format
Daniel P. Berrange [Fri, 11 Oct 2013 16:07:54 +0000]
Fix flaw in detecting log format

The log message regex has been

[0-9]{4}-[0-9]{2}-[0-9]{2} [0-9]{2}:[0-9]{2}:[0-9]{2}\.[0-9]{3}\+[0-9]{4}: [0-9]+: debug|info|warning|error :

The precedence of '|' is high though, so this is equivalent to matching

   [0-9]{4}-[0-9]{2}-[0-9]{2} [0-9]{2}:[0-9]{2}:[0-9]{2}\.[0-9]{3}\+[0-9]{4}: [0-9]+: debug






   error :

Which is clearly not what it should have done. This caused the code to
skip over things which are not log messages. The solution is to simply
add brackets.

A test case is also added to validate correctness.

Signed-off-by: Daniel P. Berrange <>
(cherry picked from commit 5787f0b95ed5a58be020836bda4b27fa3538086c)

4 years agoMove virt-login-shell into libvirt-login-shell sub-RPM
Daniel P. Berrange [Thu, 17 Oct 2013 13:18:18 +0000]
Move virt-login-shell into libvirt-login-shell sub-RPM

Many people will not want the setuid virt-login-shell binary
installed by default, so move it into a separate sub-RPM
named libvirt-login-shell. This RPM is only generated if
LXC is enabled

Signed-off-by: Daniel P. Berrange <>
(cherry picked from commit 8adc92694fecbd35f28cf236edff14509bf4eaf4)

4 years agospec: fix rpm build when lxc disabled
Eric Blake [Tue, 3 Sep 2013 21:10:01 +0000]
spec: fix rpm build when lxc disabled

'make rpm' failed if ~/.rpmmacros contains '%_without_lxc 1',
which simulates the case of not having lxc available.

RPM build errors:
    File not found: /home/eblake/rpmbuild/BUILDROOT/libvirt-1.1.1-1.fc19.x86_64/etc/libvirt/virt-login-shell.conf
    File not found by glob: /home/eblake/rpmbuild/BUILDROOT/libvirt-1.1.1-1.fc19.x86_64/usr/share/man/man1/virt-login-shell.1*
    File not found: /home/eblake/rpmbuild/BUILDROOT/libvirt-1.1.1-1.fc19.x86_64/usr/bin/virt-login-shell
make: *** [rpm] Error 1

Reported by Dan Berrange.

* Mark virt-login-shell as conditional on lxc.

Signed-off-by: Eric Blake <>
(cherry picked from commit d42906fd004dc4cda34c1d2d4b4c18fce213ede2)

4 years agoSet a sane $PATH for virt-login-shell
Daniel P. Berrange [Wed, 9 Oct 2013 10:19:52 +0000]
Set a sane $PATH for virt-login-shell

The virt-login-shell binary shouldn't need to execute programs
relying on $PATH, but just in case set a fixed $PATH value
of /bin:/usr/bin

Signed-off-by: Daniel P. Berrange <>
(cherry picked from commit d665003da1359410bc4036895a648a7d7256ddaa)

4 years agopython: Fix Create*WithFiles filefd passing
Marian Neagul [Tue, 22 Oct 2013 15:03:39 +0000]
python: Fix Create*WithFiles filefd passing

Commit d76227be added functions virDomainCreateWithFiles and
virDomainCreateXMLWithFiles, but there was a little piece missing in
python bindings.  This patch fixes proper passing of file descriptors
in the overwrites of these functions.

4 years agobuild: fix build of virt-login-shell on systems with older gnutls
Jim Fehlig [Tue, 22 Oct 2013 05:12:22 +0000]
build: fix build of virt-login-shell on systems with older gnutls

On systems where gnutls uses libgcrypt, I'm seeing the following
build failure

libvirt.c:314: error: variable 'virTLSThreadImpl' has initializer but incomplete type
libvirt.c:319: error: 'GCRY_THREAD_OPTION_PTHREAD' undeclared here (not in a function)

Fix by undefining WITH_GNUTLS_GCRYPT in config-post.h

4 years agobuild: fix linking virt-login-shell
Jim Fehlig [Mon, 21 Oct 2013 21:36:11 +0000]
build: fix linking virt-login-shell

After commit 3e2f27e1, I've noticed build failures of virt-login-shell
when libapparmor-devel is installed on the build host

CCLD     virt-login-shell
In function `virExec':
/home/jfehlig/virt/upstream/libvirt/src/util/vircommand.c:653: undefined
reference to `aa_change_profile'
collect2: error: ld returned 1 exit status

I was about to commit an easy fix under the build-breaker rule
(build-fix-1.patch), but thought to extend the notion of SECDRIVER_LIBS
to SECDRIVER_CFLAGS, and use both throughout src/ where it
makes sense (build-fix-2.patch).

Should I just stick with the simple fix, or is something along the lines
of patch 2 preferred?


>From a0f35945f3127ab70d051101037e821b1759b4bb Mon Sep 17 00:00:00 2001
From: Jim Fehlig <>
Date: Mon, 21 Oct 2013 15:30:02 -0600
Subject: [PATCH] build: fix virt-login-shell build with apparmor

With libapparmor-devel installed, virt-login-shell fails to link

CCLD     virt-login-shell
../src/.libs/libvirt-setuid-rpc-client.a(libvirt_setuid_rpc_client_la-vircommand.o): In function `virExec':
/home/jfehlig/virt/upstream/libvirt/src/util/vircommand.c:653: undefined reference to `aa_change_profile'
collect2: error: ld returned 1 exit status

Fix by linking libvirt_setuid_rpc_client with previously determined
SECDRIVER_LIBS in src/  While at it, introduce SECDRIVER_CFLAGS
and use both throughout src/ where it makes sense.

Signed-off-by: Jim Fehlig <>

src/ Context

4 years agoDon't link virt-login-shell against (CVE-2013-4400)
Daniel P. Berrange [Thu, 10 Oct 2013 12:09:08 +0000]
Don't link virt-login-shell against (CVE-2013-4400)

The library has far too many library deps to allow
linking against it from setuid programs. Those libraries can
do stuff in __attribute__((constructor) functions which is
not setuid safe.

The virt-login-shell needs to link directly against individual
files that it uses, with all library deps turned off except
for libxml2 and libselinux.

Create a library which is linked
to by virt-login-shell. A config-post.h file allows this library
to disable all external deps except libselinux and libxml2.

Signed-off-by: Daniel P. Berrange <>
(cherry picked from commit 3e2f27e13b94f7302ad948bcacb5e02c859a25fc)

4 years agoClose all non-stdio FDs in virt-login-shell (CVE-2013-4400)
Daniel P. Berrange [Wed, 9 Oct 2013 14:14:34 +0000]
Close all non-stdio FDs in virt-login-shell (CVE-2013-4400)

We don't want to inherit any FDs in the new namespace
except for the stdio FDs. Explicitly close them all,
just in case some do not have the close-on-exec flag

Signed-off-by: Daniel P. Berrange <>
(cherry picked from commit b7fcc799ad5d8f3e55b89b94e599903e3c092467)

4 years agoOnly allow 'stderr' log output when running setuid (CVE-2013-4400)
Daniel P. Berrange [Wed, 9 Oct 2013 09:59:36 +0000]
Only allow 'stderr' log output when running setuid (CVE-2013-4400)

We must not allow file/syslog/journald log outputs when running
setuid since they can be abused to do bad things. In particular
the 'file' output can be used to overwrite files.

Signed-off-by: Daniel P. Berrange <>
(cherry picked from commit 8c3586ea755c40d5e01b22cb7b5c1e668cdec994)

4 years agoAdd helpers for getting env vars in a setuid environment
Daniel P. Berrange [Wed, 9 Oct 2013 09:52:39 +0000]
Add helpers for getting env vars in a setuid environment

Care must be taken accessing env variables when running
setuid. Introduce a virGetEnvAllowSUID for env vars which
are safe to use in a setuid environment, and another
virGetEnvBlockSUID for vars which are not safe. Also add
a virIsSUID helper method for any other non-env var code
to use.

Signed-off-by: Daniel P. Berrange <>
(cherry picked from commit ae53e5d10e434e07079d7e3ba11ec654ba6a256e)

4 years agoFix perms for virConnectDomainXML{To,From}Native (CVE-2013-4401)
Daniel P. Berrange [Thu, 3 Oct 2013 15:37:57 +0000]
Fix perms for virConnectDomainXML{To,From}Native (CVE-2013-4401)

The virConnectDomainXMLToNative API should require 'connect:write'
not 'connect:read', since it will trigger execution of the QEMU
binaries listed in the XML.

Also make virConnectDomainXMLFromNative API require a full
read-write connection and 'connect:write' permission. Although the
current impl doesn't trigger execution of QEMU, we should not
rely on that impl detail from an API permissioning POV.

Signed-off-by: Daniel P. Berrange <>
(cherry picked from commit 57687fd6bf7f6e1b3662c52f3f26c06ab19dc96c)

4 years agoremote: fix regression in event deregistration
Zhou Yimin [Thu, 17 Oct 2013 07:59:21 +0000]
remote: fix regression in event deregistration

Introduced by 7b87a3
When I quit the process which only register VIR_DOMAIN_EVENT_ID_REBOOT,
I got error like:
"libvirt: XML-RPC error : internal error: domain event 0 not registered".
Then I add the following code, it fixed.

Signed-off-by: Zhou Yimin <>
Signed-off-by: Eric Blake <>
(cherry picked from commit 9712c2510ec87a87578576a407768380e250a6a4)

4 years agobuild: Add lxc testcase to dist list
Daniel Hansel [Tue, 15 Oct 2013 12:13:15 +0000]
build: Add lxc testcase to dist list

Introduced by commit 3f029fb5319b9dc9cc2fbf8d1ba4505ee9e4b1e3 the RPM build
was broken due to a missing LXC textcase.

Signed-off-by: Daniel Hansel <>
(cherry picked from commit 6285c17f790a7e5027aed0207fc5d9eb9130cc0e)

4 years agoConvert uuid to a string before printing it
Ján Tomko [Tue, 15 Oct 2013 08:29:18 +0000]
Convert uuid to a string before printing it

Introduced by 1fa7946.
(cherry picked from commit 15fac93b951eb67553ca64443c740c1b975696a9)

4 years agoLXC: Fix handling of RAM filesystem size units
Ján Tomko [Wed, 9 Oct 2013 12:17:13 +0000]
LXC: Fix handling of RAM filesystem size units

Since 76b644c when the support for RAM filesystems was introduced,
libvirt accepted the following XML:
<source usage='1024' unit='KiB'/>

This was parsed correctly and internally stored in bytes, but it
was formatted as (with an extra 's'):
<source usage='1024' units='KiB'/>
When read again, this was treated as if the units were missing,
meaning libvirt was unable to parse its own XML correctly.

The usage attribute was documented as being in KiB, but it was not
scaled if the unit was missing. Transient domains still worked,
because this was balanced by an extra 'k' in the mount options.

This patch:
Changes the parser to use 'units' instead of 'unit', as the latter
was never documented (fixing persistent domains) and some programs
(libvirt-glib, libvirt-sandbox) already parse the 'units' attribute.

Removes the extra 'k' from the tmpfs mount options, which is needed
because now we parse our own XML correctly.

Changes the default input unit to KiB to match documentation, fixing:
(cherry picked from commit 3f029fb5319b9dc9cc2fbf8d1ba4505ee9e4b1e3)

4 years agoqemuMonitorJSONSendKey: Avoid double free
Michal Privoznik [Wed, 2 Oct 2013 16:18:13 +0000]
qemuMonitorJSONSendKey: Avoid double free

After successful @cmd construction the memory where @keys points to is
part of @cmd. Avoid double freeing it.
(cherry picked from commit 3e8343e1510741623aa5bc1dfb74ec39fde868dd)

4 years agovirDomainDefParseXML: set the argument of virBitmapFree to NULL after calling virBitm...
Liuji (Jeremy) [Wed, 11 Sep 2013 02:13:32 +0000]
virDomainDefParseXML: set the argument of virBitmapFree to NULL after calling virBitmapFree

After freeing the bitmap pointer, it must set the pointer to NULL.
This will avoid any other use of the freed memory of the bitmap pointer.

Signed-off-by: Liuji (Jeremy) <>
(cherry picked from commit ef5d51d491356f1f4287aa3a8b908b183b6dd9aa)

4 years agoFree slicename in virSystemdCreateMachine
Ján Tomko [Mon, 16 Sep 2013 13:27:42 +0000]
Free slicename in virSystemdCreateMachine

1,003 bytes in 1 blocks are definitely lost in loss record 599 of 635
==404== by 0x50728A7: virBufferAddChar (virbuffer.c:185)
==404== by 0x50BC466: virSystemdEscapeName (virsystemd.c:67)
==404== by 0x50BC6B2: virSystemdMakeSliceName (virsystemd.c:108)
==404== by 0x50BC870: virSystemdCreateMachine (virsystemd.c:169)
==404== by 0x5078267: virCgroupNewMachine (vircgroup.c:1498)
(cherry picked from commit 09b48562aac12387a5ff9ac2f27f9a60490f41ee)

4 years agovirsh domjobinfo: Do not return 1 if job is NONE
Jiri Denemark [Wed, 11 Sep 2013 13:49:48 +0000]
virsh domjobinfo: Do not return 1 if job is NONE

Commit 38ab1225 changed the default value of ret from true to false but
forgot to set ret = true when job is NONE. Thus, virsh domjobinfo
returned 1 when there was no job running for a domain but it used to
(and should) return 0 in this case.
(cherry picked from commit f084caae7c5db8ae03e7fafce164c73f65681843)

4 years agoAdjust legacy max payload size to account for header information
Claudio Bley [Mon, 7 Oct 2013 10:13:00 +0000]
Adjust legacy max payload size to account for header information

Commit 27e81517a87 set the payload size to 256 KB, which is
actually the max packet size, including the size of the header.

Reduce this by VIR_NET_MESSAGE_HEADER_MAX (24) and set
VIR_NET_MESSAGE_LEGACY_PAYLOAD_MAX to 262120, which was the original
value before increasing the limit in commit eb635de1fed.

(cherry picked from commit 609eb987c6cef9082486e66b666f7b9351b783ed)

4 years agoFix max stream packet size for old clients
Daniel P. Berrange [Mon, 30 Sep 2013 16:27:51 +0000]
Fix max stream packet size for old clients

The libvirtd server pushes data out to clients. It does not
know what protocol version the client might have, so must be
conservative and use the old payload limits. ie send no more
than 256kb of data per packet.

Signed-off-by: Daniel P. Berrange <>
(cherry picked from commit 27e81517a876dcb738dd8a9bb2e0e68d71c3b7e3)

4 years agoFix crash in libvirtd when events are registered & ACLs active
Daniel P. Berrange [Fri, 27 Sep 2013 14:46:07 +0000]
Fix crash in libvirtd when events are registered & ACLs active

When a client disconnects from libvirtd, all event callbacks
must be removed. This involves running the public API


This code does not run in normal API dispatch context, so no
identity was set. The result was that the access control drivers
denied the attempt to deregister callbacks. The callbacks thus
continued to trigger after the client was free'd causing fairly
predictable use of free memory & a crash.

This can be triggered by any client with readonly access when
the ACL drivers are active.

Signed-off-by: Daniel P. Berrange <>
(cherry picked from commit 8294aa0c1750dcb49d6345cd9bd97bf421580d8b)

4 years agoqemu: Fix seamless SPICE migration
Martin Kletzander [Fri, 20 Sep 2013 14:40:20 +0000]
qemu: Fix seamless SPICE migration

Since the wait is done during migration (still inside
QEMU_ASYNC_JOB_MIGRATION_OUT), the code should enter the monitor as such
in order to prohibit all other jobs from interfering in the meantime.
This patch fixes bug #1009886 in which qemuDomainGetBlockInfo was
waiting on the monitor condition and after GetSpiceMigrationStatus
mangled its internal data, the daemon crashed.

(cherry picked from commit 484cc3217b73b865f00bf42a9c12187b37200699)

4 years agoFix typo in identity code which is pre-requisite for CVE-2013-4311
Daniel P. Berrange [Mon, 23 Sep 2013 11:46:25 +0000]
Fix typo in identity code which is pre-requisite for CVE-2013-4311

The fix for CVE-2013-4311 had a pre-requisite enhancement
to the identity code

  commit db7a5688c05f3fd60d9d2b74c72427eb9ee9c176
  Author: Daniel P. Berrange <>
  Date:   Thu Aug 22 16:00:01 2013 +0100

    Also store user & group ID values in virIdentity

This had a typo which caused the group ID to overwrite the
user ID string. This meant any checks using this would have
the wrong ID value. This only affected the ACL code, not the
initial polkit auth. It also leaked memory.

Signed-off-by: Daniel P. Berrange <>
(cherry picked from commit e4697b92abaad16e8e6b41a1e55be9b084d48d5a)

4 years agovirsh: add missing "async" option in opts_block_commit
Simone Gotti [Thu, 19 Sep 2013 13:08:29 +0000]
virsh: add missing "async" option in opts_block_commit

After commit 8aecd351266a66efa59b7f7be77bf66693d99ce0 it'll detect
that a required option is not defined and it will assert and exit with:

virsh.c:1364: vshCommandOpt: Assertion `valid->name' failed.

Problem has been latent since commit ed23b106.

Signed-off-by: Eric Blake <>
(cherry picked from commit fe64499dd14315b2d9d62cdf421bd3c97a46b7ac)

4 years agoFix crash in remoteDispatchDomainMemoryStats (CVE-2013-4296)
Daniel P. Berrange [Tue, 3 Sep 2013 15:52:06 +0000]
Fix crash in remoteDispatchDomainMemoryStats (CVE-2013-4296)

The 'stats' variable was not initialized to NULL, so if some
early validation of the RPC call fails, it is possible to jump
to the 'cleanup' label and VIR_FREE an uninitialized pointer.
This is a security flaw, since the API can be called from a
readonly connection which can trigger the validation checks.

This was introduced in release v0.9.1 onwards by

  commit 158ba8730e44b7dd07a21ab90499996c5dec080a
  Author: Daniel P. Berrange <>
  Date:   Wed Apr 13 16:21:35 2011 +0100

    Merge all returns paths from dispatcher into single path

Signed-off-by: Daniel P. Berrange <>
(cherry picked from commit e7f400a110e2e3673b96518170bfea0855dd82c0)

4 years agoAdd support for using 3-arg pkcheck syntax for process (CVE-2013-4311)
Daniel P. Berrange [Wed, 28 Aug 2013 14:25:40 +0000]
Add support for using 3-arg pkcheck syntax for process (CVE-2013-4311)

With the existing pkcheck (pid, start time) tuple for identifying
the process, there is a race condition, where a process can make
a libvirt RPC call and in another thread exec a setuid application,
causing it to change to effective UID 0. This in turn causes polkit
to do its permission check based on the wrong UID.

To address this, libvirt must get the UID the caller had at time
of connect() (from SO_PEERCRED) and pass a (pid, start time, uid)
triple to the pkcheck program.

This fix requires that libvirt is re-built against a version of
polkit that has the fix for its CVE-2013-4288, so that libvirt
can see 'pkg-config --variable pkcheck_supports_uid polkit-gobject-1'

Signed-off-by: Colin Walters <>
Signed-off-by: Daniel P. Berrange <>
(cherry picked from commit 922b7fda77b094dbf022d625238262ea05335666)

4 years agoEnsure system identity includes process start time
Daniel P. Berrange [Wed, 28 Aug 2013 14:22:05 +0000]
Ensure system identity includes process start time

The polkit access driver will want to use the process start
time field. This was already set for network identities, but
not for the system identity.

Signed-off-by: Daniel P. Berrange <>
(cherry picked from commit e65667c0c6e016d42abea077e31628ae43f57b74)

4 years agoAlso store user & group ID values in virIdentity
Daniel P. Berrange [Thu, 22 Aug 2013 15:00:01 +0000]
Also store user & group ID values in virIdentity

Future improvements to the polkit code will require access to
the numeric user ID, not merely user name.

Signed-off-by: Daniel P. Berrange <>
(cherry picked from commit db7a5688c05f3fd60d9d2b74c72427eb9ee9c176)

4 years agobuild: fix build with latest rawhide kernel headers
Eric Blake [Fri, 13 Sep 2013 16:11:26 +0000]
build: fix build with latest rawhide kernel headers

Bother those kernel developers.  In the latest rawhide, kernel
and glibc have now been unified so that <netinet/in.h> and
<linux/in6.h> no longer clash; but <linux/if_bridge.h> is still
not self-contained.  Because of the latest header change, the
build is failing with:

checking for linux/param.h... no
configure: error: You must install kernel-headers in order to compile libvirt with QEMU or LXC support

with details:

In file included from conftest.c:561:0:
/usr/include/linux/in6.h:71:18: error: field 'flr_dst' has incomplete type
  struct in6_addr flr_dst;

We need a workaround to avoid our workaround :)

* src/util/virnetdevbridge.c (includes): Use it.

Signed-off-by: Eric Blake <>
(cherry picked from commit e62e0094dcd0ca1484491a9cc62919473b647f11)

4 years agoPass AM_LDFLAGS to driver modules too
Guido Günther [Sun, 1 Sep 2013 06:50:58 +0000]
Pass AM_LDFLAGS to driver modules too

This gives us a RO got, otherwise Debian's lintian complains:

W: libvirt-bin: hardening-no-relro usr/lib/libvirt/connection-driver/
W: libvirt-bin: hardening-no-relro usr/lib/libvirt/connection-driver/
W: libvirt-bin: hardening-no-relro usr/lib/libvirt/connection-driver/
W: libvirt-bin: hardening-no-relro usr/lib/libvirt/connection-driver/
W: libvirt-bin: hardening-no-relro usr/lib/libvirt/connection-driver/
W: libvirt-bin: hardening-no-relro usr/lib/libvirt/connection-driver/
W: libvirt-bin: hardening-no-relro usr/lib/libvirt/connection-driver/
W: libvirt-bin: hardening-no-relro usr/lib/libvirt/connection-driver/
W: libvirt-sanlock: hardening-no-relro usr/lib/libvirt/lock-driver/
(cherry picked from commit f1f0e53b0814aab3c093f1219da95c0f836cdf4a)

4 years agoFix AM_LDFLAGS typo
Guido Günther [Sun, 1 Sep 2013 07:53:03 +0000]
(cherry picked from commit fe502de3bcdd76a0d256206111945ca7e4f4388a)

4 years agovirFileNBDDeviceAssociate: Avoid use of uninitialized variable
Michal Privoznik [Tue, 3 Sep 2013 16:56:06 +0000]
virFileNBDDeviceAssociate: Avoid use of uninitialized variable

The @qemunbd variable can be used uninitialized.

(cherry picked from commit 2dba0323ff0cec31bdcea9dd3b2428af297401f2)

4 years agoRelease of libvirt-1.1.2 v1.1.2
Daniel Veillard [Mon, 2 Sep 2013 01:47:37 +0000]
Release of libvirt-1.1.2

* docs/ update for the release
* po/*.po*: merged new localizations and regenerated

4 years agoqemu_hotplug: Resolve DEADCODE coverity error
John Ferlan [Sat, 31 Aug 2013 11:06:38 +0000]
qemu_hotplug: Resolve DEADCODE coverity error

Remove unused 'cgroup' variable in qemuDomainAttachDeviceDiskLive() to
resolve coverity DEADCODE complaint

4 years agoFix memory leak in cmdAttachDisk
Hongwei Bi [Sat, 31 Aug 2013 03:39:35 +0000]
Fix memory leak in cmdAttachDisk

When virBufferError is ok in cmdAttachDisk, the latter
should 'goto cleanup', instead of returning a false to
prevent memory leaking.

Signed-off-by: Eric Blake <>

4 years agobuild: fix virtlockd file distribution
Eric Blake [Sat, 31 Aug 2013 02:16:06 +0000]
build: fix virtlockd file distribution

Since virtlockd is only built when libvirtd is built, we should
not install its auxiliary files unconditionally.  This solves
two failures.  1. 'make distcheck' complains:

rm -f Makefile
ERROR: files left in build directory after distclean:

2. './' complains:

Checking for unpackaged file(s): /usr/lib/rpm/check-files
error: Installed (but unpackaged) file(s) found:



* src/ (CLEANFILES): Add virtlockd.8.
(man8_MANS, conf_DATA, augeas_DATA, augeastest_DATA): Only install
virtlockd files when daemon is built.

Signed-off-by: Eric Blake <>

4 years agobuild: shipped files must not depend on BUILT_SOURCES
Eric Blake [Fri, 30 Aug 2013 22:05:43 +0000]
build: shipped files must not depend on BUILT_SOURCES

'make distcheck' was failing with:
make[3]: Entering directory `/home/eblake/libvirt-tmp2/libvirt-1.1.1/_build/docs'
perl ../../docs/ ../../src/access/viraccessperm.h > ../../docs/aclperms.htmlinc
/bin/sh: ../../docs/aclperms.htmlinc: Permission denied

when simulating the case of a user doing a VPATH build from a
read-only source tree.  The culprit?  BUILT_SOURCES are _always_
built, and so must NOT be built into srcdir and need not be part
of the tarball.  On the other hand, shipped files must never
depend on files in the builddir.  While it would be possible to
fix the problem by generating aclperms.htmlinc into builddir,
we then have the problem that we ship acl.html - we'd have to
rejigger a lot of things to not ship pre-built html.  So this
patch goes the other direction - we don't need BUILT_SOURCES,
but instead ensure that we have proper dependencies so that
all files in srcdir are up-to-date at the time the tarball is
created.  And because we ship html files in the tarball, that
implies we don't expect users to be able to rebuild them, so
we must not clean any files that would trigger a rebuild except
under the maintainer rules.

* docs/ (BUILT_SOURCES): Delete.
(CLEANFILES): Downgrade aclperms.htmlinc cleanup...
(maintainer-clean-local): ...and move
(MAINTAINERCLEANFILES): a maintainer action.
( Write into srcdir.
(hvsupport.html): Ensure files are built in order.
(aclperms.htmlinc): Honor silent make.
(EXTRA_DIST): Ship aclperms.htmlinc.

Signed-off-by: Eric Blake <>

4 years agobuild: fix 'make distcheck' out of the box
Eric Blake [Fri, 30 Aug 2013 21:03:52 +0000]
build: fix 'make distcheck' out of the box

With the 1.1.1 tarball, if a user does 'make && make distcheck',
things pass, but if they do 'make distcheck' after 'make clean',
there is an odd failure:

  GEN      ../../docs/devhelp/index.html
I/O error : Permission denied
I/O error : Permission denied
runtime error: file ../../docs/devhelp/devhelp.xsl line 43 element document
xsltDocumentElem: unable to save to ../../docs/devhelp/libvirt-virterror.html
I/O error : Permission denied
I/O error : Permission denied

This implies that the rules for 'make dist' are missing a
dependency - the generated documentation needs to be up-to-date
before creating the tarball, or else the tarball will be missing
files, where the end user will end up trying to rebuild files in
srcdir, and that fails when srcdir is read-only.

1.1.1 plus this patch now works without issues (other issues have
crept in to 1.1.2-rc1 that prevent 'make distcheck' from working,
but those will be cleaned up in later patches).

* docs/ (dist-local): New dependency.

Signed-off-by: Eric Blake <>

4 years agobuild: only create virt-login-shell for lxc builds
Eric Blake [Fri, 30 Aug 2013 19:58:59 +0000]
build: only create virt-login-shell for lxc builds

I noticed from an ./ run that we were installing a
virt-login-shell.exe binary when cross-building for mingw,
even though such a binary is necessarily worthless since the
code depends on lxc which is a Linux-only concept.

* tools/ (conf_DATA, bin_PROGRAMS, dist_man1_MANS):
Make virt-login-shell installation conditional.

Signed-off-by: Eric Blake <>

4 years agoqemu: Only setup vhost if virtType == "kvm"
Cole Robinson [Thu, 1 Aug 2013 01:37:40 +0000]
qemu: Only setup vhost if virtType == "kvm"

vhost only works in KVM mode at the moment, and is infact compiled
out if the emulator is built for non-native architecture. While it
may work at some point in the future for plain qemu, for now it's
just noise on the command line (and which contributes to arm cli

4 years agoProcess virtlockd.conf instead of libvirtd.conf
Guido Günther [Fri, 30 Aug 2013 10:58:08 +0000]
Process virtlockd.conf instead of libvirtd.conf

4 years agoChange way we fake dbus method calls
Daniel P. Berrange [Fri, 30 Aug 2013 13:10:52 +0000]
Change way we fake dbus method calls

Ubuntu links with -Bsymbolic-functions, which means
that we can only LD_PRELOAD functions that we directly call.
Functions which calls internally can not be replaced.
Thus we cannot use dbus_message_new_error or dbus_message_new_method_return

Signed-off-by: Daniel P. Berrange <>

4 years agorandom: don't mix RAND_MAX with random_r
Eric Blake [Thu, 29 Aug 2013 23:03:34 +0000]
random: don't mix RAND_MAX with random_r

FreeBSD 10 recently changed their definition of RAND_MAX, to try
and cover the fact that their evenly distributed results of rand()
really are a smaller range than a full power of 2.  As a result,
I did some investigation, and learned:

1. POSIX requires random() to be evenly distributed across exactly
31 bits.  glibc also guarantees this for rand(), but the two are
unrelated, and POSIX only associates RAND_MAX with rand().
Avoiding RAND_MAX altogether thus avoids a build failure on
FreeBSD 10.

2. Concatenating random bits from a PRNG will NOT provide uniform
coverage over the larger value UNLESS the period of the original
PRNG is at least as large as the number of bits being concatenated.
Simple example: suppose that RAND_MAX were 1 with a period of 2**1
(which means that the PRNG merely alternates between 0 and 1).
Concatenating two successive rand() calls would then invariably
result in 01 or 10, which is a rather non-uniform distribution
(00 and 11 are impossible) and an even worse period (2**0, since
our second attempt will get the same number as our first attempt).
But a RAND_MAX of 1 with a period of 2**2 (alternating between
0, 1, 1, 0) provides sane coverage of all four values, if properly
tempered.  (Back-to-back calls would still only see half the values
if we don't do some tempering).  We therefore want to guarantee a
period of at least 2**64, preferably larger (as a tempering factor);
POSIX only makes this guarantee for random() with 256 bytes of info.

* src/util/virrandom.c (virRandomBits): Use constants that are
accurate for the PRNG we are using, not an unrelated PRNG.
(randomState): Ensure the period of our PRNG exceeds our usage.

Signed-off-by: Eric Blake <>

4 years agovirsh-domain: rename print_job_progress to vshPrintJobProgress
Peter Krempa [Mon, 26 Aug 2013 10:31:51 +0000]
virsh-domain: rename print_job_progress to vshPrintJobProgress

4 years agosecurity: provide supplemental groups even when parsing label (CVE-2013-4291) CVE-2013-4291 v1.1.2-rc2
Eric Blake [Fri, 23 Aug 2013 15:30:42 +0000]
security: provide supplemental groups even when parsing label (CVE-2013-4291)

Commit 29fe5d7 (released in 1.1.1) introduced a latent problem
for any caller of virSecurityManagerSetProcessLabel and where
the domain already had a uid:gid label to be parsed.  Such a
setup would collect the list of supplementary groups during
virSecurityManagerPreFork, but then ignores that information,
and thus fails to call setgroups() to adjust the supplementary
groups of the process.

Upstream does not use virSecurityManagerSetProcessLabel for
qemu (it uses virSecurityManagerSetChildProcessLabel instead),
so this problem remained latent until backporting the initial
commit into v0.10.2-maint (commit c061ff5, released in,
where virSecurityManagerSetChildProcessLabel has not been
backported.  As a result of using a different code path in the
backport, attempts to start a qemu domain that runs as qemu:qemu
will end up with supplementary groups unchanged from the libvirtd
parent process, rather than the desired supplementary groups of
the qemu user.  This can lead to failure to start a domain
(typical Fedora setup assigns user 107 'qemu' to both group 107
'qemu' and group 36 'kvm', so a disk image that is only readable
under kvm group rights is locked out).  Worse, it is a security
hole (the qemu process will inherit supplemental group rights
from the parent libvirtd process, which means it has access
rights to files owned by group 0 even when such files should
not normally be visible to user qemu).

LXC does not use the DAC security driver, so it is not vulnerable
at this time.  Still, it is better to plug the latent hole on
the master branch first, before cherry-picking it to the only
vulnerable branch v0.10.2-maint.

* src/security/security_dac.c (virSecurityDACGetIds): Always populate
groups and ngroups, rather than only when no label is parsed.

Signed-off-by: Eric Blake <>

4 years agoProhibit unbounded arrays in XDR protocols
Daniel P. Berrange [Mon, 19 Aug 2013 14:17:20 +0000]
Prohibit unbounded arrays in XDR protocols

The use of <> is a security issue for RPC parameters, since a
malicious client can set a huge array length causing arbitrary
memory allocation in the daemon.

It is also a robustness issue for RPC return values, because if
the stream is corrupted, it can cause the client to also allocate
arbitrary memory.

Use a syntax-check rule to prohibit any use of <>

Signed-off-by: Daniel P. Berrange <>

4 years agoAdd bounds checking on virConnectListAllSecrets RPC call
Daniel P. Berrange [Mon, 19 Aug 2013 13:49:57 +0000]
Add bounds checking on virConnectListAllSecrets RPC call

The return values for the virConnectListAllSecrets call were not
bounds checked. This is a robustness issue for clients if
something where to cause corruption of the RPC stream data.

Signed-off-by: Daniel P. Berrange <>

4 years agoAdd bounds checking on virConnectListAllNWFilters RPC call
Daniel P. Berrange [Mon, 19 Aug 2013 13:47:22 +0000]
Add bounds checking on virConnectListAllNWFilters RPC call

The return values for the virConnectListAllNWFilters call were not
bounds checked. This is a robustness issue for clients if
something where to cause corruption of the RPC stream data.

Signed-off-by: Daniel P. Berrange <>

4 years agoAdd bounds checking on virConnectListAllNodeDevices RPC call
Daniel P. Berrange [Mon, 19 Aug 2013 13:44:52 +0000]
Add bounds checking on virConnectListAllNodeDevices RPC call

The return values for the virConnectListAllNodeDevices call were not
bounds checked. This is a robustness issue for clients if
something where to cause corruption of the RPC stream data.

Signed-off-by: Daniel P. Berrange <>

4 years agoAdd bounds checking on virConnectListAllInterfaces RPC call
Daniel P. Berrange [Mon, 19 Aug 2013 13:41:56 +0000]
Add bounds checking on virConnectListAllInterfaces RPC call

The return values for the virConnectListAllInterfaces call were not
bounds checked. This is a robustness issue for clients if
something where to cause corruption of the RPC stream data.

Signed-off-by: Daniel P. Berrange <>

4 years agoAdd bounds checking on virConnectListAllNetworks RPC call
Daniel P. Berrange [Mon, 19 Aug 2013 13:37:29 +0000]
Add bounds checking on virConnectListAllNetworks RPC call

The return values for the virConnectListAllNetworks call were not
bounds checked. This is a robustness issue for clients if
something where to cause corruption of the RPC stream data.

Signed-off-by: Daniel P. Berrange <>

4 years agoAdd bounds checking on virStoragePoolListAllVolumes RPC call
Daniel P. Berrange [Mon, 19 Aug 2013 13:33:58 +0000]
Add bounds checking on virStoragePoolListAllVolumes RPC call

The return values for the virStoragePoolListAllVolumes call were not
bounds checked. This is a robustness issue for clients if
something where to cause corruption of the RPC stream data.

Signed-off-by: Daniel P. Berrange <>

4 years agoAdd bounds checking on virConnectListAllStoragePools RPC call
Daniel P. Berrange [Mon, 19 Aug 2013 13:27:56 +0000]
Add bounds checking on virConnectListAllStoragePools RPC call

The return values for the virConnectListAllStoragePools call were not
bounds checked. This is a robustness issue for clients if
something where to cause corruption of the RPC stream data.

Signed-off-by: Daniel P. Berrange <>

4 years agoAdd bounds checking on virConnectListAllDomains RPC call
Daniel P. Berrange [Mon, 19 Aug 2013 13:23:31 +0000]
Add bounds checking on virConnectListAllDomains RPC call

The return values for the virConnectListAllDomains call were not
bounds checked. This is a robustness issue for clients if
something where to cause corruption of the RPC stream data.

Signed-off-by: Daniel P. Berrange <>

4 years agoAdd bounds checking on virDomain{SnapshotListAllChildren,ListAllSnapshots} RPC calls
Daniel P. Berrange [Mon, 19 Aug 2013 11:55:53 +0000]
Add bounds checking on virDomain{SnapshotListAllChildren,ListAllSnapshots} RPC calls

The return values for the virDomain{SnapshotListAllChildren,ListAllSnapshots}
calls were not bounds checked. This is a robustness issue for clients if
something where to cause corruption of the RPC stream data.

Signed-off-by: Daniel P. Berrange <>

4 years agoAdd bounds checking on virDomainGetJobStats RPC call
Daniel P. Berrange [Mon, 19 Aug 2013 11:42:31 +0000]
Add bounds checking on virDomainGetJobStats RPC call

The return values for the virDomainGetJobStats call were not
bounds checked. This is a robustness issue for clients if
something where to cause corruption of the RPC stream data.

Signed-off-by: Daniel P. Berrange <>

4 years agoAdd bounds checking on virDomainMigrate*Params RPC calls (CVE-2013-4292) CVE-2013-4292
Daniel P. Berrange [Mon, 19 Aug 2013 13:55:21 +0000]
Add bounds checking on virDomainMigrate*Params RPC calls (CVE-2013-4292)

The parameters for the virDomainMigrate*Params RPC calls were
not bounds checks, meaning a malicious client can cause libvirtd
to consume arbitrary memory

This issue was introduced in the 1.1.0 release of libvirt

Signed-off-by: Daniel P. Berrange <>

4 years agopython: Fix a PyList usage mistake
Guan Qiang [Thu, 29 Aug 2013 11:02:25 +0000]
python: Fix a PyList usage mistake

Fix PyList usage mistake in Function libvirt_lxc_virDomainLxcOpenNamespace.

Signed-off-by: Eric Blake <>

4 years Correctly detect .git as a file
Michal Privoznik [Thu, 29 Aug 2013 11:19:45 +0000] Correctly detect .git as a file

One of my previous patches 5cfe0d37cd0be tried to handle the case when
libvirt is a submodule of another project. In that case, the .git is
just a link to the parent .git directory (which the script
didn't count on). The fix was missing 'test' though.

4 years agobridge_driver: Introduce networkObjFromNetwork
Michal Privoznik [Wed, 28 Aug 2013 12:34:34 +0000]
bridge_driver: Introduce networkObjFromNetwork

Similarly to qemu_driver.c, we can join often repeating code of looking
up network into one function: networkObjFromNetwork.

Signed-off-by: Michal Privoznik <>

4 years agoqemu_hotplug: Fix whitespace around addition in argument
Peter Krempa [Wed, 28 Aug 2013 12:56:21 +0000]
qemu_hotplug: Fix whitespace around addition in argument

4 years agoqemu: Remove hostdev entry when freeing the depending network entry
Peter Krempa [Tue, 27 Aug 2013 17:06:18 +0000]
qemu: Remove hostdev entry when freeing the depending network entry

When using a <interface type="network"> that points to a network with
hostdev forwarding mode a hostdev alias is created for the network. This
allias is inserted into the hostdev list, but is backed with a part of
the network object that it is connected to.

When a VM is being stopped qemuProcessStop() calls
networkReleaseActualDevice() which eventually frees the memory for the
hostdev object. Afterwards when the domain definition is being freed by
virDomainDefFree() an invalid pointer is accessed by
virDomainHostdevDefFree() and may cause a crash of the daemon.

This patch removes the entry in the hostdev list before freeing the
depending memory to avoid this issue.


4 years agovirsh: detect programming errors with option parsing
Eric Blake [Fri, 16 Aug 2013 22:07:31 +0000]
virsh: detect programming errors with option parsing

Noticed while reviewing another patch that had an accidental
mismatch due to refactoring.  An audit of the code showed that
very few callers of vshCommandOpt were expecting a return of
-2, indicating programmer error, and of those that DID check,
they just propagated that status to yet another caller that
did not check.  Fix this by making the code blatantly warn
the programmer, rather than silently ignoring it and possibly
doing the wrong thing downstream.

I know that we frown on assert()/abort() inside libvirtd
(libraries should NEVER kill the program that linked them),
but as virsh is an app rather than the library, and as this
is not the first use of assert() in virsh, I think this
approach is okay.

* tools/virsh.h (vshCommandOpt): Drop declaration.
* tools/virsh.c (vshCommandOpt): Make static, and add a
parameter.  Abort on programmer errors rather than making callers
repeat that logic.
(vshCommandOptInt, vshCommandOptUInt, vshCommandOptUL)
(vshCommandOptString, vshCommandOptStringReq)
(vshCommandOptLongLong, vshCommandOptULongLong)
(vshCommandOptBool): Adjust callers.

Signed-off-by: Eric Blake <>

4 years agovirt-sanlock-cleanup; Fix augtool usage v1.1.2-rc1
Jiri Denemark [Wed, 28 Aug 2013 11:50:10 +0000]
virt-sanlock-cleanup; Fix augtool usage

Surprisingly, augtool get (or print) returns "path = value" while we are
only interested in the value. We need to remove the "path = " part from
the augtool's output. The following is an example of the augtool command
as used in virt-sanlock-cleanup script:

$ augtool get /files/etc/libvirt/qemu-sanlock.conf/disk_lease_dir
/files/etc/libvirt/qemu-sanlock.conf/disk_lease_dir = /var/lib/libvirt/sanlock

4 years agovirsh: Fix debugging
Martin Kletzander [Tue, 27 Aug 2013 11:19:24 +0000]
virsh: Fix debugging

Commit a0b6a36f "fixed" what abfff210 broke (URI precedence), but
there was still one more thing missing to fix.  When using virsh
parameters to setup debugging, those weren't honored, because at the
time debugging was initializing, arguments weren't parsed yet.  To
make ewerything work as expected, we need to initialize the debugging
twice, once before debugging (so we can debug option parsing properly)
and then again after these options are parsed.

As a side effect, this patch also fixes a leak when virsh is ran with
multiple '-l' parameters.

Signed-off-by: Martin Kletzander <>

4 years agovirsh-pool.c: Don't jump over variable declaration
Michal Privoznik [Wed, 28 Aug 2013 07:25:59 +0000]
virsh-pool.c: Don't jump over variable declaration

Since 785ff34bf8 we are using the outputStr variable in cleanup label.
However, there is a possibility to jump to the label before the variable
has been declared:

virsh-pool.c: In function 'cmdPoolList':
virsh-pool.c:1121:25: error: jump skips variable initialization [-Werror=jump-misses-init]
                         goto asprintf_failure;
virsh-pool.c:1308:1: note: label 'asprintf_failure' defined here
virsh-pool.c:1267:11: note: 'outputStr' declared here
     char *outputStr = NULL;

4 years agovirsh: free the caps list properly if one of them is invalid
Ján Tomko [Tue, 27 Aug 2013 11:47:57 +0000]
virsh: free the caps list properly if one of them is invalid

VIR_FREE(caps) is not enough to free an array allocated
by vshStringToArray.

==17== 4 bytes in 1 blocks are definitely lost in loss record 4 of 728
==17==    by 0x4EFFC44: virStrdup (virstring.c:554)
==17==    by 0x128B10: _vshStrdup (virsh.c:125)
==17==    by 0x129164: vshStringToArray (virsh.c:218)
==17==    by 0x157BB3: cmdNodeListDevices (virsh-nodedev.c:409)

4 years agovirsh: free the formatting string when listing pool details
Ján Tomko [Tue, 27 Aug 2013 11:34:09 +0000]
virsh: free the formatting string when listing pool details

==23== 41 bytes in 1 blocks are definitely lost in loss record 626 of 727
==23==    by 0x4F0099F: virAsprintfInternal (virstring.c:358)
==23==    by 0x15D2C9: cmdPoolList (virsh-pool.c:1268)

4 years agovirsh: free the list from ListAll APIs even for 0 items
Ján Tomko [Tue, 27 Aug 2013 11:27:50 +0000]
virsh: free the list from ListAll APIs even for 0 items

virsh secret-list leak when no secrets are defined:

==27== 8 bytes in 1 blocks are definitely lost in loss record 6 of 726
==27==    by 0x4E941DD: virAllocN (viralloc.c:183)
==27==    by 0x5037F1A: remoteConnectListAllSecrets (remote_driver.c:3076)
==27==    by 0x5004EC6: virConnectListAllSecrets (libvirt.c:16298)
==27==    by 0x15F813: vshSecretListCollect (virsh-secret.c:397)
==27==    by 0x15F0E1: cmdSecretList (virsh-secret.c:532)

And so do some other *-list commands.

4 years agovirsh: free messages after logging them to a file
Ján Tomko [Tue, 27 Aug 2013 11:07:27 +0000]
virsh: free messages after logging them to a file

The messages were only freed on error.

==12== 1,100 bytes in 1 blocks are definitely lost in loss record 698 of 729
==12==    by 0x4E98C22: virBufferAsprintf (virbuffer.c:294)
==12==    by 0x12C950: vshOutputLogFile (virsh.c:2440)
==12==    by 0x12880B: vshError (virsh.c:2254)
==12==    by 0x131957: vshCommandOptDomainBy (virsh-domain.c:109)
==12==    by 0x14253E: cmdStart (virsh-domain.c:3333)

4 years agoTest network update XML parsing
Ján Tomko [Mon, 29 Jul 2013 15:17:47 +0000]
Test network update XML parsing

Add checks for updating sections of network definition via

4 years agoRemove the space before the slash in network XML
Ján Tomko [Tue, 30 Jul 2013 12:36:08 +0000]
Remove the space before the slash in network XML

This matches the style we use elsewhere and allows
nat-network-dns-srv-record{,-minimal}.xml to be tested in
network XML -> XML test.

4 years agoBuild QEMU command line for pcihole64
Ján Tomko [Mon, 12 Aug 2013 11:48:34 +0000]
Build QEMU command line for pcihole64

QEMU commit 3984890 introduced the "pci-hole64-size" property,
to i440FX-pcihost and q35-pcihost with a default setting of 2 GB.

Translate <pcihole64>x<pcihole64/> to:
-global q35-pcihost.pci-hole64-size=x for q35 machines and
-global i440FX-pcihost.pci-hole64-size=x for i440FX-based machines.

Error out on other machine types or if the size was specified
but the pcihost device lacks 'pci-hole64-size' property.

4 years agoAdd pcihole64 element to root PCI controllers
Ján Tomko [Mon, 12 Aug 2013 11:39:04 +0000]
Add pcihole64 element to root PCI controllers

<controller type='pci' index='0' model='pci-root'>
  <pcihole64 unit='KiB'>1048576</pcihole64>

It can be used to adjust (or disable) the size of the 64-bit
PCI hole. The size attribute is in kilobytes (different unit
can be specified on input), but it gets rounded up to
the nearest GB by QEMU.

Disabling it will be needed for guests that crash with the
64-bit PCI hole (like Windows XP), see:

4 years agoAllow controller XML parsing to use XPath context
Ján Tomko [Tue, 13 Aug 2013 13:10:17 +0000]
Allow controller XML parsing to use XPath context

virDomainParseScaledValue requires it.

4 years agoMove virDomainParseScaledValue earlier
Ján Tomko [Tue, 13 Aug 2013 13:08:19 +0000]
Move virDomainParseScaledValue earlier

Let virDomainControllerDefParseXML use it without
a forward declaration.

4 years agoAdd ftp protocol support for cdrom disk
Aline Manera [Thu, 22 Aug 2013 19:03:08 +0000]
Add ftp protocol support for cdrom disk

The ftp protocol is already recognized by qemu/KVM so add this support to
libvirt as well.
The xml should be as following:

     <disk type='network' device='cdrom'>
       <source protocol='ftp' name='/url/path'>
         <host name='' port='21'/>

Signed-off-by: Aline Manera <>

4 years agoAdd http protocol support for cdrom disk
Aline Manera [Thu, 22 Aug 2013 19:03:07 +0000]
Add http protocol support for cdrom disk

QEMU/KVM already allows a HTTP URL for the cdrom ISO image so add this support
to libvirt as well.
The xml should be as following:

    <disk type='network' device='cdrom'>
      <source protocol='http' name='/url/path'>
        <host name='' port='80'/>

Signed-off-by: Aline Manera <>

4 years agoAlways specify qcow2 compat level on qemu-img command line
Ján Tomko [Tue, 20 Aug 2013 15:37:08 +0000]
Always specify qcow2 compat level on qemu-img command line

qemu-img is going to switch the default for QCOW2
to QCOW2v3 (compat=1.1)

Extend the probing for qemu-img command line options to check
if -o compat is supported. If the volume definition specifies
the qcow2 format but no compat level and -o compat is supported,
specify -o compat=0.10 to create a QCOW2v2 image.

4 years agovirsh: fix return value error of cpu-stats
Guannan Ren [Fri, 23 Aug 2013 10:17:25 +0000]
virsh: fix return value error of cpu-stats

virsh cpu-stats guest --start 0 --count 3
It outputs right but the return value is 1 rather than 0
echo $?

Found by running libvirt-autotest
./run -t libvirt --tests virsh_cpu_stats

4 years agovirsh: C99 style for info_domfstrim and opts_lxc_enter_namespace
Tomas Meszaros [Mon, 26 Aug 2013 12:36:52 +0000]
virsh: C99 style for info_domfstrim and opts_lxc_enter_namespace

Change info_domfstrim and opts_lxc_enter_namespace initialization style
to C99.

4 years agoqemuDomainAttachHostPciDevice: Fall back to mem balloon if there's no hard_limit
Michal Privoznik [Mon, 26 Aug 2013 14:59:03 +0000]
qemuDomainAttachHostPciDevice: Fall back to mem balloon if there's no hard_limit

If there's no hard_limit set and domain uses VFIO we still must lock
the guest memory (prerequisite from qemu). Hence, we should compute
the amount to be locked from max_balloon.