Commit Graph

14335 Commits

Author SHA1 Message Date
Jiri Denemark
2842b103b1 Really don't crash if a connection closes early
https://bugzilla.redhat.com/show_bug.cgi?id=1047577

When writing commit 173c291, I missed the fact virNetServerClientClose
unlocks the client object before actually clearing client->sock and thus
it is possible to hit a window when client->keepalive is NULL while
client->sock is not NULL. I was thinking client->sock == NULL was a
better check for a closed connection but apparently we have to go with
client->keepalive == NULL to actually fix the crash.

Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
(cherry picked from commit 066c8ef6c1)
2014-01-14 21:47:05 -07:00
Jiri Denemark
ed327dfcf4 Don't crash if a connection closes early
https://bugzilla.redhat.com/show_bug.cgi?id=1047577

When a client closes its connection to libvirtd early during
virConnectOpen, more specifically just after making
REMOTE_PROC_CONNECT_SUPPORTS_FEATURE call to check if
VIR_DRV_FEATURE_PROGRAM_KEEPALIVE is supported without even waiting for
the result, libvirtd may crash due to a race in keep-alive
initialization. Once receiving the REMOTE_PROC_CONNECT_SUPPORTS_FEATURE
call, the daemon's event loop delegates it to a worker thread. In case
the event loop detects EOF on the connection and calls
virNetServerClientClose before the worker thread starts to handle
REMOTE_PROC_CONNECT_SUPPORTS_FEATURE call, client->keepalive will be
disposed by the time virNetServerClientStartKeepAlive gets called from
remoteDispatchConnectSupportsFeature. Because the flow is common for
both authenticated and read-only connections, even unprivileged clients
may cause the daemon to crash.

To avoid the crash, virNetServerClientStartKeepAlive needs to check if
the connection is still open before starting keep-alive protocol.

Every libvirt release since 0.9.8 is affected by this bug.

(cherry picked from commit 173c291473)
2014-01-14 21:47:03 -07:00
Jiri Denemark
82daa87f6a qemu: Fix job usage in virDomainGetBlockIoTune
CVE-2013-6458

Every API that is going to begin a job should do that before fetching
data from vm->def.

(cherry picked from commit 3b56425938)
2014-01-14 21:46:59 -07:00
Jiri Denemark
939b0818c2 qemu: Fix job usage in qemuDomainBlockCopy
Every API that is going to begin a job should do that before fetching
data from vm->def.

(cherry picked from commit ff5f30b6bf)
2014-01-14 21:46:58 -07:00
Jiri Denemark
bcb9a035a9 qemu: Fix job usage in qemuDomainBlockJobImpl
CVE-2013-6458

Every API that is going to begin a job should do that before fetching
data from vm->def.

(cherry picked from commit f93d2caa07)
2014-01-14 21:46:55 -07:00
Jiri Denemark
54cb7f05ec qemu: Avoid using stale data in virDomainGetBlockInfo
CVE-2013-6458

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 b799259583)
2014-01-14 21:46:53 -07:00
Jiri Denemark
17db7e28a1 qemu: Do not access stale data in virDomainBlockStats
CVE-2013-6458
https://bugzilla.redhat.com/show_bug.cgi?id=1043069

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 db86da5ca2)
2014-01-14 21:43:43 -07:00
Dario Faggioli
52c4000380 libxl: avoid crashing if calling `virsh numatune' on inactive domain
by, in libxlDomainGetNumaParameters(), calling libxl_bitmap_init() as soon as
possible, which avoids getting to 'cleanup:', where libxl_bitmap_dispose()
happens, without having initialized the nodemap, and hence crashing after some
invalid free()-s:

 # ./daemon/libvirtd -v
 *** Error in `/home/xen/libvirt.git/daemon/.libs/lt-libvirtd': munmap_chunk(): invalid pointer: 0x00007fdd42592666 ***
 ======= Backtrace: =========
 /lib64/libc.so.6(+0x7bbe7)[0x7fdd3f767be7]
 /lib64/libxenlight.so.4.3(libxl_bitmap_dispose+0xd)[0x7fdd2c88c045]
 /home/xen/libvirt.git/daemon/.libs/../../src/.libs/libvirt_driver_libxl.so(+0x12d26)[0x7fdd2caccd26]
 /home/xen/libvirt.git/src/.libs/libvirt.so.0(virDomainGetNumaParameters+0x15c)[0x7fdd4247898c]
 /home/xen/libvirt.git/daemon/.libs/lt-libvirtd(+0x1d9a2)[0x7fdd42ecc9a2]
 /home/xen/libvirt.git/src/.libs/libvirt.so.0(virNetServerProgramDispatch+0x3da)[0x7fdd424e9eaa]
 /home/xen/libvirt.git/src/.libs/libvirt.so.0(+0x1a6f38)[0x7fdd424e3f38]
 /home/xen/libvirt.git/src/.libs/libvirt.so.0(+0xa81e5)[0x7fdd423e51e5]
 /home/xen/libvirt.git/src/.libs/libvirt.so.0(+0xa783e)[0x7fdd423e483e]
 /lib64/libpthread.so.0(+0x7c53)[0x7fdd3febbc53]
 /lib64/libc.so.6(clone+0x6d)[0x7fdd3f7e1dbd]

