libvirt.git
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)

Conflicts:
src/lxc/lxc_driver.c: OOM error reporting changes
src/util/virinitctl.c: OOM error reporting changes

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)

Backport fixed for OOM error reporting

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)

Backport fixed for OOM error reporting

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 agostorage: avoid short reads while chasing backing chain
Eric Blake [Tue, 5 Nov 2013 17:30:56 +0000]
storage: avoid short reads while chasing backing chain

Our backing file chain code was not very robust to an ill-timed
EINTR, which could lead to a short read causing us to randomly
treat metadata differently than usual.  But the existing
virFileReadLimFD forces an error if we don't read the entire
file, even though we only care about the header of the file.
So add a new virFile function that does what we want.

* src/util/virfile.h (virFileReadHeaderFD): New prototype.
* src/util/virfile.c (virFileReadHeaderFD): New function.
* src/libvirt_private.syms (virfile.h): Export it.
* src/util/virstoragefile.c (virStorageFileGetMetadataInternal)
(virStorageFileProbeFormatFromFD): Use it.

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

Conflicts:
src/util/virstoragefile.c: OOM error reporting & buffer signedness

4 years agomaint: fix comment typos
Eric Blake [Thu, 26 Sep 2013 21:40:34 +0000]
maint: fix comment typos

* src/lxc/lxc_controller.c (virLXCControllerSetupDisk): Fix typo.
* src/lxc/lxc_driver.c (lxcDomainAttachDeviceDiskLive): Likewise.

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

Conflicts:
src/lxc/lxc_controller.c: No userns support yet

4 years agoLXC: free dst before lxcDomainAttachDeviceDiskLive returns
Chen Hanxiao [Thu, 26 Sep 2013 06:01:52 +0000]
LXC: free dst before lxcDomainAttachDeviceDiskLive returns

Free dst before lxcDomainAttachDeviceDiskLive returns

Signed-off-by: Chen Hanxiao <chenhanxiao@cn.fujitsu.com>
(cherry picked from commit c82513acc2e1e0b035e5e53577a34fd68a530788)

4 years agoLXC: Free variable vroot in lxcDomainDetachDeviceHostdevUSBLive()
Hongwei Bi [Mon, 9 Sep 2013 06:05:20 +0000]
LXC: Free variable vroot in lxcDomainDetachDeviceHostdevUSBLive()

The variable vroot should be freed in label cleanup.

(cherry picked from commit 46c9bce4c8b0f2222cc50587ac968ced06eb1eff)

4 years agoFix minor typos in messages and docs
Yuri Chornoivan [Tue, 30 Jul 2013 08:21:11 +0000]
Fix minor typos in messages and docs

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

4 years agoLXC: hostdev: create parent directory for hostdev
Gao feng [Tue, 9 Jul 2013 10:16:20 +0000]
LXC: hostdev: create parent directory for hostdev

Create parent directroy for hostdev automatically when we
start a lxc domain or attach a hostdev to a lxc domain.

Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
(cherry picked from commit 468ee0bc4d6007f4af51235838cac7499fa22773)

4 years agoLXC: hostdev: introduce lxcContainerSetupHostdevCapsMakePath
Gao feng [Tue, 9 Jul 2013 10:15:11 +0000]
LXC: hostdev: introduce lxcContainerSetupHostdevCapsMakePath

This helper function is used to create parent directory for
the hostdev which will be added to the container. If the
parent directory of this hostdev doesn't exist, the mknod of
the hostdev will fail. eg with /dev/net/tun

Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
(cherry picked from commit c0d8c7c88579c612cf8a9776d276f8fd59d779f3)

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)

Conflicts:
src/conf/nwfilter_conf.c
          - virReportOOMError() in context of one hunk.
src/lxc/lxc_driver.c
          - functions renamed, and lxc object locking changed, creating
            a conflict in the context.

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 agoRemove use of virConnectPtr from all remaining nwfilter code
Daniel P. Berrange [Thu, 3 Oct 2013 11:51:48 +0000]
Remove use of virConnectPtr from all remaining nwfilter code

The virConnectPtr is passed around loads of nwfilter code in
order to provide it as a parameter to the callback registered
by the virt drivers. None of the virt drivers use this param
though, so it serves no purpose.

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

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

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

4 years agoDon't pass virConnectPtr in nwfilter 'struct domUpdateCBStruct'
Daniel P. Berrange [Thu, 3 Oct 2013 11:45:26 +0000]
Don't pass virConnectPtr in nwfilter 'struct domUpdateCBStruct'

The nwfilter driver only needs a reference to its private
state object, not a full virConnectPtr. Update the domUpdateCBStruct
struct to have a 'void *opaque' field instead of a virConnectPtr.

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

4 years agoRemove virConnectPtr arg from virNWFilterDefParse*
Daniel P. Berrange [Thu, 3 Oct 2013 11:35:34 +0000]
Remove virConnectPtr arg from virNWFilterDefParse*

None of the virNWFilterDefParse* methods require a virConnectPtr
arg, so just drop it

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

