libvirt.git
3 years agoCVE-2015-0236: qemu: Check ACLs when dumping security info from snapshots
Peter Krempa [Tue, 20 Jan 2015 16:01:01 +0000]
CVE-2015-0236: qemu: Check ACLs when dumping security info from snapshots

The ACL check didn't check the VIR_DOMAIN_XML_SECURE flag and the
appropriate permission for it. Found via code inspection while fixing
permissions for save images.

(cherry picked from commit b347c0c2a321ec5c20aae214927949832a288c5a)

3 years agoCVE-2015-0236: qemu: Check ACLs when dumping security info from save image
Peter Krempa [Tue, 20 Jan 2015 16:01:01 +0000]
CVE-2015-0236: qemu: Check ACLs when dumping security info from save image

The ACL check didn't check the VIR_DOMAIN_XML_SECURE flag and the
appropriate permission for it.

(cherry picked from commit 03c3c0c874c84dfa51ef17556062b095c6e1c0a3)

3 years agoqemu: migration: Unlock vm on failed ACL check in protocol v2 APIs
Peter Krempa [Mon, 8 Dec 2014 18:25:21 +0000]
qemu: migration: Unlock vm on failed ACL check in protocol v2 APIs

Avoid leaving the domain locked on a failed ACL check in
qemuDomainMigratePerform() and qemuDomainMigrateFinish2().

Introduced in commit abf75aea247e (Add ACL checks into the QEMU driver).

(cherry picked from commit 2bdcd29c713dfedd813c89f56ae98f6f3898313d)

3 years agoFix crash in virsystemdtest with dbus 1.7.6
Cédric Bosdonnat [Wed, 18 Dec 2013 17:33:44 +0000]
Fix crash in virsystemdtest with dbus 1.7.6

D-bus introduced some changes in its locking code. Overriding the init
function skips the new locking init and thus crashes later in libvirt
test. Removing the function makes the test pass again.

(cherry picked from commit 5e397d9c0f2ce7d464f8e7e6a598d1c955dfc04a)

3 years agotests: don't fail with newer gnutls
Martin Kletzander [Fri, 2 May 2014 07:55:52 +0000]
tests: don't fail with newer gnutls

gnutls-3.3.0 and newer leaves 2 FDs open in order to be backwards
compatible when it comes to chrooted binaries [1].  Linking
commandhelper with gnutls then leaves these two FDs open and
commandtest fails thanks to that.  This patch does not link
commandhelper with libvirt.la, but rather only the utilities making
the test pass.

Based on suggestion from Daniel [2].

[1] http://lists.gnutls.org/pipermail/gnutls-help/2014-April/003429.html
[2] https://www.redhat.com/archives/libvir-list/2014-April/msg01119.html

Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
(cherry picked from commit 4cbc15d037e1cd8abf5c4aa6acc30d83ae13e34d)

3 years agomaint: cleanup detection of const'ness of selinux ctx
Jim Fehlig [Wed, 28 May 2014 19:48:21 +0000]
maint: cleanup detection of const'ness of selinux ctx

Commit 292d3f2d fixed the build with libselinux 2.3, but missed
some suggestions by eblake

https://www.redhat.com/archives/libvir-list/2014-May/msg00977.html

This patch changes the macro introduced in 292d3f2d to either be
empty in the case of newer libselinux, or contain 'const' in the
case of older libselinux.  The macro is then used directly in
tests/securityselinuxhelper.c.

(cherry picked from commit b109c097654c4fe003e8535481191f37a35d5d7b)

3 years agobuild: fix build with libselinux 2.3
Cédric Bosdonnat [Wed, 28 May 2014 12:44:08 +0000]
build: fix build with libselinux 2.3

Several function signatures changed in libselinux 2.3, now taking
a 'const char *' instead of 'security_context_t'.  The latter is
defined in selinux/selinux.h as

  typedef char *security_context_t;

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

3 years agoutil: eliminate "use after free" in callers of virNetDevLinkDump
Laine Stump [Wed, 15 Oct 2014 22:49:01 +0000]
util: eliminate "use after free" in callers of virNetDevLinkDump

virNetDevLinkDump() gets a message from netlink into "resp", then
calls nlmsg_parse() to fill the table "tb" with pointers into resp. It
then returns tb to its caller, but not before freeing the buffer at
resp. That means that all the callers of virNetDevLinkDump() are
examining memory that has already been freed. This can be verified by
filling the buffer at resp with garbage prior to freeing it (or, I
suppose, just running libvirtd under valgrind) then performing some
operation that calls virNetDevLinkDump().

The upstream commit log incorrectly states that the code has been like
this ever since virNetDevLinkDump() was written. In reality, the
problem was introduced with commit e95de74d, first in libvirt-1.0.5,
which was attempting to eliminate a typecast that caused compiler
warnings. It has only been pure luck (or maybe a lack of heavy load,
and/or maybe an allocation algorithm in malloc() that delays re-use of
just-freed memory) that has kept this from causing errors, for example
when configuring a PCI passthrough or macvtap passthrough network
interface.

The solution taken in this patch is the simplest - just return resp to
the caller along with tb, then have the caller free it after they are
finished using the data (pointers) in tb. I alternately could have
made a cleaner interface by creating a new struct that put tb and resp
together along with a vir*Free() function for it, but this function is
only used in a couple places, and I'm not sure there will be
additional new uses of virNetDevLinkDump(), so the value of adding a
new type, extra APIs, etc. is dubious.

(cherry picked from commit f9f9699f40729556238b905f67a7d6f68c084f6a)

Conflicts:
src/util/virnetdevvportprofile.c - change in cleanup label indentation

3 years agoCVE-2014-7823: dumpxml: security hole with migratable flag
Eric Blake [Sat, 1 Nov 2014 04:14:07 +0000]
CVE-2014-7823: dumpxml: security hole with migratable flag

Commit 28f8dfd (v1.0.0) introduced a security hole: in at least
the qemu implementation of virDomainGetXMLDesc, the use of the
flag VIR_DOMAIN_XML_MIGRATABLE (which is usable from a read-only
connection) triggers the implicit use of VIR_DOMAIN_XML_SECURE
prior to calling qemuDomainFormatXML.  However, the use of
VIR_DOMAIN_XML_SECURE is supposed to be restricted to read-write
clients only.  This patch treats the migratable flag as requiring
the same permissions, rather than analyzing what might break if
migratable xml no longer includes secret information.

Fortunately, the information leak is low-risk: all that is gated
by the VIR_DOMAIN_XML_SECURE flag is the VNC connection password;
but VNC passwords are already weak (FIPS forbids their use, and
on a non-FIPS machine, anyone stupid enough to trust a max-8-byte
password sent in plaintext over the network deserves what they
get).  SPICE offers better security than VNC, and all other
secrets are properly protected by use of virSecret associations
rather than direct output in domain XML.

* src/remote/remote_protocol.x (REMOTE_PROC_DOMAIN_GET_XML_DESC):
Tighten rules on use of migratable flag.
* src/libvirt-domain.c (virDomainGetXMLDesc): Likewise.

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

Conflicts:
src/libvirt-domain.c - file split from older src/libvirt.c; context with older virLibConnError
Signed-off-by: Eric Blake <eblake@redhat.com>

3 years agodomain_conf: fix domain deadlock
Pavel Hrdina [Mon, 22 Sep 2014 16:19:07 +0000]
domain_conf: fix domain deadlock

If you use public api virConnectListAllDomains() with second parameter
set to NULL to get only the number of domains you will lock out all
other operations with domains.

Introduced by commit 2c680804.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
(cherry picked from commit fc22b2e74890873848b43fffae43025d22053669)

3 years agoCVE-2014-3633: qemu: blkiotune: Use correct definition when looking up disk
Peter Krempa [Thu, 11 Sep 2014 14:35:53 +0000]
CVE-2014-3633: qemu: blkiotune: Use correct definition when looking up disk

Live definition was used to look up the disk index while persistent one
was indexed leading to a crash in qemuDomainGetBlockIoTune. Use the
correct def and report a nice error.

Unfortunately it's accessible via read-only connection, though it can
only crash libvirtd in the cases where the guest is hot-plugging disks
without reflecting those changes to the persistent definition.  So
avoiding hotplug, or doing hotplug where persistent is always modified
alongside live definition, will avoid the out-of-bounds access.

Introduced in: eca96694a7f992be633d48d5ca03cedc9bbc3c9aa (v0.9.8)
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1140724
Reported-by: Luyao Huang <lhuang@redhat.com>
Signed-off-by: Peter Krempa <pkrempa@redhat.com>

(cherry picked from commit 3e745e8f775dfe6f64f18b5c2fe4791b35d3546b)

3 years agobuild: fix 'make check' with newer git
Eric Blake [Mon, 6 Jan 2014 21:33:08 +0000]
build: fix 'make check' with newer git

Newer git doesn't like the maint.mk rule 'public-submodule-commit'
that was associated with the version of maint.mk in our checkout
of gnulib.  I tracked down that libvirt commit 8531301 picked up
a gnulib fix that makes git happy.  Basically, that libvirt commit
uses maint.mk from gnulib.git commit d18d1b802.  Rather than try
and backport .gnulib submodule changes, this is a downstream
variant that uses just the newer maint.mk from the known-good
submodule state, as follows:

mkdir -p gnulib/local/top
cd .gnulib
git checkout d18d1b802 top/maint.mk
git diff HEAD > ../gnulib/local/top/maint.mk.diff
git reset --hard
cd ..
git add gnulib/local/top

Signed-off-by: Eric Blake <eblake@redhat.com>

3 years agoqemu: copy: Accept 'format' parameter when copying to a non-existing img
Peter Krempa [Tue, 1 Jul 2014 11:52:51 +0000]
qemu: copy: Accept 'format' parameter when copying to a non-existing img

We have the following matrix of possible arguments handled by the logic
statement touched by this patch:
       | flags & _REUSE_EXT | !(flags & _REUSE_EXT)
-------+--------------------+----------------------
 format| (1)                | (2)
