9363 Commits

Author SHA1 Message Date
Guido Günther
777e19cb90 Don't depend on syslog.service
Syslog is socket activated since at least systemd v35 so we can drop
this dependency. Debian's linitian otherwise complains about it.

References:

    http://www.freedesktop.org/wiki/Software/systemd/syslog/
    http://lintian.debian.org/tags/systemd-service-file-refers-to-obsolete-target.html

(cherry picked from commit 3c9e40a1e8762f039874aab5d181864de79be069)
2013-11-20 09:04:06 -05:00
Jeremy Fitzhardinge
e51fa6486d libxl: fix dubious cpumask handling in libxlDomainSetVcpuAffinities
Rather than casting the virBitmap pointer to uint8_t* and then using
the structure contents as a byte array, use the virBitmap API to determine
the bitmap size and test each bit.

Signed-off-by: Jeremy Fitzhardinge <jeremy@goop.org>
(cherry picked from commit ba1bf10063a0205c1de12b209b0282833710214f)
2013-11-17 18:02:46 -05:00
Laine Stump
9265a89ae8 util: recognize SMB/CIFS filesystems as shared
This should resolve:

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

libvirt previously recognized NFS, GFS2, OCFS2, and AFS filesystems as
"shared", and thus eligible for exceptions to certain rules/actions
about chowning image files before handing them off to a guest. This
patch widens the definition of "shared filesystem" to include SMB and
CIFS filesystems (aka "Windows file sharing"); both of these use the
same protocol, but different drivers so there are different magic
numbers for each.

(cherry picked from commit e4e73337e5a5aa708bb356751404ab8ae6583f42)
2013-11-17 18:02:40 -05:00
Ján Tomko
b2c87be92e 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 b7829f959b33c6e32422222a9ed745c0da7dc696)
2013-11-13 11:02:20 +01:00
Michal Privoznik
11253a09f2 qemuSetupMemoryCgroup: Handle hard_limit properly
Since 16bcb3 we have a regression. The hard_limit is set
unconditionally. By default the limit is zero. Hence, if user hasn't
configured any, we set the zero in cgroup subsystem making the kernel
kill the corresponding qemu process immediately. The proper fix is to
set hard_limit iff user has configured any.

(cherry picked from commit 94a24dd3a953f415596dd2800287c9010fa51c4c)

Conflicts:
	src/qemu/qemu_cgroup.c
2013-11-06 12:16:25 -05:00
Michal Privoznik
9421fd0010 qemu: Drop qemuDomainMemoryLimit
This function is to guess the correct limit for maximal memory
usage by qemu for given domain. This can never be guessed
correctly, not to mention all the pains and sleepless nights this
code has caused. Once somebody discovers algorithm to solve the
Halting Problem, we can compute the limit algorithmically. But
till then, this code should never see the light of the release
again.

(cherry picked from commit 16bcb3b61675a88bff00317336b9610080c31000)

Conflicts:
	src/qemu/qemu_cgroup.c
	src/qemu/qemu_command.c
	src/qemu/qemu_domain.c
	src/qemu/qemu_domain.h
	src/qemu/qemu_hotplug.c
2013-11-06 11:42:45 -05:00
Zhou Yimin
03ba101aa2 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 9712c2510ec87a87578576a407768380e250a6a4)
2013-10-18 07:56:11 -06:00
Liuji (Jeremy)
cdc148ec5b 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 ef5d51d491356f1f4287aa3a8b908b183b6dd9aa)
2013-10-15 15:17:44 +02:00
Ján Tomko
34ba55e9e2 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 3f029fb5319b9dc9cc2fbf8d1ba4505ee9e4b1e3)

Conflicts:
	src/conf/domain_conf.c
	src/conf/domain_conf.h - missing format
	src/lxc/lxc_container.c - virAsprintf doesn't report OOM errors
	tests/lxcxml2xmltest.c - missing format test
2013-10-15 13:26:55 +02:00
Michal Privoznik
5b2b4d5e63 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)
2013-10-15 13:26:55 +02:00
Daniel P. Berrange
c22cb488de Remove use of virConnectPtr from all remaining nwfilter code
The virConnectPtr is passed around loads of nwfilter code in
order to provide it as a parameter to the callback registered
by the virt drivers. None of the virt drivers use this param
though, so it serves no purpose.

Avoiding the need to pass a virConnectPtr means that the
nwfilterStateReload method no longer needs to open a bogus
QEMU driver connection. This addresses a race condition that
can lead to a crash on startup.

The nwfilter driver starts before the QEMU driver and registers
some callbacks with DBus to detect firewalld reload. If the
firewalld reload happens while the QEMU driver is still starting
up though, the nwfilterStateReload method will open a connection
to the partially initialized QEMU driver and cause a crash.

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

