Commit Graph

8709 Commits

Author SHA1 Message Date
Laine Stump
0345c7281b qemu: let virCommand set child process security labels/uid/gid
The qemu driver had been calling virSecurityManagerSetProcessLabel()
from a "pre-exec hook" function that is run after the child is forked,
but before exec'ing qemu. This is problematic because the uid and gid
of the child are set by the security driver, but capabilities are
dropped by virCommand - such separation doesn't work; the two
operations must be done together or the capabilities do not transfer
properly to the child process.

This patch switches to using virSecurityManagerSetChildProcessLabel(),
which is called prior to virCommandRun() (rather than being called
*during* virCommandrun() by the hook function), and doesn't set the
UID/GID/security label directly, but instead merely informs virCommand
what it should set them all to when the time is appropriate.

This lets virCommand choose to do the uid/gid and caps dropping all at
the same time if it wants (it does *want* to, but isn't doing so yet;
that's for an upcoming patch).
2013-02-13 16:11:16 -05:00
Laine Stump
7bf1aa0b9b security: add new virSecurityManagerSetChildProcessLabel API
The existing virSecurityManagerSetProcessLabel() API is designed so
that it must be called after forking the child process, but before
exec'ing the child. Due to the way the virCommand API works, that
means it needs to be put in a "hook" function that virCommand is told
to call out to at that time.

Setting the child process label is a basic enough need when executing
any process that virCommand should have a method of doing that. But
virCommand must be told what label to set, and only the security
driver knows the answer to that question.

The new virSecurityManagerSet*Child*ProcessLabel() API is the way to
transfer the knowledge about what label to set from the security
driver to the virCommand object. It is given a virCommandPtr, and each
security driver calls the appropriate virCommand* API to tell
virCommand what to do between fork and exec.

1) in the case of the DAC security driver, it calls
virCommandSetUID/GID() to set a uid and gid that must be set for the
child process.

2) for the SELinux security driver, it calls
virCommandSetSELinuxLabel() to save a copy of the char* that will be
sent to setexeccon_raw() *after forking the child process*.

3) for the AppArmor security drivers, it calls
virCommandSetAppArmorProfile() to save a copy of the char* that will
be sent to aa_change_profile() *after forking the child process*.

With this new API in place, we will be able to remove
virSecurityManagerSetProcessLabel() from any virCommand pre-exec
hooks.

(Unfortunately, the LXC driver uses clone() rather than virCommand, so
it can't take advantage of this new security driver API, meaning that
we need to keep around the older virSecurityManagerSetProcessLabel(),
at least for now.)
2013-02-13 16:11:16 -05:00
Laine Stump
6c3f3d0d89 util: add security label setting to virCommand
virCommand gets two new APIs: virCommandSetSELinuxLabel() and
virCommandSetAppArmorProfile(), which both save a copy of a
null-terminated string in the virCommand. During virCommandRun, if the
string is non-NULL and we've been compiled with AppArmor and/or
SELinux security driver support, the appropriate security library
function is called for the child process, using the string that was
previously set. In the case of SELinux, setexeccon_raw() is called,
and for AppArmor, aa_change_profile() is called.

This functionality has been added so that users of virCommand can use
the upcoming virSecurityManagerSetChildProcessLabel() prior to running
a child process, rather than needing to setup a hook function to be
called (and in turn call virSecurityManagerSetProcessLabel()) *during*
the setup of the child process.
2013-02-13 16:11:15 -05:00
Laine Stump
4a56e80fa5 build: define SECDRIVER_LIBS in Makefile.am
This makes it simpler to include the necessary system security driver
libraries for a particular system. For this patch, several existing
conditional sections from the Makfile were replaced; I'll later be
adding SECDRIVER_LIBS to libvirt_util_la_LIBADD, because vircommand.c
will be calling a function from $securitylib.
2013-02-13 16:11:15 -05:00
Laine Stump
dd5bb43962 qemu: replace exec hook with virCommandSetUID/GID in storage_backend 2013-02-13 16:11:15 -05:00
Laine Stump
6a8ecc373e qemu: replace exec hook with virCommandSetUID/GID in qemuCaps*
Setting the uid/gid of the child process was the only thing done by
the hook function in this case, and that can now be done more simply
with virCommandSetUID/GID.
2013-02-13 16:11:15 -05:00
Laine Stump
f506a4c115 util: make virSetUIDGID a NOP only when uid or gid is -1
Rather than treating uid:gid of 0:0 as a NOP, we blindly pass that
through to the lower layers. However, we *do* check for a requested
value of "-1" to mean "don't change this setting". setregid() and
setreuid() already interpret -1 as a NOP, so this is just an
optimization, but we are also calling getpwuid_r and initgroups, and
it's unclear what the former would do with a uid of -1.
2013-02-13 16:11:15 -05:00
Laine Stump
417182b072 util: add virCommandSetUID and virCommandSetGID
If a uid and/or gid is specified for a command, it will be set just
after the user-supplied post-fork "hook" function is called.