-------+--------------------+----------------------
!format| (3)                | (4)
-------+--------------------+----------------------

In cases 1 and 2 the user provided a format, in cases 3 and 4 not. The
user requests to use a pre-existing image in 1 and 3 and libvirt will
create a new image in 2 and 4.

The difference between cases 3 and 4 is that for 3 the format is probed
from the user-provided image, whereas in 4 we just use the existing disk
format.

The current code would treat cases 1,3 and 4 correctly but in case 2 the
format provided by the user would be ignored.

The particular piece of code was broken in commit 35c7701c64508f975dfeb8
but since it was introduced a few commits before that it was never
released as working.

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

Conflicts:
src/qemu/qemu_driver.c - no refactoring of commits 7b7bf001, 4f20226

3 years agodocs: publish correct enum values
Eric Blake [Wed, 25 Jun 2014 20:54:36 +0000]
docs: publish correct enum values

We publish libvirt-api.xml for others to use, and in fact, the
libvirt-python bindings use it to generate python constants that
correspond to our enum values.  However, we had an off-by-one bug
that any enum that relied on C's rules for implicit initialization
of the first enum member to 0 got listed in the xml as having a
value of 1 (and all later members of the enum were equally
botched).

The fix is simple - since we add one to the previous value when
encountering an enum without an initializer, the previous value
must start at -1 so that the first enum member is assigned 0.

The python generator code has had the off-by-one ever since DV
first wrote it years ago, but most of our public enums were immune
because they had an explicit = 0 initializer.  The only affected
enums are:
- virDomainEventGraphicsAddressType (such as
VIR_DOMAIN_EVENT_GRAPHICS_ADDRESS_IPV4), since commit 987e31e
(libvirt v0.8.0)
- virDomainCoreDumpFormat (such as VIR_DOMAIN_CORE_DUMP_FORMAT_RAW),
since commit 9fbaff0 (libvirt v1.2.3)
- virIPAddrType (such as VIR_IP_ADDR_TYPE_IPV4), since commit
03e0e79 (not yet released)

Thanks to Nehal J Wani for reporting the problem on IRC, and
for helping me zero in on the culprit function.

* docs/apibuild.py (CParser.parseEnumBlock): Fix implicit enum
values.

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

3 years agoqemu: blockcopy: Don't remove existing disk mirror info
Peter Krempa [Wed, 25 Jun 2014 16:11:17 +0000]
qemu: blockcopy: Don't remove existing disk mirror info

When creating a new disk mirror the new struct is stored in a separate
variable until everything went well. The removed hunk would actually
remove existing mirror information for example when the api would be run
if a mirror still exists.

(cherry picked from commit 02b364e186d487f54ed410c01af042f23e812d42)

This fixes a regression introduced in commit ff5f30b.

Signed-off-by: Eric Blake <eblake@redhat.com>

Conflicts:
src/qemu/qemu_driver.c - no refactoring of commits 7b7bf001, 4f20226

4 years agoLSN-2014-0003: Don't expand entities when parsing XML
Daniel P. Berrange [Tue, 15 Apr 2014 10:20:29 +0000]
LSN-2014-0003: Don't expand entities when parsing XML

If the XML_PARSE_NOENT flag is passed to libxml2, then any
entities in the input document will be fully expanded. This
allows the user to read arbitrary files on the host machine
by creating an entity pointing to a local file. Removing
the XML_PARSE_NOENT flag means that any entities are left
unchanged by the parser, or expanded to "" by the XPath
APIs.

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

4 years agoqemu: fix crash when removing <filterref> from interface with update-device
Laine Stump [Thu, 1 May 2014 08:40:41 +0000]
qemu: fix crash when removing <filterref> from interface with update-device

If a domain network interface that contains a <filterref> is modified
"live" using "virsh update-device --live", libvirtd would crash. This
was because the code supporting live update of an interface's
filterref was assuming that a filterref might be added or modified,
but didn't account for removing the filterref, resulting in a null
dereference of the filter name.

Introduced with commit 258fb278, which was first in libvirt v1.0.1.

This addresses https://bugzilla.redhat.com/show_bug.cgi?id=1093301

(cherry picked from commit 0eac9d1e90fc3388030c6109aeb1f4860f108054)

4 years agoqemu: make sure agent returns error when required data are missing
Martin Kletzander [Thu, 3 Apr 2014 05:20:25 +0000]
qemu: make sure agent returns error when required data are missing

Commit 5b3492fa aimed to fix this and caught one error but exposed
another one.  When agent command is being executed and the thread
waiting for the reply is woken up by an event (e.g. EOF in case of
shutdown), the command finishes with no data (rxObject == NULL), but
no error is reported, since this might be desired by the caller
(e.g. suspend through agent).  However, in other situations, when the
data are required (e.g. getting vCPUs), we proceed to getting desired
data out of the reply, but none of the virJSON*() functions works well
with NULLs.  I chose the way of a new parameter for qemuAgentCommand()
function that specifies whether reply is required and behaves
according to that.

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

Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
(cherry picked from commit 736e017e3608ce4c97ee519a293ff7faecea040d)

4 years agoqemu: remove unneeded forward declaration
Martin Kletzander [Wed, 2 Apr 2014 06:57:59 +0000]
qemu: remove unneeded forward declaration

by moving qemuAgentCommand() after qemuAgentCheckError().

Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
(cherry picked from commit e9d09fe19680fcb1810774023aa5c2ef007b10c6)

Conflicts:
src/qemu/qemu_agent.c -- label indentation (5922d05a)

4 years agoqemu: cleanup error checking on agent replies
Martin Kletzander [Tue, 1 Apr 2014 12:58:56 +0000]
qemu: cleanup error checking on agent replies

On all the places where qemuAgentComand() was called, we did a check
for errors in the reply.  Unfortunately, some of the places called
qemuAgentCheckError() without checking for non-null reply which might
have resulted in a crash.

So this patch makes the error-checking part of qemuAgentCommand()
itself, which:

 a) makes it look better,

 b) makes the check mandatory and, most importantly,

 c) checks for the errors if and only if it is appropriate.

This actually fixes a potential crashers when qemuAgentComand()
returned 0, but reply was NULL.  Having said that, it *should* fix the
following bug:

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

Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
(cherry picked from commit 5b3492fadb6bfddd370e263bf8a6953b1b26116f)

4 years agovirNetClientSetTLSSession: Restore original signal mask
Michal Privoznik [Wed, 19 Mar 2014 17:10:34 +0000]
virNetClientSetTLSSession: Restore original signal mask

Currently, we use pthread_sigmask(SIG_BLOCK, ...) prior to calling
poll(). This is okay, as we don't want poll() to be interrupted.
However, then - immediately as we fall out from the poll() - we try to
restore the original sigmask - again using SIG_BLOCK. But as the man
page says, SIG_BLOCK adds signals to the signal mask:

SIG_BLOCK
      The set of blocked signals is the union of the current set and the set argument.

Therefore, when restoring the original mask, we need to completely
overwrite the one we set earlier and hence we should be using:

SIG_SETMASK
      The set of blocked signals is set to the argument set.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
(cherry picked from commit 3d4b4f5ac634c123af1981084add29d3a2ca6ab0)

4 years agoqemu: Avoid operations on NULL monitor if VM fails early
Peter Krempa [Tue, 14 Jan 2014 18:13:30 +0000]
qemu: Avoid operations on NULL monitor if VM fails early

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

If a VM dies very early during an attempted connect to the guest agent
while the locks are down the domain monitor object will be freed. The
object is then accessed later as any failure during guest agent startup
isn't considered fatal.

In the current upstream version this doesn't lead to a crash as
virObjectLock called when entering the monitor in
qemuProcessDetectVcpuPIDs checks the pointer before attempting to
dereference (lock) it. The NULL pointer is then caught in the monitor
helper code.

Before the introduction of virObjectLockable - observed on 0.10.2 - the
pointer is locked directly via virMutexLock leading to a crash.

To avoid this problem we need to differentiate between the guest agent
not being present and the VM quitting when the locks were down. The fix
reorganizes the code in qemuConnectAgent to add the check and then adds
special handling to the callers.

(cherry picked from commit b952cbbccafd5ead8b5a70b2608a1d5a7f03b31e)

4 years agoAdd a mutex to serialize updates to firewall
Daniel P. Berrange [Wed, 22 Jan 2014 18:13:30 +0000]
Add a mutex to serialize updates to firewall

The nwfilter conf update mutex previously serialized
updates to the internal data structures for firewall
rules, and updates to the firewall itself. The latter
was recently turned into a read/write lock, and filter
instantiation allowed to proceed in parallel. It was
believed that this was ok, since each filter is created
on a separate iptables/ebtables chain.

It turns out that there is a subtle lock ordering problem
on virNWFilterObjPtr instances. __virNWFilterInstantiateFilter
will hold a lock on the virNWFilterObjPtr it is instantiating.
This in turn invokes virNWFilterInstantiate which then invokes
virNWFilterDetermineMissingVarsRec which then invokes
virNWFilterObjFindByName. This iterates over every single
virNWFilterObjPtr in the list, locking them and checking their
name. So if 2 or more threads try to instantiate a filter in
parallel, they'll all hold 1 lock at the top level in the
__virNWFilterInstantiateFilter method which will cause the
other thread to deadlock in virNWFilterObjFindByName.

The fix is to add an exclusive mutex to serialize the
execution of __virNWFilterInstantiateFilter.

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

4 years agoCVE-2013-6456: Avoid unsafe use of /proc/$PID/root in LXC hotunplug code
Daniel P. Berrange [Thu, 30 Jan 2014 17:58:36 +0000]
CVE-2013-6456: Avoid unsafe use of /proc/$PID/root in LXC hotunplug code

Rewrite multiple hotunplug functions to to use the
virProcessRunInMountNamespace helper. This avoids
risk of a malicious guest replacing /dev with an absolute
symlink, tricking the driver into changing the host OS
filesystem.

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