Conflicts:
	src/nwfilter/nwfilter_driver.c
2013-10-07 09:55:30 -04:00
Daniel P. Berrange
0b9f930836 Don't pass virConnectPtr in nwfilter 'struct domUpdateCBStruct'
The nwfilter driver only needs a reference to its private
state object, not a full virConnectPtr. Update the domUpdateCBStruct
struct to have a 'void *opaque' field instead of a virConnectPtr.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
(cherry picked from commit ebca369e3fe5ac999c261c2d44e60a1bac3cfe65)
2013-10-07 09:53:33 -04:00
Daniel P. Berrange
3598addc9e Remove virConnectPtr arg from virNWFilterDefParse*
None of the virNWFilterDefParse* methods require a virConnectPtr
arg, so just drop it

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

Conflicts:
	tests/nwfilterxml2xmltest.c
2013-10-07 09:53:23 -04:00
Michal Privoznik
f22180a383 virNetDevBandwidthEqual: Make it more robust
So far the virNetDevBandwidthEqual() expected both ->in and ->out items
to be allocated for both @a and @b compared. This is not necessary true
for all our code. For instance, running 'update-device' twice over a NIC
with the very same XML results in SIGSEGV-ing in this function.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
(cherry picked from commit ee02fbc8e4a24c1347761ceff2ddb2c108e9611c)
2013-10-07 09:51:48 -04:00
Michal Privoznik
e6f4742b97 qemu_hotplug: Allow QoS update in qemuDomainChangeNet
The qemuDomainChangeNet() is called when 'virsh update-device' is
invoked on a NIC. Currently, we fail to update the QoS even though
we have routines for that.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
(cherry picked from commit 9fa10d3901a14997f724fe50ad8a33d7f0d23abe)
2013-10-07 09:51:48 -04:00
Peter Krempa
eaf803e7f0 qemu: Use "migratable" XML definition when doing external checkpoints
In the original implementation of external checkpoints I've mistakenly
used the live definition to be stored in the save image. The normal
approach is to use the "migratable" definition. This was discovered when
commit 07966f6a8b5ccb5bb4c716b25deb8ba2e572cc67 changed the behavior to
use a converted XML from the user to do the compatibility check to fix
problem when using the regular machine saving.

As the previous patch added a compatibility layer, we can now change the
type of the XML in the image.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1008340
(cherry picked from commit 1b7bfa65e36996fc3a204452d2a844ab9f4b52b3)
2013-10-06 14:46:18 -04:00
Peter Krempa
51a1506b29 qemu: Fix checking of ABI stability when restoring external checkpoints
External checkpoints have a bug in the implementation where they use the
normal definition instead of the "migratable" one. This causes errors
when the snapshot is being reverted using the workaround method via
qemuDomainRestoreFlags() with a custom XML. This issue was introduced
when commit 07966f6a8b5ccb5bb4c716b25deb8ba2e572cc67 changed the code to
compare "migratable" XMLs from the user as we should have used
migratable in the image too.

This patch adds a compatibility layer, so that fixing the snapshot code
won't make existing snapshots fail to load.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1008340
(cherry picked from commit 59898a88ce8431bd3ea249b8789edc2ef9985827)
2013-10-06 14:46:18 -04:00
Daniel P. Berrange
4407b19b83 Include process start time when doing polkit checks
Since PIDs can be reused, polkit prefers to be given
a (PID,start time) pair. If given a PID on its own,
it will attempt to lookup the start time in /proc/pid/stat,
though this is subject to races.

It is safer if the client app resolves the PID start
time itself, because as long as the app has the client
socket open, the client PID won't be reused.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
(cherry picked from commit 979e9c56a7aadf2dcfbddd1abfbad594b78b4468)
Signed-off-by: Eric Blake <eblake@redhat.com>

Conflicts:
	src/rpc/virnetsocket.h - context
	src/util/virprocess.c - needed #include "virstring.h"
	src/util/virstring.c - context
	src/util/virstring.h - context
2013-09-18 12:22:25 -06:00
Michal Privoznik
073812898a qemuDomainChangeGraphics: Check listen address change by listen type
https://bugzilla.redhat.com/show_bug.cgi?id=1006697