4 years agoDon't ignore errors parsing nwfilter rules
Daniel P. Berrange [Wed, 25 Sep 2013 14:26:58 +0000]
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 4f2094346d98f4ed6a2de115d204c166cc563496)

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:
1.1.0 had a framework for generating filter methods, but
nothing actually used them.  Therefore, the only leak in this
branch was the failure to honor connect:search_domains, and that
is fixed by backporting just the patch to remote_protocol.x to
properly annotate ACL categories, and to viraccessperms.h to
document the scope of the ACL.

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 agovirDomainEventCallbackListFree: Don't leak @list->callbacks
Michal Privoznik [Thu, 14 Nov 2013 09:33:30 +0000]
virDomainEventCallbackListFree: Don't leak @list->callbacks

The @list->callbacks is an array that is inflated whenever a new event
is added, e.g. via virDomainEventCallbackListAddID(). However, when we
are freeing the array, we free the items within it but forgot to
actually free it.

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

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

Conflicts:
src/lxc/lxc_driver.c

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)

Conflicts:
src/lxc/lxc_driver.c

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

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

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

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

https://bugzilla.redhat.com/show_bug.cgi?id=1029266
(cherry picked from commit b7829f959b33c6e32422222a9ed745c0da7dc696)

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

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

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

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

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

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

Signed-off-by: Zhou Yimin <zhouyimin@huawei.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
(cherry picked from commit 9712c2510ec87a87578576a407768380e250a6a4)

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

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

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

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

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

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

Signed-off-by: Daniel Hansel <daniel.hansel@linux.vnet.ibm.com>
(cherry picked from commit 6285c17f790a7e5027aed0207fc5d9eb9130cc0e)

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

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

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

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

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

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

Changes the default input unit to KiB to match documentation, fixing:
https://bugzilla.redhat.com/show_bug.cgi?id=1015689
(cherry picked from commit 3f029fb5319b9dc9cc2fbf8d1ba4505ee9e4b1e3)

Conflicts:
src/conf/domain_conf.c
src/lxc/lxc_container.c

4 years agoFix URI connect precedence
Martin Kletzander [Wed, 21 Aug 2013 09:02:42 +0000]
Fix URI connect precedence

Commit abfff210 changed the order of vshParseArgv() and vshInit() in
order to make fix debugging of parameter parsing.  However, vshInit()
did a vshReconnect() even though ctl->name wasn't set according to the
'-c' parameter yet.  In order to keep both issues fixed, I've split
the vshInit() into vshInitDebug() and vshInit().

One simple memleak of ctl->name is fixed as a part of this patch,
since it is related to the issue it's fixing.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=999323
(cherry picked from commit a0b6a36f9456dae895f50d344fd2d38be1167c58)

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

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

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

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

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

Signed-off-by: Liuji (Jeremy) <jeremy.liu@huawei.com>
(cherry picked from commit ef5d51d491356f1f4287aa3a8b908b183b6dd9aa)

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

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

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

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

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

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

(cherry picked from commit 609eb987c6cef9082486e66b666f7b9351b783ed)

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

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

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

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

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

  virConnectDomainEventDeregisterAny

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

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

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

Conflicts:
daemon/remote.c: int/size_t changes

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

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

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1009886
(cherry picked from commit 484cc3217b73b865f00bf42a9c12187b37200699)

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

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

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

    Also store user & group ID values in virIdentity

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

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

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

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

This was introduced in release v0.9.1 onwards by

  commit 158ba8730e44b7dd07a21ab90499996c5dec080a
  Author: Daniel P. Berrange <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 e7f400a110e2e3673b96518170bfea0855dd82c0)

Conflicts:
daemon/remote.c - context

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

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

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

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

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

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

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

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

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

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

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

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

The @qemunbd variable can be used uninitialized.

(cherry picked from commit 2dba0323ff0cec31bdcea9dd3b2428af297401f2)

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

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

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

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

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

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

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

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

This issue was introduced in the 1.1.0 release of libvirt

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

4 years agovirbitmap: Refactor virBitmapParse to avoid access beyond bounds of array
Peter Krempa [Fri, 16 Aug 2013 10:22:32 +0000]
virbitmap: Refactor virBitmapParse to avoid access beyond bounds of array

The virBitmapParse function was calling virBitmapIsSet() function that
requires the caller to check the bounds of the bitmap without checking
them. This resulted into crashes when parsing a bitmap string that was
exceeding the bounds used as argument.

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

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

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

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

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

(cherry picked from commit 47b9127e883677a0d60d767030a147450e919a25)

4 years agoSet the number of elements 0 in virNetwork*Clear
Ján Tomko [Fri, 26 Jul 2013 10:04:32 +0000]
Set the number of elements 0 in virNetwork*Clear

Decrementing it when it was already 0 causes an invalid free
in virNetworkDefUpdateDNSHost if virNetworkDNSHostDefParseXML
fails and virNetworkDNSHostDefClear gets called twice.