4 years agoCVE-2013-6456: Avoid unsafe use of /proc/$PID/root in LXC chardev hostdev hotplug
Daniel P. Berrange [Thu, 30 Jan 2014 17:47:39 +0000]
CVE-2013-6456: Avoid unsafe use of /proc/$PID/root in LXC chardev hostdev hotplug

Rewrite lxcDomainAttachDeviceHostdevMiscLive function
to use the virProcessRunInMountNamespace helper. This avoids
risk of a malicious guest replacing /dev with a absolute
symlink, tricking the driver into changing the host OS
filesystem.

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

4 years agoCVE-2013-6456: Avoid unsafe use of /proc/$PID/root in LXC block hostdev hotplug
Daniel P. Berrange [Thu, 30 Jan 2014 17:45:08 +0000]
CVE-2013-6456: Avoid unsafe use of /proc/$PID/root in LXC block hostdev hotplug

Rewrite lxcDomainAttachDeviceHostdevStorageLive function
to use the virProcessRunInMountNamespace helper. This avoids
risk of a malicious guest replacing /dev with a absolute
symlink, tricking the driver into changing the host OS
filesystem.

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

4 years agoCVE-2013-6456: Avoid unsafe use of /proc/$PID/root in LXC USB hotplug
Daniel P. Berrange [Thu, 30 Jan 2014 16:34:19 +0000]
CVE-2013-6456: Avoid unsafe use of /proc/$PID/root in LXC USB hotplug

Rewrite lxcDomainAttachDeviceHostdevSubsysUSBLive function
to use the virProcessRunInMountNamespace helper. This avoids
risk of a malicious guest replacing /dev with a absolute
symlink, tricking the driver into changing the host OS
filesystem.

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

4 years agoCVE-2013-6456: Avoid unsafe use of /proc/$PID/root in LXC disk hotplug
Daniel P. Berrange [Thu, 30 Jan 2014 15:59:20 +0000]
CVE-2013-6456: Avoid unsafe use of /proc/$PID/root in LXC disk hotplug

Rewrite lxcDomainAttachDeviceDiskLive function to use the
virProcessRunInMountNamespace helper. This avoids risk of
a malicious guest replacing /dev with a absolute symlink,
tricking the driver into changing the host OS filesystem.

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

4 years agoCVE-2013-6456: Avoid unsafe use of /proc/$PID/root in LXC shutdown/reboot code
Eric Blake [Tue, 24 Dec 2013 05:55:51 +0000]
CVE-2013-6456: Avoid unsafe use of /proc/$PID/root in LXC shutdown/reboot code

Use helper virProcessRunInMountNamespace in lxcDomainShutdownFlags and
lxcDomainReboot.  Otherwise, a malicious guest could use symlinks
to force the host to manipulate the wrong file in the host's namespace.

Idea by Dan Berrange, based on an initial report by Reco
<recoverym4n@gmail.com> at
http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=732394

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

4 years agoAdd helper for running code in separate namespaces
Daniel P. Berrange [Thu, 30 Jan 2014 13:11:23 +0000]
Add helper for running code in separate namespaces

Implement virProcessRunInMountNamespace, which runs callback of type
virProcessNamespaceCallback in a container namespace. This uses a
child process to run the callback, since you can't change the mount
namespace of a thread. This implies that callbacks have to be careful
about what code they run due to async safety rules.

Idea by Dan Berrange, based on an initial report by Reco
<recoverym4n@gmail.com> at
http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=732394

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

4 years agoAdd virFileMakeParentPath helper function
Daniel P. Berrange [Thu, 30 Jan 2014 17:06:39 +0000]
Add virFileMakeParentPath helper function

Add a helper function which takes a file path and ensures
that all directory components leading up to the file exist.
IOW, it strips the filename part of the path and passes
the result to virFileMakePath.

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

4 years agoMove check for cgroup devices ACL upfront in LXC hotplug
Daniel P. Berrange [Wed, 5 Feb 2014 17:48:03 +0000]
Move check for cgroup devices ACL upfront in LXC hotplug

The check for whether the cgroup devices ACL is available is
done quite late during LXC hotplug - in fact after the device
node is already created in the container in some cases. Better
to do it upfront so we fail immediately.

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

4 years agoDisks are always block devices, never character devices
Daniel P. Berrange [Wed, 5 Feb 2014 11:01:09 +0000]
Disks are always block devices, never character devices

The LXC disk hotplug code was allowing block or character devices
to be given as disk. A disk is always a block device.

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

4 years agoFix reset of cgroup when detaching USB device from LXC guests
Daniel P. Berrange [Tue, 4 Feb 2014 17:41:22 +0000]
Fix reset of cgroup when detaching USB device from LXC guests

When detaching a USB device from an LXC guest we must remove
the device from the cgroup ACL. Unfortunately we were telling
the cgroup code to use the guest /dev path, not the host /dev
path, and the guest device node had already been unlinked.
This was, however, fortunate since the code passed &priv->cgroup
instead of priv->cgroup, so would have crash if the device node
were accessible.

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

4 years agoRecord hotplugged USB device in LXC live guest config
Daniel P. Berrange [Tue, 4 Feb 2014 16:46:28 +0000]
Record hotplugged USB device in LXC live guest config

After hotplugging a USB device, the LXC driver forgot
to add the device def to the virDomainDefPtr.

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

4 years agoFix path used for USB device attach with LXC
Daniel P. Berrange [Tue, 4 Feb 2014 16:43:18 +0000]
Fix path used for USB device attach with LXC

The LXC code missed the 'usb' component out of the path
/dev/bus/usb/$BUSNUM/$DEVNUM, so it failed to actually
setup cgroups for the device. This was in fact lucky
because the call to virLXCSetupHostUsbDeviceCgroup
was also mistakenly passing '&priv->cgroup' instead of
just 'priv->cgroup'. So once the path is fixed, libvirtd
would then crash trying to access the bogus virCgroupPtr
pointer. This would have been a security issue, were it
not for the bogus path preventing the pointer reference
being reached.

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

4 years agoDon't block use of USB with containers
Daniel P. Berrange [Tue, 4 Feb 2014 16:21:12 +0000]
Don't block use of USB with containers

virDomainDefCompatibleDevice blocks use of USB if no USB
controller is present. This is not correct for containers
since devices can be assigned directly regardless of any
controllers.

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

4 years agoevent: move event filtering to daemon (regression fix)
Eric Blake [Tue, 28 Jan 2014 21:50:02 +0000]
event: move event filtering to daemon (regression fix)

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

Commit f9f56340 for CVE-2014-0028 almost had the right idea - we
need to check the ACL rules to filter which events to send.  But
it overlooked one thing: the event dispatch queue is running in
the main loop thread, and therefore does not normally have a
current virIdentityPtr.  But filter checks can be based on current
identity, so when libvirtd.conf contains access_drivers=["polkit"],
we ended up rejecting access for EVERY event due to failure to
look up the current identity, even if it should have been allowed.

Furthermore, even for events that are triggered by API calls, it
is important to remember that the point of events is that they can
be copied across multiple connections, which may have separate
identities and permissions.  So even if events were dispatched
from a context where we have an identity, we must change to the
correct identity of the connection that will be receiving the
event, rather than basing a decision on the context that triggered
the event, when deciding whether to filter an event to a
particular connection.

If there were an easy way to get from virConnectPtr to the
appropriate virIdentityPtr, then object_event.c could adjust the
identity prior to checking whether to dispatch an event.  But
setting up that back-reference is a bit invasive.  Instead, it
is easier to delay the filtering check until lower down the
stack, at the point where we have direct access to the RPC
client object that owns an identity.  As such, this patch ends
up reverting a large portion of the framework of commit f9f56340.
We also have to teach 'make check' to special-case the fact that
the event registration filtering is done at the point of dispatch,
rather than the point of registration.  Note that even though we
don't actually use virConnectDomainEventRegisterCheckACL (because
the RegisterAny variant is sufficient), we still generate the
function for the purposes of documenting that the filtering
takes place.

Also note that I did not entirely delete the notion of a filter
from object_event.c; I still plan on using that for my upcoming
patch series for qemu monitor events in libvirt-qemu.so.  In
other words, while this patch changes ACL filtering to live in
remote.c and therefore we have no current client of the filtering
in object_event.c, the notion of filtering in object_event.c is
still useful down the road.

* src/check-aclrules.pl: Exempt event registration from having to
pass checkACL filter down call stack.
* daemon/remote.c (remoteRelayDomainEventCheckACL)
(remoteRelayNetworkEventCheckACL): New functions.
(remoteRelay*Event*): Use new functions.
* src/conf/domain_event.h (virDomainEventStateRegister)
(virDomainEventStateRegisterID): Drop unused parameter.
* src/conf/network_event.h (virNetworkEventStateRegisterID):
Likewise.
* src/conf/domain_event.c (virDomainEventFilter): Delete unused
function.
* src/conf/network_event.c (virNetworkEventFilter): Likewise.
* src/libxl/libxl_driver.c: Adjust caller.
* src/lxc/lxc_driver.c: Likewise.
* src/network/bridge_driver.c: Likewise.
* src/qemu/qemu_driver.c: Likewise.
* src/remote/remote_driver.c: Likewise.
* src/test/test_driver.c: Likewise.
* src/uml/uml_driver.c: Likewise.
* src/vbox/vbox_tmpl.c: Likewise.
* src/xen/xen_driver.c: Likewise.

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

Conflicts:
daemon/remote.c - not backporting network events
src/conf/network_event.c - likewise
src/conf/network_event.h - likewise
src/network/bridge_driver.c - likewise
src/conf/domain_event.c - revert back to pre-CVE state
src/conf/domain_event.h - likewise
src/libxl/libxl_driver.c - likewise
src/lxc/lxc_driver.c - likewise
src/remote/remote_driver.c - likewise
src/test/test_driver.c - likewise
src/uml/uml_driver.c - likewise
src/xen/xen_driver.c - likewise

4 years agoPush nwfilter update locking up to top level
Daniel P. Berrange [Wed, 22 Jan 2014 17:28:29 +0000]
Push nwfilter update locking up to top level