Currently, we have a bug when updating a graphics device. A graphics device can
have a listen address set. This address is either defined by user (in which case
it's type is VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS) or it can be inherited
from a network (in which case it's type is
VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK). However, in both cases we have a
listen address to process (e.g. during migration, as I've tried to fix in
7f15ebc7).
Later, when a user tries to update the graphics device (e.g. set a password),
we check if listen addresses match the original as qemu doesn't know how to
change listen address yet. Hence, users are required to not change the listen
address. The implementation then just dumps listen addresses and compare them.
Previously, while dumping the listen addresses, NULL was returned for NETWORK.
After my patch, this is no longer true, and we get a listen address for olddev
even if it is a type of NETWORK. So we have a real string on one side, the NULL
from user's XML on the other side and hence we think user wants to change the
listen address and we refuse it.

Therefore, we must take the type of listen address into account as well.

(cherry picked from commit 752596b5dd0b4f3d27defc9e232552ff5a0bf08a)
2013-09-11 20:27:22 -06:00
Eric Blake
9a1145a987 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 0.10.2.7),
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 <eblake@redhat.com>
(cherry picked from commit 745aa55fbf3e076c4288d5ec3239f5a5d43508a6)
2013-08-29 09:17:31 -06:00
Peter Krempa
1ffdaced5b virbitmap: Refactor virBitmapParse to avoid access beyond bounds of array
The virBitmapParse function was calling virBitmapIsSet() function that
requires the caller to check the bounds of the bitmap without checking
them. This resulted into crashes when parsing a bitmap string that was
exceeding the bounds used as argument.

This patch refactors the function to use virBitmapSetBit without
checking if the bit is set (this function does the checks internally)
and then counts the bits in the bitmap afterwards (instead of keeping
track while parsing the string).

This patch also changes the "parse_error" label to a more common
"error".

The refactor should also get rid of the need to call sa_assert on the
returned variable as the callpath should allow coverity to infer the
possible return values.

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

Thanks to Alex Jia for tracking down the issue. This issue is introduced
by commit 0fc8909.

(cherry picked from commit 47b9127e883677a0d60d767030a147450e919a25)
2013-08-20 14:13:43 +02:00
Ján Tomko
f32ea9937f Set the number of elements 0 in virNetwork*Clear
Decrementing it when it was already 0 causes an invalid free
in virNetworkDefUpdateDNSHost if virNetworkDNSHostDefParseXML
fails and virNetworkDNSHostDefClear gets called twice.

virNetworkForwardDefClear left the number untouched even if it
freed all the elements.
(cherry picked from commit c4e23388e6c7f769e45d1c162f880cd81e4d4d3b)
2013-08-01 17:14:41 +02:00
Ján Tomko
80fffee94b Don't check validity of missing attributes in DNS SRV XML
This fixes a crash if one of them is missing.

https://bugzilla.redhat.com/show_bug.cgi?id=988718
(cherry picked from commit 461fd86a661f32a9aa8044190b2a63b05290332f)
2013-08-01 17:14:27 +02:00
Ján Tomko
90688b148c cgroup: reuse buffer for getline
Reuse the buffer for getline and track buffer allocation
separately from the string length to prevent unlikely
out-of-bounds memory access.

This fixes the following leak that happened when zero bytes were read:

==404== 120 bytes in 1 blocks are definitely lost in loss record 1,344 of 1,671
==404==    at 0x4C2C71B: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==404==    by 0x906F862: getdelim (iogetdelim.c:68)
==404==    by 0x52A48FB: virCgroupPartitionNeedsEscaping (vircgroup.c:1136)
==404==    by 0x52A0FB4: virCgroupPartitionEscape (vircgroup.c:1171)
==404==    by 0x52A0EA4: virCgroupNewDomainPartition (vircgroup.c:1450)
(cherry picked from commit cc7329317fee6088055d7b09594c19f1b8fec5e3)
2013-08-01 17:14:14 +02:00
Wido den Hollander
c636c41af7 rbd: Do not free the secret if it is not set
Not all RBD (Ceph) storage pools have cephx authentication turned on,
so "secret" might not be initialized.

It could also be that the secret couldn't be located.

Only call virSecretFree() if "secret" is initialized earlier.

Signed-off-by: Wido den Hollander <wido@widodh.nl>
(cherry picked from commit d58c8478443d49c6e702bbb2c56a567ef23f036f)
2013-08-01 17:13:58 +02:00
Guannan Ren
146d12c6cc caps: use -device for primary video when qemu >=1.6
libvirt: https://bugzilla.redhat.com/show_bug.cgi?id=986384

qemu: https://bugzilla.redhat.com/show_bug.cgi?id=981094
The commit 0ad9025ef introduce qemu flag QEMU_CAPS_DEVICE_VIDEO_PRIMARY
for using -device VGA, -device cirrus-vga, -device vmware-svga and
-device qxl-vga. In use, for -device qxl-vga, mouse doesn't display
in guest window like the desciption in above bug.
This patch try to use -device for primary video when qemu >=1.6 which
contains the bug fix patch

(cherry picked from commit e3f2686bdf6c94f658d8645c32a6039692753411)

Conflicts:
	src/qemu/qemu_capabilities.c - context with other new capabilities not backported