virNetworkForwardDefClear left the number untouched even if it
freed all the elements.
(cherry picked from commit c4e23388e6c7f769e45d1c162f880cd81e4d4d3b)

4 years agoDon't check validity of missing attributes in DNS SRV XML
Ján Tomko [Fri, 26 Jul 2013 10:11:21 +0000]
Don't check validity of missing attributes in DNS SRV XML

This fixes a crash if one of them is missing.

https://bugzilla.redhat.com/show_bug.cgi?id=988718
(cherry picked from commit 461fd86a661f32a9aa8044190b2a63b05290332f)

4 years agocgroup: reuse buffer for getline
Ján Tomko [Wed, 17 Jul 2013 08:56:05 +0000]
cgroup: reuse buffer for getline

Reuse the buffer for getline and track buffer allocation
separately from the string length to prevent unlikely
out-of-bounds memory access.

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

==404== 120 bytes in 1 blocks are definitely lost in loss record 1,344 of 1,671
==404==    at 0x4C2C71B: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==404==    by 0x906F862: getdelim (iogetdelim.c:68)
==404==    by 0x52A48FB: virCgroupPartitionNeedsEscaping (vircgroup.c:1136)
==404==    by 0x52A0FB4: virCgroupPartitionEscape (vircgroup.c:1171)
==404==    by 0x52A0EA4: virCgroupNewDomainPartition (vircgroup.c:1450)
(cherry picked from commit cc7329317fee6088055d7b09594c19f1b8fec5e3)

4 years agorbd: Do not free the secret if it is not set
Wido den Hollander [Tue, 16 Jul 2013 12:26:07 +0000]
rbd: Do not free the secret if it is not set

Not all RBD (Ceph) storage pools have cephx authentication turned on,
so "secret" might not be initialized.

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

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

Signed-off-by: Wido den Hollander <wido@widodh.nl>
(cherry picked from commit d58c8478443d49c6e702bbb2c56a567ef23f036f)

4 years agocaps: use -device for primary video when qemu >=1.6
Guannan Ren [Fri, 26 Jul 2013 12:53:47 +0000]
caps: use -device for primary video when qemu >=1.6

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

(cherry picked from commit e3f2686bdf6c94f658d8645c32a6039692753411)

4 years agovirSecurityManagerGenLabel: Skip seclabels without model
Michal Privoznik [Mon, 15 Jul 2013 13:50:29 +0000]
virSecurityManagerGenLabel: Skip seclabels without model

While generating seclabels, we check the seclabel stack if required
driver is in the stack. If not, an error is returned. However, it is
possible for a seclabel to not have any model set (happens with LXC
domains that have just <seclabel type='none'>). If that's the case,
we should just skip the iteration instead of calling STREQ(NULL, ...)
and SIGSEGV-ing subsequently.
(cherry picked from commit ba44dd2453d486e9eb8c6204f8d7c31d07007d8f)

4 years agolxcCapsInit: Allocate primary security driver unconditionally
Michal Privoznik [Mon, 15 Jul 2013 13:36:04 +0000]
lxcCapsInit: Allocate primary security driver unconditionally

Currently, if the primary security driver is 'none', we skip
initializing caps->host.secModels. This means, later, when LXC domain
XML is parsed and <seclabel type='none'/> is found (see
virSecurityLabelDefsParseXML), the model name is not copied to the
seclabel. This leads to subsequent crash in virSecurityManagerGenLabel
where we call STREQ() over the model (note, that we are expecting model
to be !NULL).
(cherry picked from commit 37d96498c6a9c3030bfad7dfbd273af5fbdd1845)

Conflicts:
src/lxc/lxc_conf.c

4 years agosecurity: fix deadlock with prefork
Eric Blake [Fri, 19 Jul 2013 15:07:19 +0000]
security: fix deadlock with prefork

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

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

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

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

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

4 years agosecurity_dac: compute supplemental groups before fork
Eric Blake [Fri, 12 Jul 2013 20:55:21 +0000]
security_dac: compute supplemental groups before fork

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

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

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

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

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

4 years agosecurity: framework for driver PreFork handler
Eric Blake [Wed, 17 Jul 2013 21:35:50 +0000]
security: framework for driver PreFork handler

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

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

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

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

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

4 years agoutil: make virSetUIDGID async-signal-safe
Eric Blake [Wed, 22 May 2013 02:59:10 +0000]
util: make virSetUIDGID async-signal-safe

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

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

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

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

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

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

Conflicts:
src/lxc/lxc_container.c - did not use setUIDGID before 1.1.0
src/util/virutil.c - oom handling changes not backported

4 years agoutil: add virGetGroupList
Eric Blake [Tue, 21 May 2013 23:47:48 +0000]
util: add virGetGroupList

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

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

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

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

4 years agoutil: improve user lookup helper
Eric Blake [Tue, 9 Jul 2013 23:57:48 +0000]
util: improve user lookup helper

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

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

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

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

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

Conflicts:
src/util/virutil.c - oom reporting changes not backported