Signed-off-by: Dario Faggili <dario.faggioli@citrix.com>
Cc: Jim Fehlig <jfehlig@suse.com>
Cc: Ian Jackson <Ian.Jackson@eu.citrix.com>
(cherry picked from commit f9ee91d355)

Conflicts:
	src/libxl/libxl_driver.c
2013-12-28 16:38:28 -06:00
Martin Kletzander
f460750474 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.

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

Backtrace:
 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 <mkletzan@redhat.com>
(cherry picked from commit 9faf3f2950)
2013-12-20 13:01:32 +00:00
Martin Kletzander
7938401848 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.

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

Backtrace:
 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 <mkletzan@redhat.com>
(cherry picked from commit f8c1cb9021)
2013-12-20 13:01:28 +00:00
Michal Privoznik
b861c09a1d 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/vgpreload_memcheck-amd64-linux.so)
==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 <mprivozn@redhat.com>
(cherry picked from commit 1f2f879ed1)
2013-11-13 11:04:46 +01:00
Ján Tomko
17cfc54664 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
To:
this function is not supported by the connection driver:
virNWFilterDefineXML

https://bugzilla.redhat.com/show_bug.cgi?id=1029266
(cherry picked from commit b7829f959b)
2013-11-13 10:50:48 +01:00
Daniel P. Berrange
0c4aec29b6 Block all use of libvirt.so in setuid programs
Avoid people introducing security flaws in their apps by
forbidding the use of libvirt.so in setuid programs, with
a check in virInitialize.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
(cherry picked from commit 9cd6a57db6)
2013-10-30 11:51:43 +00:00
Daniel P. Berrange
54b33cc986 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 <berrange@redhat.com>
(cherry picked from commit 9b0af09240)
2013-10-30 11:51:37 +00:00
Daniel P. Berrange
7bb54ef7f4 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 <berrange@redhat.com>
(cherry picked from commit c566fa1ad0)
2013-10-30 11:51:32 +00:00
Daniel P. Berrange
a8813e50c2 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 <berrange@redhat.com>
(cherry picked from commit 171bb12911)
2013-10-30 11:51:27 +00:00
Daniel P. Berrange
89b2e56d7a 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
case.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
(cherry picked from commit e22b0232c7)
2013-10-30 11:51:21 +00:00
Daniel P. Berrange
d47659dcc2 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
needs.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
(cherry picked from commit 71b21f12be)
2013-10-30 11:51:17 +00:00
Daniel P. Berrange
97f3a8d878 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 <berrange@redhat.com>
(cherry picked from commit 1e4a02bdfe)
2013-10-30 11:51:11 +00:00
Daniel P. Berrange
6f5f52c43f 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
filtering

  virCommandAddEnvPassAllowSUID
  virCommandAddEnvPassBlockSUID

And make virCommandAddEnvPassCommon use the appropriate
ones

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
(cherry picked from commit 9b8f307c6a)

Conflicts:
	src/qemu/qemu_command.c
2013-10-30 11:50:26 +00:00
Daniel P. Berrange
b4ae6429e1 Initialize threading & error layer in LXC controller
In Fedora 20, libvirt_lxc crashes immediately at startup with a
trace

 #0  0x00007f0cddb653ec in free () from /lib64/libc.so.6
 #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 libvirt.so
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 <berrange@redhat.com>
(cherry picked from commit 97973ebb7a)
2013-10-30 11:49:45 +00:00
Daniel P. Berrange
00991ef1a0 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

Or

   info

Or

   warning

Or

   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 <berrange@redhat.com>
(cherry picked from commit 5787f0b95e)
2013-10-30 11:49:39 +00:00
Daniel P. Berrange
908efd3ec9 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 <berrange@redhat.com>
(cherry picked from commit 8adc92694f)
2013-10-30 11:49:28 +00:00
Eric Blake
0a25677126 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.

* libvirt.spec.in: Mark virt-login-shell as conditional on lxc.

Signed-off-by: Eric Blake <eblake@redhat.com>
(cherry picked from commit d42906fd00)
2013-10-30 11:49:23 +00:00
Daniel P. Berrange
050c432d1a 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 <berrange@redhat.com>
(cherry picked from commit d665003da1)
2013-10-30 11:47:28 +00:00
Marian Neagul
713a4f4ba5 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.
2013-10-23 09:54:06 +01:00
Jim Fehlig
549f964985 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
2013-10-23 09:53:52 +01:00
Jim Fehlig
5a27546975 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
../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

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/Makefile.am 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?

Regards,
Jim

>From a0f35945f3127ab70d051101037e821b1759b4bb Mon Sep 17 00:00:00 2001
From: Jim Fehlig <jfehlig@suse.com>
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/Makefile.am.  While at it, introduce SECDRIVER_CFLAGS
and use both throughout src/Makefile.am where it makes sense.