2013-07-29 16:43:05 -06:00
Eric Blake
ec15df4e85 build: fix virutil build on mingw
https://bugzilla.redhat.com/show_bug.cgi?id=964358

On platforms without decent group support, the build failed:

Cannot export virGetGroupList: symbol not defined
./.libs/libvirt_security_manager.a(libvirt_security_manager_la-security_dac.o): In function `virSecurityDACPreFork':
/home/eblake/libvirt-tmp/build/src/../../src/security/security_dac.c:248: undefined reference to `virGetGroupList'
collect2: error: ld returned 1 exit status

* src/util/virutil.c (virGetGroupList): Provide dummy implementation.

Signed-off-by: Eric Blake <eblake@redhat.com>
(cherry picked from commit cd725c7a9350c8331a6f915127cf7007706f9926)
2013-07-29 16:17:40 -06:00
Eric Blake
aa11d6888f build: work around mingw header pollution
On Fedora 18, when cross-compiling to mingw with the mingw*-dbus
packages installed, compilation fails with:

  CC       libvirt_net_rpc_server_la-virnetserver.lo
In file included from /usr/i686-w64-mingw32/sys-root/mingw/include/dbus-1.0/dbus/dbus-connection.h:32:0,
                 from /usr/i686-w64-mingw32/sys-root/mingw/include/dbus-1.0/dbus/dbus-bus.h:30,
                 from /usr/i686-w64-mingw32/sys-root/mingw/include/dbus-1.0/dbus/dbus.h:31,
                 from ../../src/util/virdbus.h:26,
                 from ../../src/rpc/virnetserver.c:39:
/usr/i686-w64-mingw32/sys-root/mingw/include/dbus-1.0/dbus/dbus-message.h:74:58: error: expected ';', ',' or ')' before 'struct'

I have reported this as a bug against two packages:
- mingw-headers, for polluting the namespace
https://bugzilla.redhat.com/show_bug.cgi?id=980270
- dbus, for not dealing with the pollution
https://bugzilla.redhat.com/show_bug.cgi?id=980278

At least dbus has agreed that a future version of dbus headers will
do s/interface/iface/, regardless of what happens in mingw. But it
is also easy to workaround in libvirt in the meantime, without having
to wait for either mingw or dbus to upgrade.

* src/util/virdbus.h (includes): Undo mingw's pollution so that
dbus doesn't fail.

Signed-off-by: Eric Blake <eblake@redhat.com>
(cherry picked from commit 1528e8b23a0c0014074b06772368e00a17020a79)
2013-07-29 16:13:48 -06:00
Eric Blake
543e5adf05 build: configure must not affect tarball contents
On mingw, configure sets the name of the lxc symfile to
libvirt_lxc.defs rather than libvirt_lxc.syms.  But tarballs
must be arch-independent, regardless of the configure options
used for the tree where we ran 'make dist'.  This led to the
following failure in autobuild.sh:

  CCLD     libvirt-lxc.la
  CCLD     libvirt-qemu.la
/usr/lib64/gcc/i686-w64-mingw32/4.7.2/../../../../i686-w64-mingw32/bin/ld: cannot find libvirt_lxc.def: No such file or directory
collect2: error: ld returned 1 exit status
make[3]: *** [libvirt-lxc.la] Error 1
make[3]: *** Waiting for unfinished jobs....

We were already doing the right thing with libvirt_qemu.syms.

* src/Makefile.am (EXTRA_DIST): Don't ship a built file which
depends on configure for its final name.

Signed-off-by: Eric Blake <eblake@redhat.com>
(cherry picked from commit d79c9273b0724dfd39eaf13bef2953fcea5b8fa4)
2013-07-29 16:13:44 -06:00
Eric Blake
ef10647e2e build: avoid build failure without gnutls
Found while trying to cross-compile to mingw:

  CC       libvirt_driver_remote_la-remote_driver.lo
../../src/remote/remote_driver.c: In function 'doRemoteOpen':
../../src/remote/remote_driver.c:487:23: error: variable 'verify' set but not used [-Werror=unused-but-set-variable]

* src/remote/remote_driver.c (doRemoteOpen): Also ignore 'verify'.

Signed-off-by: Eric Blake <eblake@redhat.com>
(cherry picked from commit 4e6a78e712fc1d649f4e730815f906bf8e14f8a2)
2013-07-29 16:10:55 -06:00
Eric Blake
a10d077ef2 lxc: correctly backport /dev/tty fix
https://bugzilla.redhat.com/show_bug.cgi?id=982317

maint-only patch; reported by Geert Jansen

Commit 17cdc298 tried to backport upstream 90a0c6d, but in
resolving conflicts, failed to account that upstream commit
e1d32bb refactored code to leave off a leading /dev.

* src/lxc/lxc_container.c (lxcContainerPopulateDevices): Use
correct device name.

