Commit Graph

12507 Commits

Author SHA1 Message Date
Jiri Denemark
3779715e8d qemu: Let empty default VNC password work as documented
CVE-2016-5008

Setting an empty graphics password is documented as a way to disable
VNC/SPICE access, but QEMU does not always behaves like that. VNC would
happily accept the empty password. Let's enforce the behavior by setting
password expiration to "now".

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

Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
(cherry picked from commit bb848feec0)
(cherry picked from commit d933f68ee6)
2016-06-30 14:02:41 +01:00
Eric Blake
220c6b867c 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 b1674ad5a9)

Conflicts:
	src/libvirt-domain.c - file split from older src/libvirt.c; context with older virLibConnError
	src/remote/remote_protocol.x - no fine-grained ACLs
Signed-off-by: Eric Blake <eblake@redhat.com>
2014-11-10 09:33:56 -07:00
Pavel Hrdina
31674d08fc 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 fc22b2e748)
2014-10-01 12:18:48 -06:00
Peter Krempa
35a802639d 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 3e745e8f77)

Conflicts:
	src/qemu/qemu_driver.c - context due to fewer functions
2014-09-17 22:15:18 -06:00
Daniel P. Berrange
16fc426a27 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 d6b27d3e4c)
2014-09-17 22:14:59 -06:00
Peter Krempa
e22f1c2e13 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 35c7701c64
but since it was introduced a few commits before that it was never
released as working.

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

Conflicts:
	src/qemu/qemu_driver.c - no refactoring of commits 7b7bf001, 4f20226
2014-07-03 06:20:37 -06:00
Eric Blake
248d9a49f7 build: fix 'make check' with newer git
Newer git doesn't like the maint.mk rule 'public-submodule-commit'
run during 'make check', as inherited from our checkout of gnulib.
I tracked down that libvirt commit 8531301 picked up a gnulib fix
that makes git happy.  Rather than try and do a full .gnulib
submodule update to gnulib.git d18d1b802 (as used in that libvirt
commit), it was easier to just backport the fixed maint.mk from
gnulib on top of our existing submodule level.  I did it as follows,
where these steps will have to be repeated when cherry-picking this
commit to any other maintenance branch:

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>
2014-07-03 06:20:29 -06:00
Eric Blake
e08ee660c3 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 9b291bbe20)

Conflicts:
	docs/apibuild.py - context with 2a40951
2014-06-26 18:11:01 -06:00
Peter Krempa
6cb267e816 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 02b364e186)

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, a88fb30, 632f78c

Conflicts:
	src/qemu/qemu_driver.c
2014-06-26 18:10:59 -06:00
Laine Stump
8ff53ba991 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 0eac9d1e90)
2014-05-01 16:03:24 +03:00
Martin Kletzander
796ef25c33 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 736e017e36)

Conflicts:
	src/qemu/qemu_agent.c -- vCPU functions (3099c063)
2014-04-09 18:11:11 +02:00
Martin Kletzander
eb49d5c5d9 qemu: remove unneeded forward declaration
by moving qemuAgentCommand() after qemuAgentCheckError().

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

Conflicts:
        src/qemu/qemu_agent.c -- label indentation (5922d05a)
                                 comment removal (56874f01)
                                 VIR_ALLOC refactor (e987a30d)
2014-04-09 18:10:44 +02:00
Martin Kletzander
16ee93cc7a 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 5b3492fadb)

Conflicts:
	src/qemu/qemu_agent.c -- vCPU functions (3099c063)
2014-04-09 18:10:39 +02:00
Peter Krempa
60f3065e0f qemu: Properly report guest agent errors on command passthrough
The code for arbitrary guest agent passthrough was horribly broken since
introduction. Fix it to correctly report errors.

(cherry picked from commit 6e5b36d5d2)
2014-04-09 18:10:37 +02:00
Michal Privoznik
892107de19 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 3d4b4f5ac6)
2014-03-19 22:27:47 -06:00
Daniel P. Berrange
ebc7e55ce8 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 925de19ed7)

Conflicts:
	src/nwfilter/nwfilter_gentech_driver.c
2014-03-10 13:08:25 +00:00
Daniel P. Berrange
720e945e3b 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 6e5c79a1b5)

Conflicts:
  src/conf/nwfilter_conf.c
   - virReportOOMError() in context of one hunk.
  src/lxc/lxc_driver.c
   - trivial date conflict in copyright line
   - functions renamed, and lxc object locking changed, creating
     a conflict in the context.
  src/qemu/qemu_driver.c
   - qemuDomainStartWithFlags (called qemuDomainCreateWithFlags
     upstream) gets the domain object using
     qemuDomObjFromDomain() upstream, but
     virDomainObjListFindByUUID() in 1.0.4. This creates a
     small conflict in context.