4 years agoFix build with clang
Ján Tomko [Thu, 4 Jul 2013 09:35:59 +0000]
Fix build with clang

Partially revert cdd703f's revert of c163410, as linking with clang
with --param=ssp-buffer-size=4 still fails with:
"argument unused during compilation".

(cherry picked from commit 4b91dc24d1ed6a5ec6d5d9ab0d8522375dd77f3a)

4 years agomaint: update to latest gnulib
Eric Blake [Thu, 18 Jul 2013 21:47:41 +0000]
maint: update to latest gnulib

Upstream gnulib recently patched a bug in bootstrap, for projects
that use a different name than build-aux for a subdirectory.  We
don't, but it doesn't hurt to update.

* .gnulib: Update, for bootstrap fix.
* bootstrap: Sync to upstream.
* bootstrap.conf: Match upstream bug fix.

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

4 years agomaint: update to latest gnulib
Eric Blake [Thu, 11 Jul 2013 11:07:16 +0000]
maint: update to latest gnulib

Future patches need LGPLv2+ versions of some modules that had
recent license changes; but separating the gnulib update from
the actual use of the modules makes it easier to backport to
an older version while avoiding a submodule update (assuming,
of course, that the backport is to a system where glibc provides
adequate functionaliy without needing the gnulib module).

* .gnulib: Update to latest, for modules needed in later patches.

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

4 years agobuild: honor autogen.sh --no-git
Eric Blake [Wed, 3 Jul 2013 20:43:11 +0000]
build: honor autogen.sh --no-git

Based on a report by Chandrashekar Shastri, at
https://bugzilla.redhat.com/show_bug.cgi?id=979360

On systems where git cannot access the outside world, a developer
can instead arrange to get a copy of gnulib at the right commit
via side channels (such as NFS share drives), set GNULIB_SRCDIR,
then use ./autogen.sh --no-git.  In this setup, we will now
avoid direct use of git.  Of course, this means no automatic
gnulib updates when libvirt.git updates its submodule, but it
is expected that any developer in such a situation is already
prepared to deal with the fallout.

* .gnulib: Update to latest, for bootstrap.
* bootstrap: Synchronize from gnulib.
* autogen.sh (no_git): Avoid git when requested.
* cfg.mk (_update_required): Skip automatic rerun of bootstrap if
we can't use git.
* docs/compiling.html.in: Document this setup.
* docs/hacking.html.in: Mention this.
* HACKING: Regenerate.

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

4 years agomaint: update to latest gnulib
Eric Blake [Tue, 2 Jul 2013 23:26:42 +0000]
maint: update to latest gnulib

The latest mingw headers on Fedora 19 fail to build with gnulib
without an update.

Meanwhile, now that upstream gnulib has better handling of -W
probing for clang, we can drop some of our own solutions in
favor of upstream; thus this reverts commit c1634100, "Correctly
detect warning flags with clang".

* .gnulib: Update to latest, for mingw and clang.

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

4 years agoqemu: Fix double free of returned JSON array in qemuAgentGetVCPUs()
Peter Krempa [Tue, 16 Jul 2013 13:39:06 +0000]
qemu: Fix double free of returned JSON array in qemuAgentGetVCPUs()

CVE-2013-4153

A part of the returned monitor response was freed twice and caused
crashes of the daemon when using guest agent cpu count retrieval.

 # virsh vcpucount dom --guest

Introduced in v1.0.6-48-gc6afcb0

(cherry picked from commit dfc692350a04a70b4ca65667c30869b3bfdaf034)

4 years agoqemu: Prevent crash of libvirtd without guest agent configuration
Alex Jia [Tue, 16 Jul 2013 09:30:20 +0000]
qemu: Prevent crash of libvirtd without guest agent configuration

CVE-2013-4154

If users haven't configured guest agent then qemuAgentCommand() will
dereference a NULL 'mon' pointer, which causes crash of libvirtd when
using agent based cpu (un)plug.

With the patch, when the qemu-ga service isn't running in the guest,
a expected error "error: Guest agent is not responding: Guest agent
not available for now" will be raised, and the error "error: argument
unsupported: QEMU guest agent is not configured" is raised when the
guest hasn't configured guest agent.

GDB backtrace:

 (gdb) bt
 #0  virNetServerFatalSignal (sig=11, siginfo=<value optimized out>, context=<value optimized out>) at rpc/virnetserver.c:326
 #1  <signal handler called>
 #2  qemuAgentCommand (mon=0x0, cmd=0x7f39300017b0, reply=0x7f394b090910, seconds=-2) at qemu/qemu_agent.c:975
 #3  0x00007f39429507f6 in qemuAgentGetVCPUs (mon=0x0, info=0x7f394b0909b8) at qemu/qemu_agent.c:1475
 #4  0x00007f39429d9857 in qemuDomainGetVcpusFlags (dom=<value optimized out>, flags=9) at qemu/qemu_driver.c:4849
 #5  0x00007f3957dffd8d in virDomainGetVcpusFlags (domain=0x7f39300009c0, flags=8) at libvirt.c:9843