Signed-off-by: Eric Blake <eblake@redhat.com>
2013-07-29 15:49:22 -06:00
Eric Blake
6598510159 security: fix deadlock with prefork
https://bugzilla.redhat.com/show_bug.cgi?id=964358

Attempts to start a domain with both SELinux and DAC security
modules loaded will deadlock; latent problem introduced in commit
fdb3bde and exposed in commit 29fe5d7.  Basically, when recursing
into the security manager for other driver's prefork, we have to
undo the asymmetric lock taken at the manager level.

Reported by Jiri Denemark, with diagnosis help from Dan Berrange.

* src/security/security_stack.c (virSecurityStackPreFork): Undo
extra lock grabbed during recursion.

Signed-off-by: Eric Blake <eblake@redhat.com>
(cherry picked from commit bfc183c1e377b24cebf5cede4c00f3dc0d1b3486)
2013-07-22 14:04:22 -06:00
Eric Blake
00c2003e25 security_dac: compute supplemental groups before fork
https://bugzilla.redhat.com/show_bug.cgi?id=964358

Commit 75c1256 states that virGetGroupList must not be called
between fork and exec, then commit ee777e99 promptly violated
that for lxc's use of virSecurityManagerSetProcessLabel.  Hoist
the supplemental group detection to the time that the security
manager needs to fork.  Qemu is safe, as it uses
virSecurityManagerSetChildProcessLabel which in turn uses
virCommand to determine supplemental groups.

This does not fix the fact that virSecurityManagerSetProcessLabel
calls virSecurityDACParseIds calls parseIds which eventually
calls getpwnam_r, which also violates fork/exec async-signal-safe
safety rules, but so far no one has complained of hitting
deadlock in that case.

* src/security/security_dac.c (_virSecurityDACData): Track groups
in private data.
(virSecurityDACPreFork): New function, to set them.
(virSecurityDACClose): Clean up new fields.
(virSecurityDACGetIds): Alter signature.
(virSecurityDACSetSecurityHostdevLabelHelper)
(virSecurityDACSetChardevLabel, virSecurityDACSetProcessLabel)
(virSecurityDACSetChildProcessLabel): Update callers.

Signed-off-by: Eric Blake <eblake@redhat.com>
(cherry picked from commit 29fe5d745fbe207ec2415441d4807ae76be05974)

Conflicts:
	src/security/security_dac.c - virSecurityDACSetSecurityUSBLabel needed similar treatment
2013-07-22 14:00:57 -06:00
Eric Blake
689d014686 security: framework for driver PreFork handler
https://bugzilla.redhat.com/show_bug.cgi?id=964358

A future patch wants the DAC security manager to be able to safely
get the supplemental group list for a given uid, but at the time
of a fork rather than during initialization so as to pick up on
live changes to the system's group database.  This patch adds the
framework, including the possibility of a pre-fork callback
failing.

For now, any driver that implements a prefork callback must be
robust against the possibility of being part of a security stack
where a later element in the chain fails prefork.  This means
that drivers cannot do any action that requires a call to postfork
for proper cleanup (no grabbing a mutex, for example).  If this
is too prohibitive in the future, we would have to switch to a
transactioning sequence, where each driver has (up to) 3 callbacks:
PreForkPrepare, PreForkCommit, and PreForkAbort, to either clean
up or commit changes made during prepare.

* src/security/security_driver.h (virSecurityDriverPreFork): New
callback.
* src/security/security_manager.h (virSecurityManagerPreFork):
Change signature.
* src/security/security_manager.c (virSecurityManagerPreFork):
Optionally call into driver, and allow returning failure.
* src/security/security_stack.c (virSecurityDriverStack):
Wrap the handler for the stack driver.
* src/qemu/qemu_process.c (qemuProcessStart): Adjust caller.

Signed-off-by: Eric Blake <eblake@redhat.com>
(cherry picked from commit fdb3bde31ccf8ff172abf00ef5aa974b87af2794)
2013-07-22 11:41:50 -06:00
Eric Blake
7d24a0a226 util: make virSetUIDGID async-signal-safe
https://bugzilla.redhat.com/show_bug.cgi?id=964358