2014-02-06 15:58:39 +02:00
Daniel P. Berrange
14f6b2085e 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 c065984b58)

Conflicts:
  src/libvirt_private.syms
   - virThreadCancel (in context) was introduced after 1.0.3 branch
2014-02-06 15:49:51 +02:00
Daniel P. Berrange
e57db95253 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 999d72fbd5)

Conflicts:
  src/nwfilter/nwfilter_driver.c
    - *EnsureACL*() was added after this branch was created, and
       caused two small conflicts in the context around a hunk.
    - nwfilterDriverReload() was renamed to nwfilterStateReload()
      upstream.
2014-02-06 15:48:21 +02:00
Daniel P. Berrange
27465d9c1f 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 ebca369e3f)
2014-02-06 15:46:34 +02:00
Daniel P. Berrange
73e236cedf 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 b77b16ce41)
2014-02-06 15:46:27 +02:00
Daniel P. Berrange
2a564e421b Don't ignore errors parsing nwfilter rules
For inexplicable reasons, the nwfilter XML parser is intentionally
ignoring errors that arise during parsing. As well as meaning that
users don't get any feedback on their XML mistakes, this will lead
it to silently drop data in OOM conditions.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
(cherry picked from commit 4f2094346d)
2014-02-06 15:46:22 +02:00
Jiri Denemark
b24979a12f Really don't crash if a connection closes early
https://bugzilla.redhat.com/show_bug.cgi?id=1047577

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

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

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

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

Every libvirt release since 0.9.8 is affected by this bug.

(cherry picked from commit 173c291473)
2014-01-15 11:01:59 -07:00
Jiri Denemark
c51986ba82 qemu: Fix job usage in virDomainGetBlockIoTune
CVE-2013-6458

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

(cherry picked from commit 3b56425938)
2014-01-15 11:01:57 -07:00
Jiri Denemark
a5987e23d5 qemu: Fix job usage in qemuDomainBlockCopy
Every API that is going to begin a job should do that before fetching
data from vm->def.

(cherry picked from commit ff5f30b6bf)

Conflicts:
	src/qemu/qemu_driver.c - context
2014-01-15 11:01:56 -07:00
Jiri Denemark
da2d96d125 qemu: Fix job usage in qemuDomainBlockJobImpl
CVE-2013-6458

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

(cherry picked from commit f93d2caa07)
2014-01-15 11:01:55 -07:00
Jiri Denemark
12ca0aaf2f qemu: Avoid using stale data in virDomainGetBlockInfo
CVE-2013-6458

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

(cherry picked from commit b799259583)

Conflicts:
	src/qemu/qemu_driver.c - VIR_STRDUP not backported
2014-01-15 11:01:53 -07:00
Jiri Denemark
59d46c6cd5 qemu: Do not access stale data in virDomainBlockStats
CVE-2013-6458
https://bugzilla.redhat.com/show_bug.cgi?id=1043069

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

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

(cherry picked from commit db86da5ca2)

Conflicts:
	src/qemu/qemu_driver.c - context: no ACLs
2014-01-15 11:01:52 -07:00
Eric Blake
ab8336d3aa tests: be more explicit on qcow2 versions in virstoragetest
While working on v1.0.5-maint (the branch in use on Fedora 19)
with the host at Fedora 20, I got a failure in virstoragetest.
I traced it to the fact that we were using qemu-img to create a
qcow2 file, but qemu-img changed from creating v2 files by
default in F19 to creating v3 files in F20.  Rather than leaving
it up to qemu-img, it is better to write the test to force
testing of BOTH file formats (better code coverage and all).

This patch alone does not fix all the failures in v1.0.5-maint;
for that, we must decide to either teach the older branch to
understand v3 files, or to reject them outright as unsupported.
But for upstream, making the test less dependent on changing
qemu-img defaults is always a good thing.

* tests/virstoragetest.c (testPrepImages): Simplify creation of
raw file; check if qemu supports compat and if so use it.

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

Conflicts:
	tests/virstoragetest.c - hardcode test to v2, since this branch doesn't handle v3 correctly
2014-01-15 11:01:48 -07:00
Eric Blake
cae675f25d build: use proper pod for nested bulleted VIRSH_DEBUG list
Newer pod (hello rawhide) complains if you attempt to mix bullets
and non-bullets in the same list:

virsh.pod around line 3177: Expected text after =item, not a bullet

As our intent was to nest an inner list, we make that explicit to
keep pod happy.

* tools/virsh.pod (ENVIRONMENT): Use correct pod syntax.