The NWFilter code has as a deadlock race condition between
the virNWFilter{Define,Undefine} APIs and starting of guest
VMs due to mis-matched lock ordering.

In the virNWFilter{Define,Undefine} codepaths the lock ordering
is

  1. nwfilter driver lock
  2. virt driver lock
  3. nwfilter update lock
  4. domain object lock

In the VM guest startup paths the lock ordering is

  1. virt driver lock
  2. domain object lock
  3. nwfilter update lock

As can be seen the domain object and nwfilter update locks are
not acquired in a consistent order.

The fix used is to push the nwfilter update lock upto the top
level resulting in a lock ordering for virNWFilter{Define,Undefine}
of

  1. nwfilter driver lock
  2. nwfilter update lock
  3. virt driver lock
  4. domain object lock

and VM start using

  1. nwfilter update lock
  2. virt driver lock
  3. domain object lock

This has the effect of serializing VM startup once again, even if
no nwfilters are applied to the guest. There is also the possibility
of deadlock due to a call graph loop via virNWFilterInstantiate
and virNWFilterInstantiateFilterLate.

These two problems mean the lock must be turned into a read/write
lock instead of a plain mutex at the same time. The lock is used to
serialize changes to the "driver->nwfilters" hash, so the write lock
only needs to be held by the define/undefine methods. All other
methods can rely on a read lock which allows good concurrency.

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

4 years agoAdd a read/write lock implementation
Daniel P. Berrange [Wed, 22 Jan 2014 15:26:21 +0000]
Add a read/write lock implementation

Add virRWLock backed up by a POSIX rwlock primitive

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

4 years agoevent: filter global events by domain:getattr ACL [CVE-2014-0028]
Eric Blake [Tue, 14 Jan 2014 17:29:34 +0000]
event: filter global events by domain:getattr ACL [CVE-2014-0028]

Ever since ACL filtering was added in commit 7639736 (v1.1.1), a
user could still use event registration to obtain access to a
domain that they could not normally access via virDomainLookup*
or virConnectListAllDomains and friends.  We already have the
framework in the RPC generator for creating the filter, and
previous cleanup patches got us to the point that we can now
wire the filter through the entire object event stack.

Furthermore, whether or not domain:getattr is honored, use of
global events is a form of obtaining a list of networks, which
is covered by connect:search_domains added in a93cd08 (v1.1.0).
Ideally, we'd have a way to enforce connect:search_domains when
doing global registrations while omitting that check on a
per-domain registration.  But this patch just unconditionally
requires connect:search_domains, even when no list could be
obtained, based on the following observations:
1. Administrators are unlikely to grant domain:getattr for one
or all domains while still denying connect:search_domains - a
user that is able to manage domains will want to be able to
manage them efficiently, but efficient management includes being
able to list the domains they can access.  The idea of denying
connect:search_domains while still granting access to individual
domains is therefore not adding any real security, but just
serves as a layer of obscurity to annoy the end user.
2. In the current implementation, domain events are filtered
on the client; the server has no idea if a domain filter was
requested, and must therefore assume that all domain event
requests are global.  Even if we fix the RPC protocol to
allow for server-side filtering for newer client/server combos,
making the connect:serach_domains ACL check conditional on
whether the domain argument was NULL won't benefit older clients.
Therefore, we choose to document that connect:search_domains
is a pre-requisite to any domain event management.

Network events need the same treatment, with the obvious
change of using connect:search_networks and network:getattr.

* src/access/viraccessperm.h
(VIR_ACCESS_PERM_CONNECT_SEARCH_DOMAINS)
(VIR_ACCESS_PERM_CONNECT_SEARCH_NETWORKS): Document additional
effect of the permission.
* src/conf/domain_event.h (virDomainEventStateRegister)
(virDomainEventStateRegisterID): Add new parameter.
* src/conf/network_event.h (virNetworkEventStateRegisterID):
Likewise.
* src/conf/object_event_private.h (virObjectEventStateRegisterID):
Likewise.
* src/conf/object_event.c (_virObjectEventCallback): Track a filter.
(virObjectEventDispatchMatchCallback): Use filter.
(virObjectEventCallbackListAddID): Register filter.
* src/conf/domain_event.c (virDomainEventFilter): New function.
(virDomainEventStateRegister, virDomainEventStateRegisterID):
Adjust callers.
* src/conf/network_event.c (virNetworkEventFilter): New function.
(virNetworkEventStateRegisterID): Adjust caller.
* src/remote/remote_protocol.x
(REMOTE_PROC_CONNECT_DOMAIN_EVENT_REGISTER)
(REMOTE_PROC_CONNECT_DOMAIN_EVENT_REGISTER_ANY)
(REMOTE_PROC_CONNECT_NETWORK_EVENT_REGISTER_ANY): Generate a
filter, and require connect:search_domains instead of weaker
connect:read.
* src/test/test_driver.c (testConnectDomainEventRegister)
(testConnectDomainEventRegisterAny)
(testConnectNetworkEventRegisterAny): Update callers.
* src/remote/remote_driver.c (remoteConnectDomainEventRegister)
(remoteConnectDomainEventRegisterAny): Likewise.
* src/xen/xen_driver.c (xenUnifiedConnectDomainEventRegister)
(xenUnifiedConnectDomainEventRegisterAny): Likewise.
* src/vbox/vbox_tmpl.c (vboxDomainGetXMLDesc): Likewise.
* src/libxl/libxl_driver.c (libxlConnectDomainEventRegister)
(libxlConnectDomainEventRegisterAny): Likewise.
* src/qemu/qemu_driver.c (qemuConnectDomainEventRegister)
(qemuConnectDomainEventRegisterAny): Likewise.
* src/uml/uml_driver.c (umlConnectDomainEventRegister)
(umlConnectDomainEventRegisterAny): Likewise.
* src/network/bridge_driver.c
(networkConnectNetworkEventRegisterAny): Likewise.
* src/lxc/lxc_driver.c (lxcConnectDomainEventRegister)
(lxcConnectDomainEventRegisterAny): Likewise.

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

Conflicts:
src/conf/object_event.c - not backporting event refactoring
src/conf/object_event_private.h - likewise
src/conf/network_event.c - not backporting network events
src/conf/network_event.h - likewise
src/network/bridge_driver.c - likewise
src/access/viraccessperm.h - likewise
src/remote/remote_protocol.x - likewise
src/conf/domain_event.c - includes code that upstream has in object_event
src/conf/domain_event.h - context
src/libxl/libxl_driver.c - context
src/lxc/lxc_driver.c - context
src/remote/remote_driver.c - context, not backporting network events
src/test/test_driver.c - context, not backporting network events
src/uml/uml_driver.c - context
src/xen/xen_driver.c - context

4 years agoFix memory leak in virObjectEventCallbackListRemoveID()
Eric Blake [Tue, 14 Jan 2014 00:30:59 +0000]
Fix memory leak in virObjectEventCallbackListRemoveID()

While running objecteventtest, it was found that valgrind pointed out the
following memory leak:

==13464== 5 bytes in 1 blocks are definitely lost in loss record 7 of 134
==13464==    at 0x4A0887C: malloc (vg_replace_malloc.c:270)
==13464==    by 0x341F485E21: strdup (strdup.c:42)
==13464==    by 0x4CAE28F: virStrdup (virstring.c:554)
==13464==    by 0x4CF3CBE: virObjectEventCallbackListAddID (object_event.c:286)
==13464==    by 0x4CF49CA: virObjectEventStateRegisterID (object_event.c:729)
==13464==    by 0x4CF73FE: virDomainEventStateRegisterID (domain_event.c:1424)
==13464==    by 0x4D7358F: testConnectDomainEventRegisterAny (test_driver.c:6032)
==13464==    by 0x4D600C8: virConnectDomainEventRegisterAny (libvirt.c:19128)
==13464==    by 0x402409: testDomainStartStopEvent (objecteventtest.c:232)
==13464==    by 0x403451: virtTestRun (testutils.c:138)
==13464==    by 0x402012: mymain (objecteventtest.c:395)
==13464==    by 0x403AF2: virtTestMain (testutils.c:593)
==13464==

(cherry picked from commit 34d52b3471a18c72b7a02e27d65857505d858a8e)

Conflicts:
src/conf/object_event.c - 1.2.1 refactoring to object_event not
backported, so change applied directly in older domain_event.c instead

4 years agoReally don't crash if a connection closes early
Jiri Denemark [Mon, 13 Jan 2014 14:46:24 +0000]
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 066c8ef6c18bc1faf8b3e10787b39796a7a06cc0)

4 years agoDon't crash if a connection closes early
Jiri Denemark [Thu, 9 Jan 2014 21:26:40 +0000]
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 173c2914734eb5c32df6d35a82bf503e12261bcf)

4 years agoqemu: Fix job usage in virDomainGetBlockIoTune
Jiri Denemark [Fri, 20 Dec 2013 14:41:04 +0000]
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 3b56425938e2f97208d5918263efa0d6439e4ecd)

4 years agoqemu: Fix job usage in qemuDomainBlockCopy
Jiri Denemark [Fri, 20 Dec 2013 14:08:06 +0000]
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 ff5f30b6bfa317f2a4c33f69289baf4e887eb048)

4 years agoqemu: Fix job usage in qemuDomainBlockJobImpl
Jiri Denemark [Fri, 20 Dec 2013 14:04:09 +0000]
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 f93d2caa070f6197ab50d372d286018b0ba6bbd8)

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

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 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

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 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: =========
 /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 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.

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 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.

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 f8c1cb90213508c4f32549023b0572ed774e48aa)

4 years agostorage: fix omitted slash in gluster volume URI
Eric Blake [Tue, 10 Dec 2013 12:45:26 +0000]
storage: fix omitted slash in gluster volume URI

When doing 'virsh vol-dumpxml' on a gluster pool's volume, the
resulting URI incorrectly omitted a slash between hostname and
path:  gluster://192.168.122.206rhsvol1/fedora-19.img