POSIX states that multi-threaded apps should not use functions
that are not async-signal-safe between fork and exec, yet we
were using getpwuid_r and initgroups.  Although rare, it is
possible to hit deadlock in the child, when it tries to grab
a mutex that was already held by another thread in the parent.
I actually hit this deadlock when testing multiple domains
being started in parallel with a command hook, with the following
backtrace in the child:

 Thread 1 (Thread 0x7fd56bbf2700 (LWP 3212)):
 #0  __lll_lock_wait ()
     at ../nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.S:136
 #1  0x00007fd5761e7388 in _L_lock_854 () from /lib64/libpthread.so.0
 #2  0x00007fd5761e7257 in __pthread_mutex_lock (mutex=0x7fd56be00360)
     at pthread_mutex_lock.c:61
 #3  0x00007fd56bbf9fc5 in _nss_files_getpwuid_r (uid=0, result=0x7fd56bbf0c70,
     buffer=0x7fd55c2a65f0 "", buflen=1024, errnop=0x7fd56bbf25b8)
     at nss_files/files-pwd.c:40
 #4  0x00007fd575aeff1d in __getpwuid_r (uid=0, resbuf=0x7fd56bbf0c70,
     buffer=0x7fd55c2a65f0 "", buflen=1024, result=0x7fd56bbf0cb0)
     at ../nss/getXXbyYY_r.c:253
 #5  0x00007fd578aebafc in virSetUIDGID (uid=0, gid=0) at util/virutil.c:1031
 #6  0x00007fd578aebf43 in virSetUIDGIDWithCaps (uid=0, gid=0, capBits=0,
     clearExistingCaps=true) at util/virutil.c:1388
 #7  0x00007fd578a9a20b in virExec (cmd=0x7fd55c231f10) at util/vircommand.c:654
 #8  0x00007fd578a9dfa2 in virCommandRunAsync (cmd=0x7fd55c231f10, pid=0x0)
     at util/vircommand.c:2247
 #9  0x00007fd578a9d74e in virCommandRun (cmd=0x7fd55c231f10, exitstatus=0x0)
     at util/vircommand.c:2100
 #10 0x00007fd56326fde5 in qemuProcessStart (conn=0x7fd53c000df0,
     driver=0x7fd55c0dc4f0, vm=0x7fd54800b100, migrateFrom=0x0, stdin_fd=-1,
     stdin_path=0x0, snapshot=0x0, vmop=VIR_NETDEV_VPORT_PROFILE_OP_CREATE,
     flags=1) at qemu/qemu_process.c:3694
 ...

The solution is to split the work of getpwuid_r/initgroups into the
unsafe portions (getgrouplist, called pre-fork) and safe portions
(setgroups, called post-fork).

* src/util/virutil.h (virSetUIDGID, virSetUIDGIDWithCaps): Adjust
signature.
* src/util/virutil.c (virSetUIDGID): Add parameters.
(virSetUIDGIDWithCaps): Adjust clients.
* src/util/vircommand.c (virExec): Likewise.
* src/util/virfile.c (virFileAccessibleAs, virFileOpenForked)
(virDirCreate): Likewise.
* src/security/security_dac.c (virSecurityDACSetProcessLabel):
Likewise.
* src/lxc/lxc_container.c (lxcContainerSetID): Likewise.
* configure.ac (AC_CHECK_FUNCS_ONCE): Check for setgroups, not
initgroups.

Signed-off-by: Eric Blake <eblake@redhat.com>
(cherry picked from commit ee777e994927ed5f2d427fbc5a53cbe8b5969bda)

Conflicts:
	src/lxc/lxc_container.c - did not use setUIDGID before 1.1.0
	src/util/virutil.c - oom handling changes not backported
	src/util/virfile.c - functions still lived in virutil.c this far back
	configure.ac - context with previous commit
2013-07-22 11:40:26 -06:00
Eric Blake
fcdaa3df33 util: add virGetGroupList
https://bugzilla.redhat.com/show_bug.cgi?id=964358

Since neither getpwuid_r() nor initgroups() are safe to call in
between fork and exec (they obtain a mutex, but if some other
thread in the parent also held the mutex at the time of the fork,
the child will deadlock), we have to split out the functionality
that is unsafe.  At least glibc's initgroups() uses getgrouplist
under the hood, so the ideal split is to expose getgrouplist for
use before a fork.  Gnulib already gives us a nice wrapper via
mgetgroups; we wrap it once more to look up by uid instead of name.

* bootstrap.conf (gnulib_modules): Add mgetgroups.
* src/util/virutil.h (virGetGroupList): New declaration.
* src/util/virutil.c (virGetGroupList): New function.
* src/libvirt_private.syms (virutil.h): Export it.

Signed-off-by: Eric Blake <eblake@redhat.com>
(cherry picked from commit 75c125641ac73473ba4b0542524d67a184769c8e)

Conflicts:
	bootstrap.conf - not updating gnulib submodule...
	configure.ac - ...so checking for getgrouplist by hand...
	src/util/virutil.c - ...and copying only the getgrouplist implementation rather than calling the gnulib function
2013-07-22 10:25:48 -06:00
Eric Blake
3a2d5e7e70 util: improve user lookup helper
https://bugzilla.redhat.com/show_bug.cgi?id=964358

