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: NDaniel P. Berrange <berrange@redhat.com>
(cherry picked from commit 6e5c79a1)

Add virRWLock backed up by a POSIX rwlock primitive
Signed-off-by: NDaniel P. Berrange <berrange@redhat.com>
(cherry picked from commit c065984b)

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: NDaniel P. Berrange <berrange@redhat.com>
(cherry picked from commit 999d72fb)

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: NDaniel P. Berrange <berrange@redhat.com>
(cherry picked from commit ebca369e)

None of the virNWFilterDefParse* methods require a virConnectPtr
arg, so just drop it
Signed-off-by: NDaniel P. Berrange <berrange@redhat.com>
(cherry picked from commit b77b16ce)

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: NDaniel P. Berrange <berrange@redhat.com>
(cherry picked from commit 4f209434)

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

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

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

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

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

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

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

Ever since ACL filtering was added in commit 76397360 (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 a93cd08f (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: NEric Blake <eblake@redhat.com>
(cherry picked from commit f9f56340)

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

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

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

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: NMichal Privoznik <mprivozn@redhat.com>
(cherry picked from commit ea13a759)

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

When writing commit 173c2914, 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: NJiri Denemark <jdenemar@redhat.com>
(cherry picked from commit 066c8ef6)

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

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

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

(cherry picked from commit ff5f30b6)

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

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

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

by, in libxlDomainGetNumaParameters(), calling libxl_bitmap_init() as soon as
possible, which avoids getting to 'cleanup:', where libxl_bitmap_dispose()
happens, without having initialized the nodemap, and hence crashing after some
invalid free()-s:

 # ./daemon/libvirtd -v
 *** Error in `/home/xen/libvirt.git/daemon/.libs/lt-libvirtd': munmap_chunk(): invalid pointer: 0x00007fdd42592666 ***
 ======= Backtrace: =========
 /lib64/libc.so.6(+0x7bbe7)[0x7fdd3f767be7]
 /lib64/libxenlight.so.4.3(libxl_bitmap_dispose+0xd)[0x7fdd2c88c045]
 /home/xen/libvirt.git/daemon/.libs/../../src/.libs/libvirt_driver_libxl.so(+0x12d26)[0x7fdd2caccd26]
 /home/xen/libvirt.git/src/.libs/libvirt.so.0(virDomainGetNumaParameters+0x15c)[0x7fdd4247898c]
 /home/xen/libvirt.git/daemon/.libs/lt-libvirtd(+0x1d9a2)[0x7fdd42ecc9a2]
 /home/xen/libvirt.git/src/.libs/libvirt.so.0(virNetServerProgramDispatch+0x3da)[0x7fdd424e9eaa]
 /home/xen/libvirt.git/src/.libs/libvirt.so.0(+0x1a6f38)[0x7fdd424e3f38]
 /home/xen/libvirt.git/src/.libs/libvirt.so.0(+0xa81e5)[0x7fdd423e51e5]
 /home/xen/libvirt.git/src/.libs/libvirt.so.0(+0xa783e)[0x7fdd423e483e]
 /lib64/libpthread.so.0(+0x7c53)[0x7fdd3febbc53]
 /lib64/libc.so.6(clone+0x6d)[0x7fdd3f7e1dbd]
Signed-off-by: NDario Faggili <dario.faggioli@citrix.com>
Cc: Jim Fehlig <jfehlig@suse.com>
Cc: Ian Jackson <Ian.Jackson@eu.citrix.com>
(cherry picked from commit f9ee91d3)

Conflicts:
	src/libxl/libxl_driver.c

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: NMartin Kletzander <mkletzan@redhat.com>
(cherry picked from commit 9faf3f29)

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: NMartin Kletzander <mkletzan@redhat.com>
(cherry picked from commit f8c1cb90)

Since 86d90b3a (yes, my patch; again) we are supporting NBD storage
migration. However, on error recovery path we got the steps reversed.
The correct order is: return NBD port to the virPortAllocator and then
either unlock the vm or remove it from the driver. Not vice versa.

==11192== Invalid write of size 4
==11192==    at 0x11488559: qemuMigrationPrepareAny (qemu_migration.c:2459)
==11192==    by 0x11488EA6: qemuMigrationPrepareDirect (qemu_migration.c:2652)
==11192==    by 0x114D1509: qemuDomainMigratePrepare3Params (qemu_driver.c:10332)
==11192==    by 0x519075D: virDomainMigratePrepare3Params (libvirt.c:7290)
==11192==    by 0x1502DA: remoteDispatchDomainMigratePrepare3Params (remote.c:4798)
==11192==    by 0x12DECA: remoteDispatchDomainMigratePrepare3ParamsHelper (remote_dispatch.h:5741)
==11192==    by 0x5212127: virNetServerProgramDispatchCall (virnetserverprogram.c:435)
==11192==    by 0x5211C86: virNetServerProgramDispatch (virnetserverprogram.c:305)
==11192==    by 0x520A8FD: virNetServerProcessMsg (virnetserver.c:165)
==11192==    by 0x520A9E1: virNetServerHandleJob (virnetserver.c:186)
==11192==    by 0x50DA78F: virThreadPoolWorker (virthreadpool.c:144)
==11192==    by 0x50DA11C: virThreadHelper (virthreadpthread.c:161)
==11192==  Address 0x1368baa0 is 576 bytes inside a block of size 688 free'd
==11192==    at 0x4A07F5C: free (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==11192==    by 0x5079A2F: virFree (viralloc.c:580)
==11192==    by 0x11456C34: qemuDomainObjPrivateFree (qemu_domain.c:267)
==11192==    by 0x50F41B4: virDomainObjDispose (domain_conf.c:2034)
==11192==    by 0x50C2991: virObjectUnref (virobject.c:262)
==11192==    by 0x50F4CFC: virDomainObjListRemove (domain_conf.c:2361)
==11192==    by 0x1145C125: qemuDomainRemoveInactive (qemu_domain.c:2087)
==11192==    by 0x11488520: qemuMigrationPrepareAny (qemu_migration.c:2456)
==11192==    by 0x11488EA6: qemuMigrationPrepareDirect (qemu_migration.c:2652)
==11192==    by 0x114D1509: qemuDomainMigratePrepare3Params (qemu_driver.c:10332)
==11192==    by 0x519075D: virDomainMigratePrepare3Params (libvirt.c:7290)
==11192==    by 0x1502DA: remoteDispatchDomainMigratePrepare3Params (remote.c:4798)
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
(cherry picked from commit 1f2f879e)

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

Avoid people introducing security flaws in their apps by
forbidding the use of libvirt.so in setuid programs, with
a check in virInitialize.
Signed-off-by: NDaniel P. Berrange <berrange@redhat.com>
(cherry picked from commit 9cd6a57d)

Most of the usage of getuid()/getgid() is in cases where we are
considering what privileges we have. As such the code should be
using the effective IDs, not real IDs.
Signed-off-by: NDaniel P. Berrange <berrange@redhat.com>
(cherry picked from commit 9b0af092)

We already have stubs for getuid, geteuid, getgid but
not for getegid. Something in gnulib already does a
check for it during configure, so we already have the
HAVE_GETEGID macro defined.
Signed-off-by: NDaniel P. Berrange <berrange@redhat.com>
(cherry picked from commit c566fa1a)

We don't want setuid programs automatically spawning libvirtd,
so disable any use of autostart when setuid.
Signed-off-by: NDaniel P. Berrange <berrange@redhat.com>
(cherry picked from commit 171bb129)

We don't know enough about quality of external libraries used
for non-UNIX transports, nor do we want to spawn external
commands when setuid. Restrict to the bare minimum which is
UNIX transport for local usage. Users shouldn't need to be
running setuid if connecting to remote hypervisors in any
case.
Signed-off-by: NDaniel P. Berrange <berrange@redhat.com>
(cherry picked from commit e22b0232)

The use of getenv is typically insecure, and we want people
to use our wrappers, to force them to think about setuid
needs.
Signed-off-by: NDaniel P. Berrange <berrange@redhat.com>
(cherry picked from commit 71b21f12)

Unconditional use of getenv is not secure in setuid env.
While not all libvirt code runs in a setuid env (since
much of it only exists inside libvirtd) this is not always
clear to developers. So make all the code paranoid, even
if it only ever runs inside libvirtd.
Signed-off-by: NDaniel P. Berrange <berrange@redhat.com>
(cherry picked from commit 1e4a02bd)

When running setuid, we must be careful about what env vars
we allow commands to inherit from us. Replace the
virCommandAddEnvPass function with two new ones which do
filtering

  virCommandAddEnvPassAllowSUID
  virCommandAddEnvPassBlockSUID

And make virCommandAddEnvPassCommon use the appropriate
ones
Signed-off-by: NDaniel P. Berrange <berrange@redhat.com>
(cherry picked from commit 9b8f307c)

Conflicts:
	src/qemu/qemu_command.c

In Fedora 20, libvirt_lxc crashes immediately at startup with a
trace

 #0  0x00007f0cddb653ec in free () from /lib64/libc.so.6
 #1  0x00007f0ce0e16f4a in virFree (ptrptr=ptrptr@entry=0x7f0ce1830058) at util/viralloc.c:580
 #2  0x00007f0ce0e2764b in virResetError (err=0x7f0ce1830030) at util/virerror.c:354
 #3  0x00007f0ce0e27a5a in virResetLastError () at util/virerror.c:387
 #4  0x00007f0ce0e28858 in virEventRegisterDefaultImpl () at util/virevent.c:233
 #5  0x00007f0ce0db47c6 in main (argc=11, argv=0x7fff4596c328) at lxc/lxc_controller.c:2352

Normally virInitialize calls virErrorInitialize and
virThreadInitialize, but we don't link to libvirt.so
in libvirt_lxc, and nor did we ever call the error
or thread initializers.

I have absolutely no idea how this has ever worked, let alone
what caused it to stop working in Fedora 20.

In addition not all code paths from virLogSetFromEnv will
ensure virLogInitialize is called correctly, which is another
possible crash scenario.
Signed-off-by: NDaniel P. Berrange <berrange@redhat.com>
(cherry picked from commit 97973ebb)

The log message regex has been

[0-9]{4}-[0-9]{2}-[0-9]{2} [0-9]{2}:[0-9]{2}:[0-9]{2}\.[0-9]{3}\+[0-9]{4}: [0-9]+: debug|info|warning|error :

The precedence of '|' is high though, so this is equivalent to matching

   [0-9]{4}-[0-9]{2}-[0-9]{2} [0-9]{2}:[0-9]{2}:[0-9]{2}\.[0-9]{3}\+[0-9]{4}: [0-9]+: debug

Or

   info

Or

   warning

Or

   error :

Which is clearly not what it should have done. This caused the code to
skip over things which are not log messages. The solution is to simply
add brackets.

A test case is also added to validate correctness.
Signed-off-by: NDaniel P. Berrange <berrange@redhat.com>
(cherry picked from commit 5787f0b9)

Many people will not want the setuid virt-login-shell binary
installed by default, so move it into a separate sub-RPM
named libvirt-login-shell. This RPM is only generated if
LXC is enabled
Signed-off-by: NDaniel P. Berrange <berrange@redhat.com>
(cherry picked from commit 8adc9269)

'make rpm' failed if ~/.rpmmacros contains '%_without_lxc 1',
which simulates the case of not having lxc available.

RPM build errors:
    File not found: /home/eblake/rpmbuild/BUILDROOT/libvirt-1.1.1-1.fc19.x86_64/etc/libvirt/virt-login-shell.conf
    File not found by glob: /home/eblake/rpmbuild/BUILDROOT/libvirt-1.1.1-1.fc19.x86_64/usr/share/man/man1/virt-login-shell.1*
    File not found: /home/eblake/rpmbuild/BUILDROOT/libvirt-1.1.1-1.fc19.x86_64/usr/bin/virt-login-shell
make: *** [rpm] Error 1

Reported by Dan Berrange.

* libvirt.spec.in: Mark virt-login-shell as conditional on lxc.
Signed-off-by: NEric Blake <eblake@redhat.com>
(cherry picked from commit d42906fd)

The virt-login-shell binary shouldn't need to execute programs
relying on $PATH, but just in case set a fixed $PATH value
of /bin:/usr/bin
Signed-off-by: NDaniel P. Berrange <berrange@redhat.com>
(cherry picked from commit d665003d)

Commit d76227be added functions virDomainCreateWithFiles and
virDomainCreateXMLWithFiles, but there was a little piece missing in
python bindings.  This patch fixes proper passing of file descriptors
in the overwrites of these functions.

On systems where gnutls uses libgcrypt, I'm seeing the following
build failure

libvirt.c:314: error: variable 'virTLSThreadImpl' has initializer but incomplete type
libvirt.c:319: error: 'GCRY_THREAD_OPTION_PTHREAD' undeclared here (not in a function)
...

Fix by undefining WITH_GNUTLS_GCRYPT in config-post.h

After commit 3e2f27e1, I've noticed build failures of virt-login-shell
when libapparmor-devel is installed on the build host

CCLD     virt-login-shell
../src/.libs/libvirt-setuid-rpc-client.a(libvirt_setuid_rpc_client_la-vircommand.o):
In function `virExec':
/home/jfehlig/virt/upstream/libvirt/src/util/vircommand.c:653: undefined
reference to `aa_change_profile'
collect2: error: ld returned 1 exit status

I was about to commit an easy fix under the build-breaker rule
(build-fix-1.patch), but thought to extend the notion of SECDRIVER_LIBS
to SECDRIVER_CFLAGS, and use both throughout src/Makefile.am where it
makes sense (build-fix-2.patch).

Should I just stick with the simple fix, or is something along the lines
of patch 2 preferred?

Regards,
Jim

>From a0f35945f3127ab70d051101037e821b1759b4bb Mon Sep 17 00:00:00 2001
From: Jim Fehlig <jfehlig@suse.com>
Date: Mon, 21 Oct 2013 15:30:02 -0600
Subject: [PATCH] build: fix virt-login-shell build with apparmor

With libapparmor-devel installed, virt-login-shell fails to link

CCLD     virt-login-shell
../src/.libs/libvirt-setuid-rpc-client.a(libvirt_setuid_rpc_client_la-vircommand.o): In function `virExec':
/home/jfehlig/virt/upstream/libvirt/src/util/vircommand.c:653: undefined reference to `aa_change_profile'
collect2: error: ld returned 1 exit status

Fix by linking libvirt_setuid_rpc_client with previously determined
SECDRIVER_LIBS in src/Makefile.am.  While at it, introduce SECDRIVER_CFLAGS
and use both throughout src/Makefile.am where it makes sense.
Signed-off-by: NJim Fehlig <jfehlig@suse.com>

Conflicts:
	src/Makefile.am: Context

The libvirt.so library has far too many library deps to allow
linking against it from setuid programs. Those libraries can
do stuff in __attribute__((constructor) functions which is
not setuid safe.

The virt-login-shell needs to link directly against individual
files that it uses, with all library deps turned off except
for libxml2 and libselinux.

Create a libvirt-setuid-rpc-client.la library which is linked
to by virt-login-shell. A config-post.h file allows this library
to disable all external deps except libselinux and libxml2.
Signed-off-by: NDaniel P. Berrange <berrange@redhat.com>
(cherry picked from commit 3e2f27e1)

We don't want to inherit any FDs in the new namespace
except for the stdio FDs. Explicitly close them all,
just in case some do not have the close-on-exec flag
set.
Signed-off-by: NDaniel P. Berrange <berrange@redhat.com>
(cherry picked from commit b7fcc799)