How to reproduce?

 # To start a guest without guest agent configuration
 # then run the following cmdline

 # virsh vcpucount foobar --guest
 error: End of file while reading data: Input/output error
 error: One or more references were leaked after disconnect from the hypervisor
 error: Failed to reconnect to the hypervisor

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

Signed-off-by: Alex Jia <ajia@redhat.com>
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
(cherry picked from commit 96518d4316b711c72205117f8d5c967d5127bbb6)

4 years agostorage: return -1 when fs pool can't be mounted
Ján Tomko [Thu, 11 Jul 2013 10:36:59 +0000]
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 13fde7ceab556804dc6cfb3e56938fb948ffe83d)

4 years agoFix crash when multiple event callbacks were registered
Ján Tomko [Tue, 2 Jul 2013 13:17:09 +0000]
Fix crash when multiple event callbacks were registered

CVE-2013-2230

Don't overwrite the callback ID returned by
virDomainEventStateRegisterID in ret by 0.

Introduced by abf75aea.
(cherry picked from commit f38c8185f97720ecae7ef2291fbaa5d6b0209e17)

4 years agoqemu: fix double free in qemuMigrationPrepareDirect
Ján Tomko [Wed, 10 Jul 2013 10:33:29 +0000]
qemu: fix double free in qemuMigrationPrepareDirect

Remove assignment of the string freed by virURIFree
to hostname, since it's not used anywhere.

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

https://bugzilla.redhat.com/show_bug.cgi?id=977961
(cherry picked from commit 5744d96f211160d406ec0498c2f814a67d1a3fc8)

4 years agoUnlock the storage volume object after looking it up
Ján Tomko [Thu, 4 Jul 2013 12:41:46 +0000]
Unlock the storage volume object after looking it up

Introduced by c930410.

https://bugzilla.redhat.com/show_bug.cgi?id=980676
(cherry picked from commit fe89fd3b4071242ce9bbae6e1178fee30dc2f4f9)

4 years agoscsi: Fix construction of sysfs device path
Viktor Mihajlovski [Mon, 8 Jul 2013 16:57:58 +0000]
scsi: Fix construction of sysfs device path

The device bus value was used instead of the device target when
building the sysfs device path. Trivial.

Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com>
(cherry picked from commit 2c94e00c6098662661cd67a10e982a32b6054a3c)

4 years agobuild: don't ship access syms files in tarball
Eric Blake [Tue, 2 Jul 2013 16:28:20 +0000]
build: don't ship access syms files in tarball

On a mingw VPATH build (such as done by ./autobuild.sh), the tarball
created by 'make dist' was including generated files.  The VPATH
rules were then seeing that the tarball files were up-to-date, and
not regenerating files locally, leading to this failure:

  GEN      libvirt.syms
cat: libvirt_access.syms: No such file or directory
cat: libvirt_access_qemu.syms: No such file or directory
cat: libvirt_access_lxc.syms: No such file or directory
make: *** [libvirt.syms] Error 1

We already have a category for generated sym files, which are
intentionally not part of the tarball; stick the access sym
files in that category.  The rearrange the declarations a bit
to make it harder to repeat the problem, dropping things that
are now redundant (for example, BUILT_FILES already includes
GENERATED_SYM_FILES, so it does not also need to call out
ACCESS_DRIVER_SYM_FILES).

* src/Makefile.am (USED_SYM_FILES): Don't include generated files.
(GENERATED_SYM_FILES): Access syms files are generated.
(libvirt.syms): Include access syms files here.
(ACCESS_DRIVER_SYMFILES): Rename...
(ACCESS_DRIVER_SYM_FILES): ...for consistency.

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

4 years agobuild: work around mingw header pollution
Eric Blake [Tue, 2 Jul 2013 12:09:30 +0000]
build: work around mingw header pollution

On Fedora 18, when cross-compiling to mingw with the mingw*-dbus
packages installed, compilation fails with:

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

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

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

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

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

4 years agoqemuNodeDeviceDetachFlags: Avoid use of uninitialized variables
Michal Privoznik [Tue, 2 Jul 2013 09:20:53 +0000]
qemuNodeDeviceDetachFlags: Avoid use of uninitialized variables

After abf75aea24 the compiler screams:

qemu/qemu_driver.c: In function 'qemuNodeDeviceDetachFlags':
qemu/qemu_driver.c:10693:9: error: 'domain' may be used uninitialized in this function [-Werror=maybe-uninitialized]
     pci = virPCIDeviceNew(domain, bus, slot, function);
         ^
qemu/qemu_driver.c:10693:9: error: 'bus' may be used uninitialized in this function [-Werror=maybe-uninitialized]
qemu/qemu_driver.c:10693:9: error: 'slot' may be used uninitialized in this function [-Werror=maybe-uninitialized]
qemu/qemu_driver.c:10693:9: error: 'function' may be used uninitialized in this function [-Werror=maybe-uninitialized]