A future patch needs to look up pw_gid; but it is wasteful
to crawl through getpwuid_r twice for two separate pieces
of information, and annoying to copy that much boilerplate
code for doing the crawl.  The current internal-only
virGetUserEnt is also a rather awkward interface; it's easier
to just design it to let callers request multiple pieces of
data as needed from one traversal.

And while at it, I noticed that virGetXDGDirectory could deref
NULL if the getpwuid_r lookup fails.

* src/util/virutil.c (virGetUserEnt): Alter signature.
(virGetUserDirectory, virGetXDGDirectory, virGetUserName): Adjust
callers.

Signed-off-by: Eric Blake <eblake@redhat.com>
(cherry picked from commit c1983ba4e3902308054e961fcae75cece73ef4ba)

Conflicts:
	src/util/virutil.c - oom reporting/strdup changes not backported
2013-07-19 17:14:34 -06:00
Ján Tomko
efa2739be6 qemu: fix double free in qemuMigrationPrepareDirect
Remove assignment of the string freed by virURIFree
to hostname, since it's not used anywhere.

Double free introduced by ddf8ad8, useless code
introduced by f03dcc5.

https://bugzilla.redhat.com/show_bug.cgi?id=977961
(cherry picked from commit 5744d96f211160d406ec0498c2f814a67d1a3fc8)
2013-07-12 09:27:04 +02:00
Laine Stump
b5b269cbc5 pci: initialize virtual_functions array pointer to avoid segfault
This fixes https://bugzilla.redhat.com/show_bug.cgi?id=971325

The problem was that if virPCIGetVirtualFunctions was given the name
of a non-existent interface, it would return to its caller without
initializing the pointer to the array of virtual functions to NULL,
and the caller (virNetDevGetVirtualFunctions) would try to VIR_FREE()
the invalid pointer.

The final error message before the crash would be:

 virPCIGetVirtualFunctions:2088 :
  Failed to open dir '/sys/class/net/eth2/device':
  No such file or directory

In this patch I move the initialization in virPCIGetVirtualFunctions()
to the begining of the function, and also do an explicit
initialization in virNetDevGetVirtualFunctions, just in case someone
in the future adds code into that function prior to the call to
virPCIGetVirtualFunctions.

(cherry picked from commit 2c2525ab6a6f0ad5d75a6c60711e2e28cb1cebe9)
2013-07-11 17:49:19 -04:00
Ján Tomko
a717d7ba0f qemu: check if block I/O limits fit into long long
We can only pass values up to LLONG_MAX through JSON
and QEMU checks if the int64_t number is not negative
at startup since 1.5.0.

https://bugzilla.redhat.com/show_bug.cgi?id=974010
(cherry picked from commit d3c87884927e6b745d0a5680e6d94a40c4f484f7)
2013-07-11 17:48:44 -04:00
Laine Stump
22d9d9a28f network: increase max number of routes
This fixes the problem reported in:

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

When checking for a collision of a new libvirt network's subnet with
any existing routes, we read all of /proc/net/route into memory, then
parse all the entries. The function that we use to read this file
requires a "maximum length" parameter, which had previously been set
to 64*1024. As each line in /proc/net/route is 128 bytes, this would
allow for a maximum of 512 entries in the routing table.

This patch increases that number to 128 * 100000, which allows for
100,000 routing table entries. This means that it's possible that 12MB
would be allocated, but that would only happen if there really were
100,000 route table entries on the system, it's only held for a very
short time.

Since there is no method of specifying and unlimited max (and that
would create a potential denial of service anyway) hopefully this
limit is large enough to accomodate everyone.

(cherry picked from commit 2bdf548f5f0f709c9dec8ba1872e4fcaf8c25fbf)
2013-07-11 17:47:58 -04:00
Ján Tomko
0943603d9e qemu: allow restore with non-migratable XML input
Convert input XML to migratable before using it in
qemuDomainSaveImageOpen.

XML in the save image is migratable, i.e. doesn't contain implicit
controllers. If these controllers were in a non-default order in the
input XML, the ABI check would fail. Removing and re-adding these
controllers fixes it.

https://bugzilla.redhat.com/show_bug.cgi?id=834196
(cherry picked from commit 07966f6a8b5ccb5bb4c716b25deb8ba2e572cc67)
2013-07-11 17:47:39 -04:00
Michal Privoznik
ca305a756d qemu_migrate: Dispose listen address if set from config
https://bugzilla.redhat.com/show_bug.cgi?id=971485

As of d7f9d827531bc843b7c5aa9d3e8c08738a1de248 we copy the listen
address from the qemu.conf config file in case none has been provided
via XML. But later, when migrating, we should not include such listen
address in the migratable XML as it is something autogenerated, not
requested by user. Moreover, the binding to the listen address will
likely fail, unless the address is '0.0.0.0' or its IPv6 equivalent.
This patch introduces a new boolean attribute to virDomainGraphicsListenDef
to distinguish autofilled listen addresses. However, we must keep the
attribute over libvirtd restarts, so it must be kept within status XML.