The intent is that this can replace user hook functions that set
uid/gid. This moves the setting of uid/gid and dropping of
capabilities closer to each other, which is important since the two
should really be done at the same time (libcapng provides a single
function that does both, which we will be unable to use, but want to
mimic as closely as possible).
2013-02-13 16:11:15 -05:00
Laine Stump
ad5cb11be6 util: refactor virCommandHook into virExec and virCommandHandshakeChild 2013-02-13 16:11:15 -05:00
Laine Stump
5f2ce53984 util: eliminate extra args from virExec
All args except "cmd" in the call to virExec are now redundant, since
they can all be found in cmd, so remove the args and reference the
data directly in cmd. One exception to this is that "infd" was being
modified within virExec, and modifying the original in cmd caused make
check failures, so cmd->infd is copied to a local, and the local is
used during virExec().
2013-02-13 16:11:15 -05:00
Laine Stump
b6decc57b1 util: eliminate generic hook from virExecWithHook
virExecWithHook is only called from one place, so it always has the
same "hook" function (virHookCommand), and the data sent to that
function is always a virCommandPtr, so eliminate the function and
generic data from the arglist, and replace it with "virCommandPtr
cmd". The call to (hook)(data) is replaced with
"virHookCommand(cmd)". Finally, virExecWithHook is renamed to virExec.