Since the other functions qemuNodeDeviceReAttach and qemuNodeDeviceReset
looks exactly the same, I've initialized the variables there as well.
However, I am still wondering why those functions don't matter to gcc
while the first one does.
(cherry picked from commit bc09c5d3355483bbf4cec55ca82689ae67af6d44)

4 years agoqemu: fix return value of qemuDomainBlockPivot on errors
Ján Tomko [Mon, 1 Jul 2013 10:41:34 +0000]
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 c34107dfd3a25232255e6d6f559b1306ef99bb3b)

4 years agobuild: configure must not affect tarball contents
Eric Blake [Mon, 1 Jul 2013 22:48:11 +0000]
build: configure must not affect tarball contents

On mingw, configure sets the name of the lxc symfile to
libvirt_lxc.defs rather than libvirt_lxc.syms.  But tarballs
must be arch-independent, regardless of the configure options
used for the tree where we ran 'make dist'.  This led to the
following failure in autobuild.sh:

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

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

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

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

4 years agobuild: avoid build failure without gnutls
Eric Blake [Mon, 1 Jul 2013 19:21:57 +0000]
build: avoid build failure without gnutls

Found while trying to cross-compile to mingw:

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

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

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

4 years agoRelease of libvirt-1.1.0 v1.1.0
Daniel Veillard [Mon, 1 Jul 2013 09:22:49 +0000]
Release of libvirt-1.1.0