(cherry picked from commit 00d69b4af1)
2014-01-15 11:01:46 -07:00
Jim Fehlig
e69022b14f libxl: fix build with Xen4.3
Xen 4.3 fixes a mistake in the libxl event handler signature where the
event owned by the application was defined as const.  Detect this and
define the libvirt libxl event handler signature appropriately.
(cherry picked from commit 43b0ff5b1e)
2014-01-15 11:01:43 -07:00
Zhou Yimin
bb250de38c remote: fix regression in event deregistration
Introduced by 7b87a3
When I quit the process which only register VIR_DOMAIN_EVENT_ID_REBOOT,
I got error like:
"libvirt: XML-RPC error : internal error: domain event 0 not registered".
Then I add the following code, it fixed.

Signed-off-by: Zhou Yimin <zhouyimin@huawei.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
(cherry picked from commit 9712c2510e)
2013-10-18 07:56:31 -06:00
Osier Yang
8a7dbdc344 virsh: Fix regression of vol-resize
Introduced by commit 1daa4ba33a. vshCommandOptStringReq returns
0 on *success* or the option is not required && not present, both
are right result. Error out when returning 0 is not correct.
the caller, it doesn't have to check wether it
(cherry picked from commit 2a3a725c33)
2013-10-03 10:35:19 +02:00
Daniel P. Berrange
a413bc2dda Fix crash in remoteDispatchDomainMemoryStats (CVE-2013-4296)
The 'stats' variable was not initialized to NULL, so if some
early validation of the RPC call fails, it is possible to jump
to the 'cleanup' label and VIR_FREE an uninitialized pointer.
This is a security flaw, since the API can be called from a
readonly connection which can trigger the validation checks.

This was introduced in release v0.9.1 onwards by

  commit 158ba8730e
  Author: Daniel P. Berrange <berrange@redhat.com>
  Date:   Wed Apr 13 16:21:35 2011 +0100

    Merge all returns paths from dispatcher into single path

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

Conflicts:
	daemon/remote.c - context
2013-09-18 15:08:35 -06:00
Eric Blake
c7df5ed8d8 maint: update to latest gnulib
Upstream gnulib determined that we were needlessly compiling in
gnulib's regex instead of glibc's when targetting new-enough glibc,
because the m4 test was being too strict in requiring a particular
answer to undefined behavior.
https://lists.gnu.org/archive/html/bug-gnulib/2013-04/msg00032.html

* .gnulib: Update to latest, for regex.

(cherry picked from commit 842432390b)
2013-09-18 15:04:36 -06:00
Eric Blake
aaee109fbb maint: update to latest gnulib
While this update doesn't address any reported problems in libvirt,
doing a post-release update to latest gnulib makes it easier to
stay in sync with best upstream practices.

* .gnulib: Update to latest.
* bootstrap: Resynchronize.

(cherry picked from commit d7468b7d47)
2013-09-18 15:04:30 -06:00
Daniel P. Berrange
6a98ac9000 Fix deps for generating RPC dispatch code
The src/lxc/lxc_*_dispatch.h files only had deps on the
RPC generator script & the XDR definition file. So when
the Makefile.am args passed to the generator were change,
the disaptch code was not re-generated. This caused a
build failure

  CC       libvirt_lxc-lxc_controller.o
lxc/lxc_controller.c: In function 'virLXCControllerSetupServer':
lxc/lxc_controller.c:718:47: error: 'virLXCMonitorProcs' undeclared (first use in this function)
lxc/lxc_controller.c:718:47: note: each undeclared identifier is reported only once for each function it appears in
lxc/lxc_controller.c:719:47: error: 'virLXCMonitorNProcs' undeclared (first use in this function)
make[3]: *** [libvirt_lxc-lxc_controller.o] Error 1

For added fun, the generated files were not listed in
CLEANFILES, so only a 'git clean -f' would fix the build

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
(cherry picked from commit 0946c5f5fc)
2013-09-18 15:00:43 -06:00
Daniel P. Berrange
69a4bc670b Add support for using 3-arg pkcheck syntax for process (CVE-2013-4311)
With the existing pkcheck (pid, start time) tuple for identifying
the process, there is a race condition, where a process can make
a libvirt RPC call and in another thread exec a setuid application,
causing it to change to effective UID 0. This in turn causes polkit
to do its permission check based on the wrong UID.

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

Signed-off-by: Colin Walters <walters@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
(cherry picked from commit 922b7fda77)

Conflicts:
	src/access/viraccessdriverpolkit.c

Resolution:
  Dropped file that does not exist in this branch.
2013-09-18 17:44:15 +01:00
Daniel P. Berrange
93853474a6 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 979e9c56a7)

Conflicts:
	src/util/virprocess.c
	src/util/virstring.c
	src/util/virstring.h
	src/rpc/virnetserverclient.c
	src/rpc/virnetsocket.h
	src/util/viridentity.h