Indentation has been updated only for code that will remain after the
next patch, which will remove all other args to virExec (since they
are now redundant, as they're all members of virCommandPtr).
2013-02-13 16:11:15 -05:00
Daniel P. Berrange
a9e97e0c30 Remove qemuDriverLock from almost everywhere
With the majority of fields in the virQEMUDriverPtr struct
now immutable or self-locking, there is no need for practically
any methods to be using the QEMU driver lock. Only a handful
of helper APIs in qemu_conf.c now need it
2013-02-13 11:10:30 +00:00
Michal Privoznik
3178df9afa virCommand: Don't misuse the eventloop for async IO
Currently, if a command wants to do asynchronous IO, a callback
is registered in the libvirtd eventloop to handle writes and
reads. However, there's a race in virCommandWait. The eventloop
may already be executing the callback, while virCommandWait is
mangling internal state of virCommand. To deal with it, we need
to either introduce locking or spawn a separate thread where we
poll() on stdio from child. The former, however, requires to
unlock all mutexes held, as the event loop may execute other
callbacks which tries to lock one of the mutexes, deadlock and
thus never wake us up. So it's safer to spawn a separate thread.
2013-02-13 09:54:19 +01:00
Eric Blake
069b5c5a58 xen: clean up the mess with cpumap
Commit 8b55992f added some Coverity comments to silence what was
a real bug in the code.  Since then, we've had a miserable run
of trying to fix the underlying problem (commits c059cde and
ba5193c), and still have a problem on 32-bit machines.

This fixes the problem for once and for all, by realizing that
on older xen, cpumap_t is identical to uint64_t, and using the
new virendian.h to do the transformation from the API (documented
to be little-endian) to the host structure.

* src/xen/xen_hypervisor.c (virXen_setvcpumap): Do the conversion
correctly.  Finally.
2013-02-12 09:00:17 -07:00
Eric Blake
731ad69240 util: use new virendian.h macros
This makes code easier to read, by avoiding lines longer than
80 columns and removing the repetition from the callers.

* src/util/virstoragefile.c (qedGetHeaderUL, qedGetHeaderULL):
Delete in favor of more generic macros.
(qcow2GetBackingStoreFormat, qcowXGetBackingStore)
(qedGetBackingStore, virStorageFileMatchesVersion)
(virStorageFileGetMetadataInternal): Use new macros.
* src/cpu/cpu_x86.c (x86VendorLoad): Likewise.
2013-02-12 09:00:17 -07:00
Eric Blake
c6f1060ca7 util: add virendian.h macros
We have several cases where we need to read endian-dependent
data regardless of host endianness; rather than open-coding
these call sites, it will be nicer to funnel things through
a macro.

The virendian.h file can be expanded to add writer functions,
and/or 16-bit access patterns, if needed.  Also, if we need
to turn things into a function to avoid multiple evaluations
of buf, that can be done later.  But for now, a macro worked.

* src/util/virendian.h: New file.
* src/Makefile.am (UTIL_SOURCES): Ship it.
* tests/virendiantest.c: New test.
* tests/Makefile.am (test_programs, virendiantest_SOURCES): Run
the test.
* .gitignore: Ignore built file.
2013-02-12 09:00:15 -07:00
Daniel P. Berrange
d1c7b00b60 Release VM lock before acquiring virDomainObjListPtr lock
When removing a VM from the virDomainObjListPtr, we must not
be holding the VM lock while acquiring the list lock. Re-order
code to ensure that we can release the VM lock early.
2013-02-12 11:06:03 +00:00
Daniel P. Berrange
61b52d2e38 Fix potential deadlock across fork() in QEMU driver
The hook scripts used by virCommand must be careful wrt
accessing any mutexes that may have been held by other
threads in the parent process. With the recent refactoring
there are 2 potential flaws lurking, which will become real
deadlock bugs once the global QEMU driver lock is removed.

Remove use of the QEMU driver lock from the hook function
by passing in the 'virQEMUDriverConfigPtr' instance directly.

Add functions to the virSecurityManager to be invoked before
and after fork, to ensure the mutex is held by the current
thread. This allows it to be safely used in the hook script
in the child process.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
2013-02-12 11:05:31 +00:00
Eric Blake
736a87b916 build: fix compilation of selinux on RHEL 5
On RHEL 5, I got:

security/security_selinux.c: In function 'getContext':
security/security_selinux.c:971: warning: unused parameter 'mgr' [-Wunused-parameter]

* src/security/security_selinux.c (getContext): Mark potentially
unused parameter.
2013-02-11 17:12:25 -07:00
Viktor Mihajlovski
0bbbd42c30 S390: domain_conf support for CCW
Add necessary handling code for the new s390 CCW address type to
virDomainDeviceInfo. Further, introduce  memory management, XML
parsing, output formatting and range validation for the new
virDomainDeviceCCWAddress type.

Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com>
2013-02-11 16:38:21 -07:00
Laine Stump
6627ea3cbf build: fix make check of remote_protocol-structs
Broken by incorrect formatting / spelling of remote_nonnull in commit
39758e7567
2013-02-11 15:34:49 -05:00
John Ferlan
ba5193c8d8 hypervisor: Restore pm initialization
Adjustment for 'c059cdeaf' due to older compiler complaint about pm
not being initialized even though the j&7 == 0 does the trick.
2013-02-12 00:32:57 +08:00
Osier Yang
fb2e465362 nodedev: Implement virNodeDeviceLookupSCSIHostByWWN
This just simply changes nodeDeviceLookupByWWN to be not static,
and its name into nodeDeviceLookupSCSIHostByWWN. And use that for
udev and HAL backends.
2013-02-12 00:23:57 +08:00
Osier Yang
39758e7567 remote: Wire up the remote protocol
Like virNodeDeviceCreateXML, virNodeDeviceLookupSCSIHostByWWN
has to be treated specially when generating the RPC codes. Also
new rules are added in fixup_name to keep the name SCSIHostByWWN.
2013-02-12 00:23:57 +08:00
Osier Yang
efed366eb7 Introduce API virNodeDeviceLookupSCSIHostByWWN
Since the name (like scsi_host10) is not stable for vHBA, (it can
be changed either after recreating or system rebooting), current
API virNodeDeviceLookupByName is not nice to use for management app
in this case. (E.g. one wants to destroy the vHBA whose name has
been changed after system rebooting, he has to find out current
name first).

Later patches will support the persistent vHBA via storage pool,
with which one can identify the vHBA stably by the wwnn && wwpn
pair.

So this new API comes.
2013-02-12 00:23:57 +08:00
Daniel P. Berrange
0ab49601a8 Remove re-entrant API call in SELinux/AppArmor security managers
The security manager drivers are not allowed to call back
out to top level security manager APIs, since that results
in recursive mutex acquisition and thus deadlock. Remove
calls to virSecurityManagerGetModel from SELinux / AppArmor
drivers

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
2013-02-11 16:05:05 +00:00
John Ferlan
c059cdeaf3 hypervisor: Revisit Coverity issues regarding cpumap
Turns out the issue regarding ptr_arith and sign_exension weren't false
positives. When shifting an 'unsigned char' as a target, it gets promoted
to an 'int'; however, that 'int' cannot be shifted 32 bits which was how
the algorithm was written. For the ptr_arith rather than index into the
cpumap, change the to address as necessary and assign directly.
2013-02-11 09:50:11 -05:00
John Ferlan
cbdf3b7c97 hypervisor: Remove redundant validity checks, clean up function headers
Arguments for driver entry points are checked in libvirt.c, so no need to
check again. Make function entry points consistent. Don't type caste the
privateData.
2013-02-11 09:50:01 -05:00
John Ferlan
f9799f1abf xend: Fix a memory leak found by Coverity
Commit id '87b4c10c' moved the VIR_ALLOC_N, but didn't check if 'cpuset'
had been allocated on failure.
2013-02-11 09:50:01 -05:00
John Ferlan
d951c1a0b9 xend: Remove redundant validity checks, clean up function headers
Arguments for driver entry points are checked in libvirt.c, so no need to
check again. Make function entry points consistent. Don't type caste the
privateData.
2013-02-11 09:49:48 -05:00
John Ferlan
273f34ba36 xm: Remove redundant validity checks, clean up function headers
Arguments for driver entry points are checked in libvirt.c, so no need to
check again. Make function entry points consistent. Don't type caste the
privateData.
2013-02-11 09:49:34 -05:00
John Ferlan
d67c2b76c7 inotify: Clean up some function headers 2013-02-11 09:49:17 -05:00
John Ferlan
2ca9c3cefe xs: Remove redundant validity checks, clean up function headers
Arguments for driver entry points are checked in libvirt.c, so no need to
check again. Make function entry points consistent.
2013-02-11 09:48:51 -05:00
Daniel P. Berrange
8cdd5faf46 Pass virQEMUDriverPtr into APIs managed shared disk list
Currently the APIs for managing the shared disk list take
a virHashTablePtr as the primary argument. This is bad
because it requires the caller to deal with locking of
the QEMU driver. Switch the APIs to take the full
virQEMUDriverPtr instance

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
2013-02-11 12:48:22 +00:00
Daniel P. Berrange
48b49a631a Serialize execution of security manager APIs
Add locking to virSecurityManagerXXX APIs, so that use of the
security drivers is internally serialized. This avoids the need
to rely on the global driver locks to achieve serialization

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
2013-02-11 12:33:44 +00:00
Daniel P. Berrange
11d926659b Turn virSecurityManager into a virObjectLockable
To enable locking to be introduced to the security manager
objects later, turn virSecurityManager into a virObjectLockable
class

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
2013-02-11 12:33:41 +00:00
Harry Wei
ad11ecd965 sheepdog: skip refresh on creation failure
Don't try to refresh Sheepdog volume if creating volume fails.

Signed-off-by: Harry Wei <harryxiyou@gmail.com>
2013-02-08 14:38:52 -07:00
Natanael Copa
f3531a040c util: refactor iptables command construction into multiple steps
Instead of creating an iptables command in one shot, do it in steps
so we can add conditional options like physdev and protocol.

This removes code duplication while keeping existing behaviour.

Signed-off-by: Natanael Copa <ncopa@alpinelinux.org>
Signed-off-by: Eric Blake <eblake@redhat.com>
2013-02-08 14:19:30 -07:00
Laine Stump
66d9bc00ab qemu: support vhost-net for generic ethernet devices
From qemu's point of view these are still just tap devices, so there's
no reason they shouldn't work with vhost-net; as a matter of fact,
Raja Sivaramakrishnan <srajag00@yahoo.com> verified on libvir-list
that at least the qemu_command.c part of this patch works:

  https://www.redhat.com/archives/libvir-list/2012-December/msg01314.html

(the hotplug case is extrapolation on my part).
2013-02-08 13:13:55 -05:00
Michal Privoznik
a6cfed260b network_conf.c: Free xmlDoc after use
The virNetworkObjUpdateParseFile() function was not freeing the xml
variable, leaving us with a memory leak.
2013-02-08 16:01:58 +01:00
Daniel P. Berrange
020a030786 Stop accessing driver->caps directly in QEMU driver
The 'driver->caps' pointer can be changed on the fly. Accessing
it currently requires the global driver lock. Isolate this
access in a single helper, so a future patch can relax the
locking constraints.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
2013-02-08 11:49:16 +00:00
Daniel P. Berrange
32803ba409 Rename 'qemuCapsXXX' to 'virQEMUCapsXXX'
To avoid confusion between 'virCapsPtr' and 'qemuCapsPtr'
do some renaming of various fucntions/variables. All
instances of 'qemuCapsPtr' are renamed to 'qemuCaps'. To
avoid that clashing with the 'qemuCaps' typedef though,
rename the latter to virQEMUCaps.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
2013-02-08 11:49:14 +00:00
Daniel P. Berrange
fed92f08db Turn virCapabilities into a virObject
To enable virCapabilities instances to be reference counted,
turn it into a virObject. All cases of virCapabilitiesFree
turn into virObjectUnref

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
2013-02-08 11:34:26 +00:00
Daniel P. Berrange
5b984370f6 Fix comment about virCgroupPtr locking rules in QEMU driver
The virCgroupPtr instance APIs are safe to use without locking
in the QEMU driver, since all internal state they rely on is
immutable. Update the comment to reflect this.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
2013-02-08 11:34:25 +00:00
Michal Privoznik
0d36f228a4 virCondDestroy: Lose attribute RETURN_CHECK
We are wrapping it in ignore_value() anyway.
2013-02-08 09:12:11 +01:00
Michal Privoznik
4ca6f5089f Drop useless virFileWrapperFdCatchError
We are requesting for stderr catching for all cases in
virFileWrapperFdNew(). There is no need to have a separate
function just to report an error, esp. when we can do it in
virFileWrapperFdClose().
2013-02-08 09:11:51 +01:00
John Ferlan
2792ad143e virnetttlcontext: Resolve issues found by Valgrind
Need to initialize 'usage' and 'critical' since the VIR_DEBUG will
attempt to use them.
2013-02-07 14:08:14 -05:00
John Ferlan
890b6b351f qemu_command: Resolve resource leaks found by Valgrind
The qemuParseGlusterString() replaced dst->src without a VIR_FREE() of
what was in there before.

The qemuBuildCommandLine() did not properly free the boot_buf depending
on various usages.

The qemuParseCommandLineDisk() had numerous paths that didn't clean up
the virDomainDiskDefPtr def properly. Adjust the logic to go through an
error: label before cleanup in order to free the resource.
2013-02-07 14:08:14 -05:00
John Ferlan
7af7c42d05 domain_conf: Resolve resource leaks found by Valgrind
Fix various resource leaks discovered while parsing through Valgrind output
2013-02-07 14:08:14 -05:00
John Ferlan
cbd5dc09e8 vport_profile_conf: Resolve memory leak found by Valgrind
The 'virtPortInterfaceID' was not VIR_FREE()'d
2013-02-07 14:08:14 -05:00