(cherry picked from commit 6546017c50c104d0b9867137b64ab1f4a312e436)
2013-07-11 17:47:18 -04:00
Ján Tomko
3b6bc5eda6 iscsi: pass hostnames to iscsiadm instead of resolving them
iscsiadm now supports specifying hostnames in the portal argument [1]

Instead of resolving the hostname to a single IPv4 address, pass the
hostname to isciadm, allowing IPv6 targets to work.

[1] https://bugzilla.redhat.com/show_bug.cgi?id=624437

(cherry picked from commit cbdb3c7326f7298d57233d835a5966df2e359d99)
2013-07-11 17:46:53 -04:00
Osier Yang
818a933141 qemu: Report the offset from host UTC for RTC_CHANGE event
https://bugzilla.redhat.com/show_bug.cgi?id=964177

Though both libvirt and QEMU's document say RTC_CHANGE returns
the offset from the host UTC, qemu actually returns the offset
from the specified date instead when specific date is provided
(-rtc base=$date).

It's not safe for qemu to fix it in code, it worked like that
for 3 years, changing it now may break other QEMU use cases.
What qemu tries to do is to fix the document:

http://lists.gnu.org/archive/html/qemu-devel/2013-05/msg04782.html

And in libvirt side, instead of replying on the value from qemu,
this converts the offset returned from qemu to the offset from
host UTC, by:

  /*
   * a: the offset from qemu RTC_CHANGE event
   * b: The specified date (-rtc base=$date)
   * c: the host date when libvirt gets the RTC_CHANGE event
   * offset: What libvirt will report
   */

  offset = a + (b - c);

The specified date (-rtc base=$date) is recorded in clock's def as
an internal only member (may be useful to exposed outside?).

Internal only XML tag "basetime" is introduced to not lose the
guest's basetime after libvirt restarting/reloading:

<clock offset='variable' adjustment='304' basis='utc' basetime='1370423588'/>

(cherry picked from commit e31b5cf393857a6ca78d148c19393e28dfb39de1)
2013-07-11 17:46:45 -04:00
Peter Krempa
44758cea54 storage: Provide better error message if metadata pre-alloc is unsupported
Instead of a unknown flag error report that metadata pre-allocation is
not supported with the requested volume creation method.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=969982
(cherry picked from commit 77e7f69c3b88ae0a7364fa88334a483a247d7200)
2013-07-11 17:46:32 -04:00
Martin Kletzander
ffe68a85c9 usb: don't spoil decimal addresses
For USB devices, dev->name gets formated as %.3o:%.3o even though the
numbers are decimal.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=967712
(cherry picked from commit dc0c7915697a902d6edc5820f5930653b7684c3c)
2013-07-11 17:45:55 -04:00
John Ferlan
e01eedf9bf Check for existence of interface prior to setting terminate flag
https://bugzilla.redhat.com/show_bug.cgi?id=903480

During domain destruction it's possible that the learnIPAddressThread has
already removed the interface prior to the teardown filter path being run.
The teardown code would only be telling the thread to terminate.

(cherry picked from commit 64919d978e4cc910a4475f77c0f04b866051935d)
2013-07-11 17:45:32 -04:00
Peter Krempa
b14ced3932 qemu: snapshot: Don't kill access to disk if snapshot creation fails
If snapshot creation failed for example due to invalid use of the
"REUSE_EXTERNAL" flag, libvirt killed access to the original image file
instead of the new image file. On machines with selinux this kills the
whole VM as the selinux context is enforced immediately.

* qemu_driver.c:qemuDomainSnapshotUndoSingleDiskActive():
    - Kill access to the new image file instead of the old one.

Partially resolves: https://bugzilla.redhat.com/show_bug.cgi?id=906639

(cherry picked from commit 177046753fa427609f217c319dba6fa465a4add4)
2013-07-11 17:45:22 -04:00
Martin Kletzander
b2c2dd60f1 Fix blkdeviotune for shutoff domain
Function qemuDomainSetBlockIoTune() was checking QEMU capabilities
even when !(flags & VIR_DOMAIN_AFFECT_LIVE) and the domain was
shutoff, resulting in the following problem:

 virsh # domstate asdf; blkdeviotune asdf vda --write-bytes-sec 100
 shut off

 error: Unable to change block I/O throttle
 error: unsupported configuration: block I/O throttling not supported with this QEMU binary

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=965016
(cherry picked from commit 5af3ce827767707097affc5e001eab8cbabc28a7)
2013-07-11 17:45:08 -04:00