* configure.ac docs/news.html.in libvirt.spec.in: updated for the release
* po/*.po*: updated localizations and regenerated

4 years agoCrash of libvirtd by unprivileged user in virConnectListAllInterfaces CVE-2013-2218
Daniel P. Berrange [Fri, 28 Jun 2013 12:21:33 +0000]
Crash of libvirtd by unprivileged user in virConnectListAllInterfaces

On Thu, Jun 27, 2013 at 03:56:42PM +0100, Daniel P. Berrange wrote:
> Hi Security Team,
>
> I've discovered a way for an unprivileged user with a readonly connection
> to libvirtd, to crash the daemon.

Ok, the final patch for this is issue will be the simpler variant that
Eric suggested

The embargo can be considered to be lifted on Monday July 1st, at
0900 UTC

The following is the GIT change that DV or myself will apply to libvirt
GIT master immediately before the 1.1.0 release:

>From 177b4165c531a4b3ba7f6ab6aa41dca9ceb0b8cf Mon Sep 17 00:00:00 2001
From: "Daniel P. Berrange" <berrange@redhat.com>
Date: Fri, 28 Jun 2013 10:48:37 +0100
Subject: [PATCH] CVE-2013-2218: Fix crash listing network interfaces with
 filters

The virConnectListAllInterfaces method has a double-free of the
'struct netcf_if' object when any of the filtering flags cause
an interface to be skipped over. For example when running the
command 'virsh iface-list --inactive'

This is a regression introduced in release 1.0.6 by

  commit 7ac2c4fe624f30f2c8270116513fa2ddab07631f
  Author: Guannan Ren <gren@redhat.com>
  Date:   Tue May 21 21:29:38 2013 +0800

    interface: list all interfaces with flags == 0

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

4 years agopci: initialize virtual_functions array pointer to avoid segfault
Laine Stump [Mon, 1 Jul 2013 03:52:43 +0000]
pci: initialize virtual_functions array pointer to avoid segfault

This fixes https://bugzilla.redhat.com/show_bug.cgi?id=971325

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

The final error message before the crash would be:

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

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

4 years agonode device driver: update driver name during dumpxml
Laine Stump [Sun, 30 Jun 2013 18:49:21 +0000]
node device driver: update driver name during dumpxml

This fixes:

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

The node device driver was written with the assumption that udev would
use a "change" event to notify libvirt of any change to device status
(including the name of the driver it was bound to). It turns out this
is not the case (see Comment 4 of BZ 979290). That means that a
dumpxml for a device would always show whatever driver happened to be
bound at the time libvirt was started (when the node device cache was
built).

There was already code in the driver (for the benefit of the HAL
backend) that updated the driver name from sysfs each time a device's
info was retrieved from the cache. This patch just enables that manual
update for the udev backend as well.

4 years agoDocument security reporting & handling process
Daniel P. Berrange [Tue, 4 Jun 2013 10:06:01 +0000]
Document security reporting & handling process

Historically security issues in libvirt have been primarily
triaged & fixed by the Red Hat libvirt members & Red Hat
security team, who then usually notify other vendors via
appropriate channels. There have been a number of times
when vendors have not been properly notified ahead of
announcement. It has also disadvantaged community members
who have to backport fixes to releases for which there are
no current libvirt stable branches.

To address this, we want to make the libvirt security process
entirely community focused / driven. To this end I have setup
a new email address "libvirt-security@redhat.com" for end
users to report bugs which have (possible) security implications.

This email addr is backed by an invitation only, private
archive, mailing list. The intent is for the list membership
to comprise a subset of the libvirt core team, along with any
vendor security team engineers who wish to participate in a
responsible disclosure process for libvirt. Members of the
list will be responsible for analysing the problem to determine
if a security issue exists and then issue fixes for all current
official stable branches & git master.

I am proposing the following libvirt core team people as
members of the security team / list (all cc'd):

   Daniel Berrange (Red Hat)
   Eric Blake (Red Hat)
   Jiri Denemar (Red Hat)
   Daniel Veillard (Red Hat)
   Jim Fehlig (SUSE)
   Doug Goldstein (Gentoo)
   Guido Günther (Debian)

We don't have anyone from Ubuntu on the libvirt core team.
Serge Hallyn is the most frequent submitter of patches from
Ubuntu in recent history, so I'd like to invite him to join.
Alternatively, Serge, feel free to suggest someone else to
represent Ubuntu's interests.

If any other vendors/distros have security people who are
responsible for dealing with libvirt security issues, and
want to join to get early disclosure of issues, they can
suggest people. Existing security team members will vet /
approve such requests to ensure they are genuine.

Anyone on the team / list will be **required** to honour any
embargo period agreed between members for non-public issues
that are reported. The aim will be to have a maximum 2 week
embargo period in the common case, extendable to 1 month if
there is sufficient justification made. If anyone feels they
are unable to follow such an embargo process for whatever
reason, please decline membership of the security list/team.

The patch which follows puts up some docs on the website
about all of this....

Document how to report security bugs and the process that
will be used for addressing them.

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

4 years agospec: require xen-devel for libxl driver
Eric Blake [Sat, 29 Jun 2013 20:50:14 +0000]
spec: require xen-devel for libxl driver

When using 'rpmbuild --define "_without_xen 1"', but on a new enough
Fedora where %{with_libxl} still gets set to 1 by default, the
build dependencies were incomplete, which could result in 'make rpm'
failing because ./configure failed to build the libxl driver.

* libvirt.spec.in (BuildRequires): Fix xen-devel condition.

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

4 years agoResolve valgrind errors for nodedev cap parsing
John Ferlan [Fri, 28 Jun 2013 19:07:05 +0000]
Resolve valgrind errors for nodedev cap parsing

There were two errors, one as a direct result of commit id '8807b285'
and the other from cut-n-paste

TEST: nodedevxml2xmltest
      ..............                           14  OK
==25735== 3 bytes in 1 blocks are definitely lost in loss record 1 of 24
==25735==    at 0x4A0887C: malloc (vg_replace_malloc.c:270)
==25735==    by 0x344D2AF275: xmlStrndup (in /usr/lib64/libxml2.so.2.9.1)
==25735==    by 0x4D0C767: virNodeDeviceDefParseNode (node_device_conf.c:997)
==25735==    by 0x4D0D3D2: virNodeDeviceDefParse (node_device_conf.c:1337)
==25735==    by 0x401CA4: testCompareXMLToXMLHelper (nodedevxml2xmltest.c:28)
==25735==    by 0x402B2F: virtTestRun (testutils.c:158)
==25735==    by 0x401B27: mymain (nodedevxml2xmltest.c:81)
==25735==    by 0x40316A: virtTestMain (testutils.c:722)
==25735==    by 0x37C1021A04: (below main) (libc-start.c:225)
==25735==
==25735== 16 bytes in 1 blocks are definitely lost in loss record 10 of 24
==25735==    at 0x4A08A6E: realloc (vg_replace_malloc.c:662)
==25735==    by 0x4C7385E: virReallocN (viralloc.c:184)
==25735==    by 0x4C73906: virExpandN (viralloc.c:214)
==25735==    by 0x4C73B4A: virInsertElementsN (viralloc.c:324)
==25735==    by 0x4D0C84C: virNodeDeviceDefParseNode (node_device_conf.c:1026)
==25735==    by 0x4D0D3D2: virNodeDeviceDefParse (node_device_conf.c:1337)
==25735==    by 0x401CA4: testCompareXMLToXMLHelper (nodedevxml2xmltest.c:28)
==25735==    by 0x402B2F: virtTestRun (testutils.c:158)
==25735==    by 0x401B27: mymain (nodedevxml2xmltest.c:81)
==25735==    by 0x40316A: virtTestMain (testutils.c:722)
==25735==    by 0x37C1021A04: (below main) (libc-start.c:225)
==25735==
PASS: nodedevxml2xmltest

The first error was resolved by adding a missing VIR_FREE(numberStr); in
the new function virNodeDevCapPciDevIommuGroupParseXML().

The second error was a bit more opaque as the error was a result of copying
the free methodolgy of the existing code in virNodeDevCapsDefFree(). The code
would free each of the entries in the array, but not the memory for the
array itself.  Added the necessary VIR_FREE(data->pci_dev.iommuGroupDevices)
and while at it added the missing VIR_FREE(data->pci_dev.virtual_functions)
although there wasn't a test that tripped across it (thus it's been lurking
since commit id 'a010165d').

4 years agoResolve valgrind error in remoteConfigGetStringList()
John Ferlan [Fri, 28 Jun 2013 17:10:10 +0000]
Resolve valgrind error in remoteConfigGetStringList()

Commit id 'ed3bac71' introduced the following:

TEST: libvirtdconftest
      ........................................ 40  OK
==25875== 690 (480 direct, 210 indirect) bytes in 30 blocks are definitely lost in loss record 18 of 24
==25875==    at 0x4A06B6F: calloc (vg_replace_malloc.c:593)
==25875==    by 0x4C737DF: virAllocN (viralloc.c:152)
==25875==    by 0x403BC8: remoteConfigGetStringList (libvirtd-config.c:74)
==25875==    by 0x4042CF: daemonConfigLoadOptions (libvirtd-config.c:382)
==25875==    by 0x4052F5: daemonConfigLoadData (libvirtd-config.c:479)
==25875==    by 0x40222C: testCorrupt (libvirtdconftest.c:112)
==25875==    by 0x40321F: virtTestRun (testutils.c:158)
==25875==    by 0x401FEE: mymain (libvirtdconftest.c:228)
==25875==    by 0x40385A: virtTestMain (testutils.c:722)
==25875==    by 0x37C1021A04: (below main) (libc-start.c:225)
==25875==
PASS: libvirtdconftest

4 years agoResolve valgrind error in virStorageBackendCreateQemuImgCmd()
John Ferlan [Fri, 28 Jun 2013 16:18:01 +0000]
Resolve valgrind error in virStorageBackendCreateQemuImgCmd()

Commit id '53d5967c' introduced the following:

TEST: storagevolxml2argvtest
      ..............                           14  OK
==25636== 358 (264 direct, 94 indirect) bytes in 1 blocks are definitely lost in loss record 67 of 75
==25636==    at 0x4A06B6F: calloc (vg_replace_malloc.c:593)
==25636==    by 0x4C95791: virAlloc (viralloc.c:124)
==25636==    by 0x4CA0BB4: virCommandNewArgs (vircommand.c:805)
==25636==    by 0x4CA0C88: virCommandNew (vircommand.c:789)
==25636==    by 0x408602: virStorageBackendCreateQemuImgCmd (storage_backend.c:849)
==25636==    by 0x405427: testCompareXMLToArgvHelper (storagevolxml2argvtest.c:61)
==25636==    by 0x4064DF: virtTestRun (testutils.c:158)
==25636==    by 0x40516F: mymain (storagevolxml2argvtest.c:195)
==25636==    by 0x406B1A: virtTestMain (testutils.c:722)
==25636==    by 0x37C1021A04: (below main) (libc-start.c:225)
==25636==
PASS: storagevolxml2argvtest

4 years agoResolve valgrind error in virNetDevVlanParse()
John Ferlan [Fri, 28 Jun 2013 16:04:37 +0000]
Resolve valgrind error in virNetDevVlanParse()

Commit '861d4056' introduced the following:

TEST: networkxml2xmltest
      ..................                       18  OK
==25504== 7 bytes in 1 blocks are definitely lost in loss record 5 of 23
==25504==    at 0x4A0887C: malloc (vg_replace_malloc.c:270)
==25504==    by 0x37C1085D71: strdup (strdup.c:42)
==25504==    by 0x4CB835F: virStrdup (virstring.c:546)
==25504==    by 0x4CC5179: virXPathString (virxml.c:90)
==25504==    by 0x4CC75C2: virNetDevVlanParse (netdev_vlan_conf.c:78)
==25504==    by 0x4CF928A: virNetworkPortGroupParseXML (network_conf.c:1555)
==25504==    by 0x4CFE385: virNetworkDefParseXML (network_conf.c:2049)
==25504==    by 0x4D0113B: virNetworkDefParseNode (network_conf.c:2273)
==25504==    by 0x4D01254: virNetworkDefParse (network_conf.c:2234)
==25504==    by 0x401E80: testCompareXMLToXMLHelper (networkxml2xmltest.c:32)
==25504==    by 0x402D4F: virtTestRun (testutils.c:158)
==25504==    by 0x401CE9: mymain (networkxml2xmltest.c:110)
==25504==
PASS: networkxml2xmltest

Also changed the label from error to cleanup and adjusted code since it's
all one exit path

4 years agodoc: Fix reference to #elementsUSB
Philipp Hahn [Fri, 28 Jun 2013 17:06:30 +0000]
doc: Fix reference to #elementsUSB

aae0fc2a922b3e31dae7648c547fca2ac2587625 removed the #elementsUSB anchor
but did not update the links to point to the new section #elementsHostDev.

Signed-off-by: Philipp Hahn <hahn@univention.de>

4 years agoConditionalize use of IF_MAXUNIT in virnetdevtap.c
Daniel P. Berrange [Fri, 28 Jun 2013 14:17:41 +0000]
Conditionalize use of IF_MAXUNIT in virnetdevtap.c

The IF_MAXUNIT macro is not present on all BSDs, so
make its use conditional, to avoid breaking OS-X.

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