This is fallout from me rebasing earlier versions of my patch
that ended up as commit efee1af; I had originally played with
always requiring the gluster volume to have a leading slash,
but it was easier to use the gluster API if the gluster volume
name was guaranteed to have no slash.  While I got the URI of
the pool correct, I forgot to fix the URI of a libvirt volume.

* src/storage/storage_backend_gluster.c
(virStorageBackendGlusterRefreshVol): Use correct starting point
since uri construction requires leading slash.

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

4 years agoRelease of libvirt-1.2.0 v1.2.0
Daniel Veillard [Mon, 2 Dec 2013 04:17:51 +0000]
Release of libvirt-1.2.0

* configure.ac docs/news.html.in libvirt.spec.in: update for release
* po/*.po*: update and merge localizations from transifex

4 years agovbox: handle errors of virDomainHostdevDefAlloc correctly
Ryota Ozaki [Sun, 1 Dec 2013 14:46:06 +0000]
vbox: handle errors of virDomainHostdevDefAlloc correctly

The original code ignored errors of virDomainHostdevDefAlloc,
however, we should properly do error return from the function
if it occurs.

The fix pulls out virDomainHostdevDefAlloc from the loop and
executes it all together before the loop. So we can easily
return on errors without the notion of other memory allocations
in the loop.

The deallocation code is separated from the allocation code
because it will be used by a further patch for fixing other error
handlings.

Reported-by: Laine Stump <laine@laine.org>
Signed-off-by: Ryota Ozaki <ozaki.ryota@gmail.com>

4 years agovbox: fix incorrect loop condition in vboxHostDeviceGetXMLDesc
Ryota Ozaki [Sun, 1 Dec 2013 14:46:05 +0000]
vbox: fix incorrect loop condition in vboxHostDeviceGetXMLDesc

The fixed loop used logical OR to combine two conditions, however,
it is apparently incorrect and logical AND is correct.

We can fix it by replacing OR with AND, but this patch instead
fixes the problem by getting rid of the first conditional
statement: USBFilterCount < def->nhostdevs. It isn't needed
because USBFilterCount will never be greater than or equal to
def->nhostdevs.

def->nhostdevs is calculated in the following code
above the loop in question like this:

    for (i = 0; i < deviceFilters.count; i++) {
        PRBool active = PR_FALSE;
        IUSBDeviceFilter *deviceFilter = deviceFilters.items[i];

        deviceFilter->vtbl->GetActive(deviceFilter, &active);
        if (active) {
            def->nhostdevs++;
        }
    }

And the loop is constructed as like this:

    for (i = 0; (USBFilterCount < def->nhostdevs) || (i < deviceFilters.count); i++) {
        PRBool active                  = PR_FALSE;
(snip)
        deviceFilter->vtbl->GetActive(deviceFilter, &active);
        if (!active)
            continue;
(snip)
        USBFilterCount++;
    }

So def->nhostdevs is the number of active device filters and
USBFilterCount is counted up only when a device filter is active.
Thus, we can remove USBFilterCount < def->nhostdevs safely.

Reported-by: Laine Stump <laine@laine.org>
Signed-off-by: Ryota Ozaki <ozaki.ryota@gmail.com>

4 years agodocs: fix typos in libvirt.h.in
Chen Hanxiao [Mon, 2 Dec 2013 02:36:16 +0000]
docs: fix typos in libvirt.h.in

s/caused/causes

Signed-off-by: Chen Hanxiao <chenhanxiao@cn.fujitsu.com>

4 years agoLink libvirt-sandbox from apps page
Daniel P. Berrange [Fri, 29 Nov 2013 16:27:42 +0000]
Link libvirt-sandbox from apps page

Add a link to the http://sandbox.libvirt.org sub-site from the
list of libraries related to libvirt. Also fix formatting for
the ruby libvirt binding.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>

4 years agoAdd docs about audit subsystem logging
Daniel P. Berrange [Fri, 29 Nov 2013 16:23:42 +0000]
Add docs about audit subsystem logging

Adds a new page to the website "Deployment" section describing
what data is sent to the audit logs and how to configure libvirtd
audit settings.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>

4 years agoFix memory leak in virNWFilterDefParseXML()
Nehal J Wani [Fri, 29 Nov 2013 15:28:51 +0000]
Fix memory leak in virNWFilterDefParseXML()

While running nwfilterxml2xmltest, it was found that valgrind pointed out the
following error...

==7466== 16 bytes in 1 blocks are definitely lost in loss record 26 of 90
==7466==    at 0x4A06B6F: calloc (vg_replace_malloc.c:593)
    ==7466==    by 0x4C651AD: virAlloc (viralloc.c:142)
    ==7466==    by 0x4D0450D: virNWFilterDefParseNode (nwfilter_conf.c:2575)
    ==7466==    by 0x4D05D84: virNWFilterDefParse (nwfilter_conf.c:2647)
    ==7466==    by 0x401FDE: testCompareXMLToXMLHelper (nwfilterxml2xmltest.c:39)
    ==7466==    by 0x402DE1: virtTestRun (testutils.c:138)
    ==7466==    by 0x4018E9: mymain (nwfilterxml2xmltest.c:111)
    ==7466==    by 0x403482: virtTestMain (testutils.c:593)
    ==7466==    by 0x341F421A04: (below main) (libc-start.c:225)

...21 times, which are related to 21 tests in nwfilterxml2xmltest.c which sent
EXPECT_WARN = false. There were two scenarios in virNWFilterDefParseXML(),
when the variable 'entry' was malloc'ed, but not freed.

4 years agoFix memory leak in virDomainDefParseXML()
Nehal J Wani [Mon, 2 Dec 2013 02:32:12 +0000]
Fix memory leak in virDomainDefParseXML()

This patch fixes the memory leaks found while running qemuxml2argvtest

==8260== 3 bytes in 1 blocks are definitely lost in loss record 1 of
129
==8260==    at 0x4A0887C: malloc (vg_replace_malloc.c:270)
==8260==    by 0x341F485E21: strdup (strdup.c:42)
==8260==    by 0x4CADCFF: virStrdup (virstring.c:554)
==8260==    by 0x4CBB839: virXPathString (virxml.c:90)
==8260==    by 0x4CE753A: virDomainDefParseXML (domain_conf.c:11478)
==8260==    by 0x4CEB4FE: virDomainDefParseNode (domain_conf.c:12742)
==8260==    by 0x4CEB675: virDomainDefParse (domain_conf.c:12684)
==8260==    by 0x425958: testCompareXMLToArgvHelper (qemuxml2argvtest.c:107)
==8260==    by 0x427111: virtTestRun (testutils.c:138)
==8260==    by 0x41D3FE: mymain (qemuxml2argvtest.c:452)
==8260==    by 0x4277B2: virtTestMain (testutils.c:593)
==8260==    by 0x341F421A04: (below main) (libc-start.c:225)
==8260==
==8260== 4 bytes in 1 blocks are definitely lost in loss record 5 of
129
==8260==    at 0x4A0887C: malloc (vg_replace_malloc.c:270)
==8260==    by 0x341F485E21: strdup (strdup.c:42)
==8260==    by 0x4CADCFF: virStrdup (virstring.c:554)
==8260==    by 0x4CBB839: virXPathString (virxml.c:90)
==8260==    by 0x4CE753A: virDomainDefParseXML (domain_conf.c:11478)
==8260==    by 0x4CEB4FE: virDomainDefParseNode (domain_conf.c:12742)
==8260==    by 0x4CEB675: virDomainDefParse (domain_conf.c:12684)
==8260==    by 0x425958: testCompareXMLToArgvHelper (qemuxml2argvtest.c:107)
==8260==    by 0x427111: virtTestRun (testutils.c:138)
==8260==    by 0x41D39A: mymain (qemuxml2argvtest.c:451)
==8260==    by 0x4277B2: virtTestMain (testutils.c:593)
==8260==    by 0x341F421A04: (below main) (libc-start.c:225)
==8260==

4 years agovirsh: fix doc typos
Nehal J Wani [Sat, 30 Nov 2013 06:33:17 +0000]
virsh: fix doc typos

Fix 6 minor spelling errors in virsh doc

4 years agoFix typos in various docs
Nehal J Wani [Sat, 30 Nov 2013 18:27:15 +0000]
Fix typos in various docs

Fix 8 minor spelling errors in docs/*.html.in

4 years agoLXC: Ensure security context is set when mounting images
Daniel P. Berrange [Fri, 29 Nov 2013 12:19:37 +0000]
LXC: Ensure security context is set when mounting images

When setting up filesystems backed by block devices or file
images, the SELinux mount options must be used to ensure the
correct context is set

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>

4 years agonetwork: properly update iptables rules during net-update
Laine Stump [Wed, 27 Nov 2013 15:07:34 +0000]
network: properly update iptables rules during net-update

This patch resolves:

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

The basic problem is that during a network update, the required
iptables rules sometimes change, and this was being handled by simply
removing and re-adding the rules. However, the removal of the old
rules was done based on the *new* state of the network, which would
mean that some of the rules would not match those currently in the
system, so the old rules wouldn't be removed.

This patch removes the old rules prior to updating the network
definition then adds the new rules as soon as the definition is
updated. Note that this could lead to a stray packet or two during the
interim, but that was already a problem before (the period of limbo is
now just slightly longer).

While moving the location for the rules, I added a few more sections
that should result in the iptables rules being redone:

DHCP_RANGE and DHCP_HOST - these are needed because adding/removing a dhcp
host entry could lead to the dhcp service being started/stopped, which
would require that the mangle rule that fixes up dhcp response
checksums sould need to be added/removed, and this wasn't being done.

4 years agoFix bug in identifying sub-mounts
Daniel P. Berrange [Wed, 27 Nov 2013 15:21:41 +0000]
Fix bug in identifying sub-mounts

The code for extracting sub-mounts would just do a STRPREFIX
check on the mount. This was flawed because if there were
the following mounts

 /etc/aliases
 /etc/aliases.db

and '/etc/aliases' was asked for, it would return both even
though the latter isn't a sub-mount.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>

4 years agoPull lxcContainerGetSubtree out into shared virfile module
Daniel P. Berrange [Wed, 27 Nov 2013 15:19:49 +0000]
Pull lxcContainerGetSubtree out into shared virfile module

Move the code for lxcContainerGetSubtree into the virfile
module creating 2 new functions

  int virFileGetMountSubtree(const char *mtabpath,
                             const char *prefix,
                             char ***mountsret,
                             size_t *nmountsret);
  int virFileGetMountReverseSubtree(const char *mtabpath,
                                    const char *prefix,
                                    char ***mountsret,
                                    size_t *nmountsret);

Add a new virfiletest.c test case to validate the new code.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>

4 years agoIntroduce standard methods for sorting strings with qsort
Daniel P. Berrange [Thu, 28 Nov 2013 11:14:59 +0000]
Introduce standard methods for sorting strings with qsort

Add virStringSortCompare and virStringSortRevCompare as
standard functions to use with qsort.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>

4 years agotests: fix virpcitest with read-only srcdir v1.2.0-rc2
Eric Blake [Wed, 27 Nov 2013 21:59:52 +0000]
tests: fix virpcitest with read-only srcdir

'make distcheck' has been broken since commit 21685c9; basically,
it emulates the case of a read-only $(srcdir) (such as building
from a tarball exploded onto a CD-ROM), but we were creating our
fake pci device as a symlink into $(srcdir) and failing when that
requires opening the config file for writing:

 3) testVirPCIDeviceReset                                             ... libvirt:  error : Failed to open config space file '/sys/bus/pci/devices/0000:00:01.0/config': Permission denied

Fix it by copying rather than symlinking.

* tests/virpcimock.c (make_file): Add parameter to allow binary
creation; adjust all callers.
(pci_device_new_from_stub): Copy rather than symlink.

Signed-off-by: Eric Blake <eblake@redhat.com>

4 years agotests: guarantee abs_srcdir in all C tests
Eric Blake [Wed, 27 Nov 2013 21:31:53 +0000]
tests: guarantee abs_srcdir in all C tests

While trying to debug a failure of virpcitest during 'make distcheck',
I noticed that with a VPATH build, 'cd tests; ./virpcitest' fails for
an entirely different reason.  To reproduce the distcheck failure, I
had to run 'cd tests; abs_srcdir=/path/to/src ./virpcitest'.  But we
document in HACKING that all of our tests are supposed to be runnable
without requiring extra environment variables.

The solution: hardcode the location of srcdir into the just-built
binaries, rather than requiring make to prepopulate environment
variables.  With this, './virpcitest' passes even in a VPATH build
(provided that $(srcdir) is writable; a followup patch will fix the
conditions required by 'make distcheck').  [Note: the makefile must
still pass on directory variables to the test environment of shell
scripts, since those aren't compiled.  So while this solves the case
of a compiled test, it still requires environment variables to pass
a VPATH build of any shell script test case that relies on srcdir.]

* tests/Makefile.am (AM_CFLAGS): Define abs_srcdir in all compiled
tests.
* tests/testutils.h (abs_srcdir): Quit declaring.
* tests/testutils.c (virtTestMain): Rely on define rather than
environment variable.
* tests/virpcimock.c (pci_device_new_from_stub): Rely on define.
* tests/cputest.c (mymain): Adjust abs_top_srcdir default.
* tests/qemuxml2argvtest.c (mymain): Likewise.
* tests/qemuxmlnstest.c (mymain): Likewise.

Signed-off-by: Eric Blake <eblake@redhat.com>

4 years agoconf: Export virStorageVolType enum helper functions
Peter Krempa [Wed, 20 Nov 2013 15:02:00 +0000]
conf: Export virStorageVolType enum helper functions

Export string conversion from and to the virStorageVolType enum.

4 years agostorage: skip selinux cleanup when fd not available
Eric Blake [Wed, 27 Nov 2013 03:57:05 +0000]
storage: skip selinux cleanup when fd not available

When attempting to backport gluster pools to an older versoin
where there is no VIR_STRDUP, I got a crash from calling
strdup(,NULL). Rather than relying on the current else branch
safely doing nothing when there is no fd, it is easier to just
skip it. While at it, there's no need to explicitly set
perms.label to NULL after a VIR_FREE().

* src/storage/storage_backend.c
(virStorageBackendUpdateVolTargetInfoFD): Minor optimization.

Signed-off-by: Eric Blake <eblake@redhat.com>

4 years agoqemu: preserve netdev MAC address during 'domxml-to-native'
Bing Bu Cao [Wed, 27 Nov 2013 10:52:12 +0000]
qemu: preserve netdev MAC address during 'domxml-to-native'

The virsh command 'domxml-to-native' (virConnectDomainXMLToNative())
converts all network devices to "type='ethernet'" in order to make it
more likely that the generated command could be run directly from a
shell (other libvirt network device types end up referencing file
descriptors for tap devices assumed to have been created by libvirt,
which can't be done in this case).

During this conversion, all of the netdev parameters are cleared out,
then specific items are filled in after changing the type. The MAC
address was not one of these preserved items, and the result was that
mac addresses in the generated commandlines were always
00:00:00:00:00:00.

This patch saves the mac address before the conversion, then
repopulates it afterwards, so the proper mac addresses show up in the
commandline.

Signed-off-by: Bing Bu Cao <mars@linux.vnet.ibm.com>
Signed-off-by: Laine Stump <laine@laine.org>

4 years agolook for numad in /usr/sbin
Jim Fehlig [Wed, 27 Nov 2013 04:32:43 +0000]
look for numad in /usr/sbin

When looking for numad with AC_PATH_PROG, include /usr/sbin in
the search path.

4 years agostorage: don't read storage volumes in nonblock mode
Eric Blake [Mon, 25 Nov 2013 21:38:22 +0000]
storage: don't read storage volumes in nonblock mode

Commit 348b4e2 introduced a potential problem (thankfully not
in any release): we are attempting to use virFileReadHeaderFD()
on a file that was opened with O_NONBLOCK.  While this
shouldn't be a problem in practice (because O_NONBLOCK
typically doesn't affect regular or block files, and fifos and
sockets cannot be storage volumes), it's better to play it safe
to avoid races from opening an unexpected file type while also
avoiding problems with having to handle EAGAIN while read()ing.

Based on a report by Dan Berrange.

* src/storage/storage_backend.c
(virStorageBackendVolOpenCheckMode): Fix up fd after avoiding race.

Signed-off-by: Eric Blake <eblake@redhat.com>

4 years agoLXC: add securetty related note in Device nodes
Gao feng [Mon, 25 Nov 2013 06:47:53 +0000]
LXC: add securetty related note in Device nodes

Tell user how to resolve the problem that fail to log in
the container.

Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>

4 years agoLXC: don't unmount mounts for shared root
Gao feng [Wed, 20 Nov 2013 02:11:09 +0000]
LXC: don't unmount mounts for shared root

Also after commit 5ff9d8a65ce80efb509ce4e8051394e9ed2cd942
vfs: Lock in place mounts from more privileged users,

unprivileged user has no rights to umount the mounts that
inherited from parent mountns.

right now, I have no good idea to fix this problem, we need
to do more research. this patch just skip unmounting these
mounts for shared root.

BTW, I think when libvirt lxc enables user namespace, the
configuation that shares root with host is very rara.

Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>

4 years agoLXC: fix the problem that libvirt lxc fail to start on latest kernel
Gao feng [Wed, 20 Nov 2013 02:11:08 +0000]
LXC: fix the problem that libvirt lxc fail to start on latest kernel

After kernel commit 5ff9d8a65ce80efb509ce4e8051394e9ed2cd942
vfs: Lock in place mounts from more privileged users,

unprivileged user has no rights to move the mounts that
inherited from parent mountns. we use this feature to move
the /stateDir/domain-name.{dev, devpts} to the /dev/ and
/dev/pts directroy of container. this commit breaks libvirt lxc.

this patch changes the behavior to bind these mounts when
user namespace is enabled and move these mounts when user
namespace is disabled.

Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>

4 years agosasl: Replace 'restep' label with 'continue'
Christophe Fergeau [Tue, 26 Nov 2013 10:34:46 +0000]
sasl: Replace 'restep' label with 'continue'

Since the label is at the beginning of the loop, this has the same effect.

4 years agosasl: Fix authentication when using PLAIN mechanism
Christophe Fergeau [Thu, 21 Nov 2013 17:40:52 +0000]
sasl: Fix authentication when using PLAIN mechanism

With some authentication mechanism (PLAIN for example), sasl_client_start()
can return SASL_OK, which translates to virNetSASLSessionClientStart()
returning VIR_NET_SASL_COMPLETE.
cyrus-sasl documentation is a bit vague as to what to do in such situation,
but upstream clarified this a bit in
http://asg.andrew.cmu.edu/archive/message.php?mailbox=archive.cyrus-sasl&msg=10104

When we got VIR_NET_SASL_COMPLETE after virNetSASLSessionClientStart() and
if the remote also tells us that authentication is complete, then we should
end the authentication procedure rather than forcing a call to
virNetSASLSessionClientStep(). Without this patch, when trying to use SASL
PLAIN, I get:
error :authentication failed : Failed to step SASL negotiation: -1
(SASL(-1): generic failure: Unable to find a callback: 32775)

This patch is based on a spice-gtk patch by Dietmar Maurer.

4 years agoFix invalid read in virNetSASLSessionClientStep debug log
Christophe Fergeau [Fri, 22 Nov 2013 16:54:53 +0000]
Fix invalid read in virNetSASLSessionClientStep debug log

virNetSASLSessionClientStep logs the data that is going to be passed to
sasl_client_step as input data. However, it tries to log it as a string,
while there is no guarantee that this data is going to be nul-terminated.
This leads to this valgrind log:

==20938== Invalid read of size 1
==20938==    at 0x8BDB08F: vfprintf (vfprintf.c:1635)
==20938==    by 0x8C06DF2: vasprintf (vasprintf.c:62)
==20938==    by 0x4CCEDF9: virVasprintfInternal (virstring.c:337)
==20938==    by 0x4CA9516: virLogVMessage (virlog.c:842)
==20938==    by 0x4CA939A: virLogMessage (virlog.c:778)
==20938==    by 0x4E21E0D: virNetSASLSessionClientStep (virnetsaslcontext.c:458)
==20938==    by 0x4DE47B8: remoteAuthSASL (remote_driver.c:4136)
==20938==    by 0x4DE33AE: remoteAuthenticate (remote_driver.c:3635)
==20938==    by 0x4DDBFAA: doRemoteOpen (remote_driver.c:832)
==20938==    by 0x4DDC8BA: remoteConnectOpen (remote_driver.c:1027)
==20938==    by 0x4D8595F: do_open (libvirt.c:1239)
==20938==    by 0x4D863F3: virConnectOpenAuth (libvirt.c:1481)
==20938==    by 0x12762B: vshReconnect (virsh.c:337)
==20938==    by 0x12C9B0: vshInit (virsh.c:2470)
==20938==    by 0x12E9A5: main (virsh.c:3338)
==20938==  Address 0xe329ccd is 0 bytes after a block of size 141 alloc'd
==20938==    at 0x4A081D4: calloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==20938==    by 0x8CB91B4: xdr_array (xdr_array.c:94)
==20938==    by 0x4E039C2: xdr_remote_auth_sasl_start_ret (remote_protocol.c:3134)
==20938==    by 0x4E1F8AA: virNetMessageDecodePayload (virnetmessage.c:405)
==20938==    by 0x4E119F5: virNetClientProgramCall (virnetclientprogram.c:377)
==20938==    by 0x4DF8141: callFull (remote_driver.c:5794)
==20938==    by 0x4DF821A: call (remote_driver.c:5816)
==20938==    by 0x4DE46CF: remoteAuthSASL (remote_driver.c:4112)
==20938==    by 0x4DE33AE: remoteAuthenticate (remote_driver.c:3635)
==20938==    by 0x4DDBFAA: doRemoteOpen (remote_driver.c:832)
==20938==    by 0x4DDC8BA: remoteConnectOpen (remote_driver.c:1027)
==20938==    by 0x4D8595F: do_open (libvirt.c:1239)
==20938==    by 0x4D863F3: virConnectOpenAuth (libvirt.c:1481)
==20938==    by 0x12762B: vshReconnect (virsh.c:337)
==20938==    by 0x12C9B0: vshInit (virsh.c:2470)
==20938==    by 0x12E9A5: main (virsh.c:3338)

4 years agoTie SASL callbacks lifecycle to virNetSessionSASLContext
Christophe Fergeau [Fri, 22 Nov 2013 16:27:21 +0000]
Tie SASL callbacks lifecycle to virNetSessionSASLContext

The array of sasl_callback_t callbacks which is passed to sasl_client_new()
must be kept alive as long as the created sasl_conn_t object is alive as
cyrus-sasl uses this structure internally for things like logging, so
the memory used for callbacks must only be freed after sasl_dispose() has
been called.

During testing of successful SASL logins with
virsh -c qemu+tls:///system list --all
I've been getting invalid read reports from valgrind

==9237== Invalid read of size 8
==9237==    at 0x6E93B6F: _sasl_getcallback (common.c:1745)
==9237==    by 0x6E95430: _sasl_log (common.c:1850)
==9237==    by 0x16593D87: digestmd5_client_mech_dispose (digestmd5.c:4580)
==9237==    by 0x6E91653: client_dispose (client.c:332)
==9237==    by 0x6E9476A: sasl_dispose (common.c:851)
==9237==    by 0x4E225A1: virNetSASLSessionDispose (virnetsaslcontext.c:678)
==9237==    by 0x4CBC551: virObjectUnref (virobject.c:262)
==9237==    by 0x4E254D1: virNetSocketDispose (virnetsocket.c:1042)
==9237==    by 0x4CBC551: virObjectUnref (virobject.c:262)
==9237==    by 0x4E2701C: virNetSocketEventFree (virnetsocket.c:1794)
==9237==    by 0x4C965D3: virEventPollCleanupHandles (vireventpoll.c:583)
==9237==    by 0x4C96987: virEventPollRunOnce (vireventpoll.c:652)
==9237==    by 0x4C94730: virEventRunDefaultImpl (virevent.c:274)
==9237==    by 0x12C7BA: vshEventLoop (virsh.c:2407)
==9237==    by 0x4CD3D04: virThreadHelper (virthreadpthread.c:161)
==9237==    by 0x7DAEF32: start_thread (pthread_create.c:309)
==9237==    by 0x8C86EAC: clone (clone.S:111)
==9237==  Address 0xe2d61b0 is 0 bytes inside a block of size 168 free'd
==9237==    at 0x4A07577: free (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==9237==    by 0x4C73827: virFree (viralloc.c:580)
==9237==    by 0x4DE4BC7: remoteAuthSASL (remote_driver.c:4219)
==9237==    by 0x4DE33D0: remoteAuthenticate (remote_driver.c:3639)
==9237==    by 0x4DDBFAA: doRemoteOpen (remote_driver.c:832)
==9237==    by 0x4DDC8DC: remoteConnectOpen (remote_driver.c:1031)
==9237==    by 0x4D8595F: do_open (libvirt.c:1239)
==9237==    by 0x4D863F3: virConnectOpenAuth (libvirt.c:1481)
==9237==    by 0x12762B: vshReconnect (virsh.c:337)
==9237==    by 0x12C9B0: vshInit (virsh.c:2470)
==9237==    by 0x12E9A5: main (virsh.c:3338)

This commit changes virNetSASLSessionNewClient() to take ownership of the SASL
callbacks. Then we can free them in virNetSASLSessionDispose() after the corresponding
sasl_conn_t has been freed.

4 years agoremote: Don't leak priv->tls object on connection failure
Christophe Fergeau [Fri, 22 Nov 2013 16:24:32 +0000]
remote: Don't leak priv->tls object on connection failure

When testing SASL authentication over TLS with
virsh -c qemu+tls:///system list --all
I got this valgrind trace after entering wrong credentials:

==30540== 26,903 (88 direct, 26,815 indirect) bytes in 1 blocks are definitely lost in loss record 289 of 293
==30540==    at 0x4A081D4: calloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==30540==    by 0x4C7379A: virAllocVar (viralloc.c:558)
==30540==    by 0x4CBC178: virObjectNew (virobject.c:190)
==30540==    by 0x4CBC329: virObjectLockableNew (virobject.c:216)
==30540==    by 0x4E2D003: virNetTLSContextNew (virnettlscontext.c:719)
==30540==    by 0x4E2DC3F: virNetTLSContextNewPath (virnettlscontext.c:930)
==30540==    by 0x4E2DD5B: virNetTLSContextNewClientPath (virnettlscontext.c:957)
==30540==    by 0x4DDB618: doRemoteOpen (remote_driver.c:627)
==30540==    by 0x4DDC8BA: remoteConnectOpen (remote_driver.c:1031)
==30540==    by 0x4D8595F: do_open (libvirt.c:1239)
==30540==    by 0x4D863F3: virConnectOpenAuth (libvirt.c:1481)
==30540==    by 0x12762B: vshReconnect (virsh.c:337)
==30540==    by 0x12C9B0: vshInit (virsh.c:2470)
==30540==    by 0x12E9A5: main (virsh.c:3338)

4 years agoFix three minor typos
Yuri Chornoivan [Tue, 26 Nov 2013 07:15:09 +0000]
Fix three minor typos

4 years agostorage: fix typo in previous patch v1.2.0-rc1
Eric Blake [Mon, 25 Nov 2013 21:44:30 +0000]
storage: fix typo in previous patch

You'd think I'd learn to actually COMMIT my working tree
between testing that a last-minute fix compiles and pushing.

* src/storage/storage_backend_gluster.c
(virStorageBackendGlusterRefreshVol): Typo fix.

Signed-off-by: Eric Blake <eblake@redhat.com>

4 years agostorage: probe qcow2 volumes in gluster pool
Eric Blake [Tue, 12 Nov 2013 04:08:27 +0000]
storage: probe qcow2 volumes in gluster pool

Putting together pieces from previous patches, it is now possible
for 'virsh vol-dumpxml --pool gluster volname' to report metadata
about a qcow2 file stored on gluster.  The backing file is still
treated as raw; to fix that, more patches are needed to make the
storage backing chain analysis recursive rather than halting at
a network protocol name, but that work will not need any further
calls into libgfapi so much as just reusing this code, and that
should be the only code outside of the storage driver that needs
any help from libgfapi.  Any additional use of libgfapi within
libvirt should only be needed for implementing storage pool APIs
such as volume creation or resizing, where backing chain analysis
should be unaffected.

* src/storage/storage_backend_gluster.c
(virStorageBackendGlusterReadHeader): New helper function.
(virStorageBackendGlusterRefreshVol): Probe non-raw files.

Signed-off-by: Eric Blake <eblake@redhat.com>

4 years agostorage: improve handling of symlinks in gluster
Eric Blake [Mon, 18 Nov 2013 22:24:05 +0000]
storage: improve handling of symlinks in gluster

With this patch, dangling and looping symlinks are silently
ignored, while links to files and directories are treated the
same as the underlying file or directory.  This is the same
behavior as both 'directory' and 'netfs' pools.

* src/storage/storage_backend_gluster.c
(virStorageBackendGlusterRefreshVol): Treat symlinks similar to
directory and netfs pools.

Signed-off-by: Eric Blake <eblake@redhat.com>

4 years agostorage: improve allocation stats reported on gluster files
Eric Blake [Mon, 18 Nov 2013 19:45:31 +0000]
storage: improve allocation stats reported on gluster files

We already had code for handling allocation different than
capacity for sparse files; we just had to wire it up to be
used when inspecting gluster images.

* src/storage/storage_backend.c
(virStorageBackendUpdateVolTargetInfoFD): Handle no fd.
* src/storage/storage_backend_gluster.c
(virStorageBackendGlusterRefreshVol): Handle sparse files.

Signed-off-by: Eric Blake <eblake@redhat.com>

4 years agostorage: improve directory support in gluster pool
Eric Blake [Wed, 20 Nov 2013 20:17:55 +0000]
storage: improve directory support in gluster pool

Take advantage of the previous patch's addition of 'netdir' as
a distinct volume type, to expose rather than silently skip
directories embedded in a gluster pool.  Also serves as an XML
validation for the previous patch.

* src/storage/storage_backend_gluster.c
(virStorageBackendGlusterRefreshVol): Don't skip directories.
* tests/storagevolxml2xmltest.c (mymain): Add test.
* tests/storagevolxml2xmlin/vol-gluster-dir.xml: New file.
* tests/storagevolxml2xmlout/vol-gluster-dir.xml: Likewise.

Signed-off-by: Eric Blake <eblake@redhat.com>

4 years agostorage: add network-dir as new storage volume type
Eric Blake [Mon, 18 Nov 2013 23:43:06 +0000]
storage: add network-dir as new storage volume type

In the 'directory' and 'netfs' storage pools, a user can see
both 'file' and 'dir' storage volume types, to know when they
can descend into a subdirectory.  But in a network-based storage
pool, such as the upcoming 'gluster' pool, we use 'network'
instead of 'file', and did not have any counterpart for a
directory until this patch.  Adding a new volume type
'network-dir' is better than reusing 'dir', because it makes
it clear that the only way to access 'network' volumes within
that container is through the network mounting (leaving 'dir'
for something accessible in the local file system).

* include/libvirt/libvirt.h.in (virStorageVolType): Expand enum.
* docs/formatstorage.html.in: Document it.
* docs/schemasa/storagevol.rng (vol): Allow new value.
* src/conf/storage_conf.c (virStorageVol): Use new value.
* src/qemu/qemu_command.c (qemuBuildVolumeString): Fix client.
* src/qemu/qemu_conf.c (qemuTranslateDiskSourcePool): Likewise.
* tools/virsh-volume.c (vshVolumeTypeToString): Likewise.
* src/storage/storage_backend_fs.c
(virStorageBackendFileSystemVolDelete): Likewise.

Signed-off-by: Eric Blake <eblake@redhat.com>

4 years agostorage: implement rudimentary glusterfs pool refresh
Eric Blake [Wed, 30 Oct 2013 03:28:16 +0000]
storage: implement rudimentary glusterfs pool refresh

Actually put gfapi to use, by allowing the creation of a gluster
pool.  Right now, all volumes are treated as raw and directories
are skipped; further patches will allow peering into files to
allow for qcow2 files and backing chains, and reporting proper
volume allocation.  This implementation was tested against Fedora
19's glusterfs 3.4.1; it might be made simpler by requiring a
higher minimum, and/or require more hacks to work with a lower
minimum.

* src/storage/storage_backend_gluster.c
(virStorageBackendGlusterRefreshPool): Initial implementation.
(virStorageBackendGlusterOpen, virStorageBackendGlusterClose)
(virStorageBackendGlusterRefreshVol): New helper functions.

Signed-off-by: Eric Blake <eblake@redhat.com>

4 years agostorage: document gluster pool
Eric Blake [Tue, 15 Oct 2013 23:06:18 +0000]
storage: document gluster pool

Add support for a new <pool type='gluster'>, similar to
RBD and Sheepdog.  Terminology wise, a gluster volume
forms a libvirt storage pool, within the gluster volume,
individual files are treated as libvirt storage volumes.

* docs/schemas/storagepool.rng (poolgluster): New pool type.
* docs/formatstorage.html.in: Document gluster.
* docs/storage.html.in: Likewise, and contrast it with netfs.
* tests/storagepoolxml2xmlin/pool-gluster.xml: New test.
* tests/storagepoolxml2xmlout/pool-gluster.xml: Likewise.
* tests/storagepoolxml2xmltest.c (mymain): Likewise.

Signed-off-by: Eric Blake <eblake@redhat.com>

4 years agostorage: initial support for linking with libgfapi
Eric Blake [Tue, 19 Nov 2013 23:26:05 +0000]
storage: initial support for linking with libgfapi

We support gluster volumes in domain XML, so we also ought to
support them as a storage pool.  Besides, a future patch will
want to take advantage of libgfapi to handle the case of a
gluster device holding qcow2 rather than raw storage, and for
that to work, we need a storage backend that can read gluster
storage volume contents.  This sets up the framework.

Note that the new pool is named 'gluster' to match a
<disk type='network'><source protocol='gluster'> image source
already supported in a <domain>; it does NOT match the
<pool type='netfs'><source><target type='glusterfs'>,
since that uses a FUSE mount to a local file name rather than
a network name.

This and subsequent patches have been tested against glusterfs
3.4.1 (available on Fedora 19); there are likely bugs in older
versions that may prevent decent use of gfapi, so this patch
enforces the minimum version tested.  A future patch may lower
the minimum.  On the other hand, I hit at least two bugs in
3.4.1 that will be fixed in 3.5/3.4.2, where it might be worth
raising the minimum: glfs_readdir is nicer to use than
glfs_readdir_r [1], and glfs_fini should only return failure on
an actual failure [2].

[1] http://lists.gnu.org/archive/html/gluster-devel/2013-10/msg00085.html
[2] http://lists.gnu.org/archive/html/gluster-devel/2013-10/msg00086.html

* configure.ac (WITH_STORAGE_GLUSTER): New conditional.
* m4/virt-gluster.m4: new file.
* libvirt.spec.in (BuildRequires): Support gluster in spec file.
* src/conf/storage_conf.h (VIR_STORAGE_POOL_GLUSTER): New pool
type.
* src/conf/storage_conf.c (poolTypeInfo): Treat similar to
sheepdog and rbd.
(virStoragePoolDefFormat): Don't output target for gluster.
* src/storage/storage_backend_gluster.h: New file.
* src/storage/storage_backend_gluster.c: Likewise.
* po/POTFILES.in: Add new file.
* src/storage/storage_backend.c (backends): Register new type.
* src/Makefile.am (STORAGE_DRIVER_GLUSTER_SOURCES): Build new files.
* src/storage/storage_backend.h (_virStorageBackend): Documet
assumption.

Signed-off-by: Eric Blake <eblake@redhat.com>

4 years agostorage: expose volume meta-type in XML
Eric Blake [Tue, 19 Nov 2013 20:14:54 +0000]
storage: expose volume meta-type in XML

I got annoyed at having to use both 'virsh vol-list $pool --details'
AND 'virsh vol-dumpxml $vol $pool' to learn if I had populated
the volume correctly.  Since two-thirds of the data present in
virStorageVolGetInfo() already appears in virStorageVolGetXMLDesc(),
this just adds the remaining piece of information, as:

<volume type='...'>
  ...
</volume>

* docs/formatstorage.html.in: Document new <volume type=...>.
* docs/schemas/storagevol.rng (vol): Add it to RelaxNG.
* src/conf/storage_conf.h (virStorageVolTypeToString): Declare.
* src/conf/storage_conf.c (virStorageVolTargetDefFormat): Output
the metatype.
(virStorageVolDefParseXML): Parse it, for unit tests.
* tests/storagevolxml2xmlout/vol-*.xml: Update tests to match.

Signed-off-by: Eric Blake <eblake@redhat.com>

4 years agoRemove obsolete 'tests' makefile target
Daniel P. Berrange [Mon, 25 Nov 2013 16:14:23 +0000]
Remove obsolete 'tests' makefile target

The 'docs/examples' code was long ago removed and now the
python code was gone too, the custom 'tests' makefile target
serves no purpose

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>

4 years agoRemove python binding
Daniel P. Berrange [Fri, 22 Nov 2013 16:42:22 +0000]
Remove python binding

The python binding now lives in

  http://libvirt.org/git/?p=libvirt-python.git

that repo also provides an RPM which is upgrade compatible
with the old libvirt-python sub-RPM.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>

4 years agospec: Don't save/restore running VMs on libvirt-client update
Jiri Denemark [Mon, 25 Nov 2013 15:37:32 +0000]
spec: Don't save/restore running VMs on libvirt-client update

The previous attempt (commit d65e0e1) removed just one of two
libvirt-guests restarts that happened on libvirt-client update. Let's
remove the last one too :-)

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

Signed-off-by: Jiri Denemark <jdenemar@redhat.com>

4 years agovirsh domxml-from-native to treat SCSI as the bus type for pseries by default
Shivaprasad G Bhat [Fri, 22 Nov 2013 17:27:25 +0000]
virsh domxml-from-native to treat SCSI as the bus type for pseries by default

The bus type IDE being enum Zero, the bus type on pseries system appears as IDE for all the -hda/-cdrom and for disk drives with if="none" type. Pseries platform needs this to appear as SCSI instead of IDE. The ide being not supported, the explicit requests for ide devices will return an error.

Signed-off-by: Shivaprasad G Bhat <sbhat@linux.vnet.ibm.com>

4 years agostorage: allow interleave in volume XML
Eric Blake [Fri, 22 Nov 2013 19:40:15 +0000]
storage: allow interleave in volume XML

The RNG grammar did not allow arbitrary interleaving, which makes
it harder than necessary to create a new volume from handwritten XML.
(Compare also to commit caf516db for pools).

* docs/schemas/storagevol.rng: Support interleaving.
* tests/storagevolxml2xmlin/vol-file-backing.xml: Test it.

Signed-off-by: Eric Blake <eblake@redhat.com>

4 years agomaint: next release is 1.2.0
Eric Blake [Mon, 25 Nov 2013 14:31:08 +0000]
maint: next release is 1.2.0

I didn't find any other instances with:
git grep '1\.1\.5'

* src/test/test_driver.c (testDriver): Tweak version info.

Signed-off-by: Eric Blake <eblake@redhat.com>