2013-09-18 17:43:54 +01:00
Ján Tomko
a710962d86 storage: return -1 when fs pool can't be mounted
Don't reuse the return value of virStorageBackendFileSystemIsMounted.
If it's 0, we'd return it even if the mount command failed.

Also, don't report another error if it's -1, since one has already
been reported.

Introduced by 258e06c.

https://bugzilla.redhat.com/show_bug.cgi?id=981251
(cherry picked from commit 13fde7ceab)
2013-07-11 15:45:25 +02:00
Ján Tomko
d145c8300f qemu: fix return value of qemuDomainBlockPivot on errors
If qemuMonitorBlockJob returned 0, qemuDomainBlockPivot
might return 0 even if an error occured.

https://bugzilla.redhat.com/show_bug.cgi?id=977678
(cherry picked from commit c34107dfd3)
2013-07-11 15:45:17 +02:00
Ján Tomko
fbd23fbcbf bridge: don't crash on bandwidth unplug with no bandwidth
If networkUnplugBandwidth is called on a network which has
no bandwidth defined, print a warning instead of crashing.

This can happen when destroying a domain with bandwidth if
bandwidth was removed from the network after the domain was
started.

https://bugzilla.redhat.com/show_bug.cgi?id=975359
(cherry picked from commit 658c932ab4)
2013-07-01 15:02:20 +02:00
Ján Tomko
0f953ec3de Fix invalid read in virCgroupGetValueStr
Don't check for '\n' at the end of file if zero bytes were read.

Found by valgrind:
==404== Invalid read of size 1
==404==    at 0x529B09F: virCgroupGetValueStr (vircgroup.c:540)
==404==    by 0x529AF64: virCgroupMoveTask (vircgroup.c:1079)
==404==    by 0x1EB475: qemuSetupCgroupForEmulator (qemu_cgroup.c:1061)
==404==    by 0x1D9489: qemuProcessStart (qemu_process.c:3801)
==404==    by 0x18557E: qemuDomainObjStart (qemu_driver.c:5787)
==404==    by 0x190FA4: qemuDomainCreateWithFlags (qemu_driver.c:5839)

Introduced by 0d0b409.

https://bugzilla.redhat.com/show_bug.cgi?id=978356
(cherry picked from commit 306c49ffd5)
2013-07-01 15:02:10 +02:00
Ján Tomko
08fa08c647 virsh: edit: don't leak XML string on reedit or redefine
Free the old XML strings before overwriting them if the user
has chosen to reedit the file or force the redefinition.

Found by Alex Jia trying to reproduce another bug:
https://bugzilla.redhat.com/show_bug.cgi?id=977430#c3
(cherry picked from commit 1e3a252974)
2013-07-01 15:02:03 +02:00
Laine Stump
749d1fd321 qemu: prevent termination of guests w/hostdev on driver reconnect
This should resolve:

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

The problem was that qemuUpdateActivePciHostdevs was returning 0
(success) when no hostdevs were present, but would otherwise return -1
(failure) even when it completed successfully. It is only called from
qemuProcessReconnect(), and when qemuProcessReconnect got back an
error, it would not only stop reconnecting, but would terminate the
guest qemu process "to remove danger of it ending up running twice if
user tries to start it again later".

(This bug was introduced in commit 011cf7ad, which was pushed between
v1.0.2 and v1.0.3, so all maintenance branches from v1.0.3 up to 1.0.5
will need this one line patch applied.)
(cherry picked from commit 2ea45647bc)
2013-05-31 15:00:10 -04:00
Ján Tomko
7d5e3f0266 daemon: fix leak after listing all volumes
CVE-2013-1962

remoteDispatchStoragePoolListAllVolumes wasn't freeing the pool.
The pool also held a reference to the connection, preventing it from
getting freed and closing the netcf interface driver, which held two
sockets open.
(cherry picked from commit ca697e90d5)
2013-05-16 16:01:16 +02:00
Ján Tomko
8e2d8d19bd don't mention disk controllers in generic controller errors
The controller element supports non-disk controller types too.

https://bugzilla.redhat.com/show_bug.cgi?id=960958
(cherry picked from commit c075f89fa2)
2013-05-09 15:56:08 +02:00
Ján Tomko
be563bcfe6 iscsi: don't leak portal string when starting a pool
(cherry picked from commit 413274f63b)
2013-05-09 15:55:37 +02:00
Ján Tomko
f048d08408 qemu: fix default spice password setting
Set spice password even if default VNC password hasn't been set.

https://bugzilla.redhat.com/show_bug.cgi?id=953720
(cherry picked from commit 4327df7eee)
2013-05-09 15:54:52 +02:00