Signed-off-by: Jim Fehlig <jfehlig@suse.com>

Conflicts:
	src/Makefile.am: Context
2013-10-23 09:53:19 +01:00
Daniel P. Berrange
31a3086d73 Don't link virt-login-shell against libvirt.so (CVE-2013-4400)
The libvirt.so 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 libvirt-setuid-rpc-client.la 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 <berrange@redhat.com>
(cherry picked from commit 3e2f27e13b)
2013-10-21 14:20:40 +01:00
Daniel P. Berrange
9ab478edad 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
set.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
(cherry picked from commit b7fcc799ad)
2013-10-21 14:20:35 +01:00
Daniel P. Berrange
bd047ba666 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 <berrange@redhat.com>
(cherry picked from commit 8c3586ea75)
2013-10-21 14:20:31 +01:00
Daniel P. Berrange
7faae9d9fa 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 <berrange@redhat.com>
(cherry picked from commit ae53e5d10e)
2013-10-21 14:20:27 +01:00
Daniel P. Berrange
90171893ce 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 <berrange@redhat.com>
(cherry picked from commit 57687fd6bf)
2013-10-21 14:20:22 +01:00
Zhou Yimin
0eb43777db 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 <zhouyimin@huawei.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
(cherry picked from commit 9712c2510e)
2013-10-18 07:23:35 -06:00
Daniel Hansel
ff221ba512 build: Add lxc testcase to dist list
Introduced by commit 3f029fb531 the RPM build
was broken due to a missing LXC textcase.

Signed-off-by: Daniel Hansel <daniel.hansel@linux.vnet.ibm.com>
(cherry picked from commit 6285c17f79)
2013-10-15 15:57:40 +02:00
Ján Tomko
2ba1ad4fb2 Convert uuid to a string before printing it
Introduced by 1fa7946.

https://bugzilla.redhat.com/show_bug.cgi?id=1019023
(cherry picked from commit 15fac93b95)
2013-10-15 13:05:57 +02:00
Ján Tomko
4fc4467712 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:
https://bugzilla.redhat.com/show_bug.cgi?id=1015689
(cherry picked from commit 3f029fb531)
2013-10-15 13:05:40 +02:00
Michal Privoznik
aa24b068fd 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 3e8343e151)
2013-10-15 13:05:23 +02:00
Liuji (Jeremy)
7b113d8a99 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.

https://bugzilla.redhat.com/show_bug.cgi?id=1006710

Signed-off-by: Liuji (Jeremy) <jeremy.liu@huawei.com>
(cherry picked from commit ef5d51d491)
2013-10-15 13:05:06 +02:00
Ján Tomko
184af2b574 Free slicename in virSystemdCreateMachine
https://bugzilla.redhat.com/show_bug.cgi?id=1008619

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 09b48562aa)
2013-10-15 13:04:38 +02:00
Jiri Denemark
c93f79290d virsh domjobinfo: Do not return 1 if job is NONE
https://bugzilla.redhat.com/show_bug.cgi?id=1006864

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 f084caae7c)
2013-10-15 13:04:06 +02:00
Claudio Bley
b07d45bde1 Adjust legacy max payload size to account for header information
Commit 27e81517a8 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 eb635de1fe.

(cherry picked from commit 609eb987c6)
2013-10-07 12:53:25 +01:00
Daniel P. Berrange
177952a75f 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 <berrange@redhat.com>
(cherry picked from commit 27e81517a8)
2013-10-01 13:57:57 +01:00
Daniel P. Berrange
841284a048 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

  virConnectDomainEventDeregisterAny

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 <berrange@redhat.com>
(cherry picked from commit 8294aa0c17)
2013-09-27 16:50:16 +01:00
Martin Kletzander
b6ea7abcf7 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.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1009886
(cherry picked from commit 484cc3217b)
2013-09-25 10:49:53 -05:00
Daniel P. Berrange
2a32bbbfb1 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 db7a5688c0
  Author: Daniel P. Berrange <berrange@redhat.com>
  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 <berrange@redhat.com>
(cherry picked from commit e4697b92ab)
2013-09-23 14:13:11 -06:00
Simone Gotti
a61ad1bce4 virsh: add missing "async" option in opts_block_commit
After commit 8aecd35126 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 <eblake@redhat.com>
(cherry picked from commit fe64499dd1)
2013-09-19 13:50:11 -06:00
Daniel P. Berrange
10d159fee2 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 158ba8730e
  Author: Daniel P. Berrange <berrange@redhat.com>
  Date:   Wed Apr 13 16:21:35 2011 +0100

    Merge all returns paths from dispatcher into single path

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
(cherry picked from commit e7f400a110)
2013-09-18 13:21:24 -06:00
Daniel P. Berrange
8616dc8b4f 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 <walters@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
(cherry picked from commit 922b7fda77)
2013-09-18 15:45:53 +01:00
Daniel P. Berrange
606ceb41be 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 <berrange@redhat.com>
(cherry picked from commit e65667c0c6)
2013-09-18 15:45:48 +01:00