Add virRWLock backed up by a POSIX rwlock primitive
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
(cherry picked from commit c065984b58000a44c90588198d222a314ac532fd)
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)
Conflicts:
src/nwfilter/nwfilter_driver.c
- *EnsureACL*() was added after this branch was created, and
caused two small conflicts in the context around a hunk.
- nwfilterDriverReload() was renamed to nwfilterStateReload()
upstream.
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)
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)
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)
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)
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)
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)
Every API that is going to begin a job should do that before fetching
data from vm->def.
(cherry picked from commit ff5f30b6bfa317f2a4c33f69289baf4e887eb048)
Conflicts:
src/qemu/qemu_driver.c - context
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)
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)
Conflicts:
src/qemu/qemu_driver.c - VIR_STRDUP not backported
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)
Conflicts:
src/qemu/qemu_driver.c - context: no ACLs
Xen 4.3 fixes a mistake in the libxl event handler signature where the
event owned by the application was defined as const. Detect this and
define the libvirt libxl event handler signature appropriately.
(cherry picked from commit 43b0ff5b1eb7fbcdc348b2a53088a7db939d5e48)
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)
Since PIDs can be reused, polkit prefers to be given
a (PID,start time) pair. If given a PID on its own,
it will attempt to lookup the start time in /proc/pid/stat,
though this is subject to races.
It is safer if the client app resolves the PID start
time itself, because as long as the app has the client
socket open, the client PID won't be reused.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
(cherry picked from commit 979e9c56a7aadf2dcfbddd1abfbad594b78b4468)
Conflicts:
src/util/virprocess.c
src/util/virstring.c
src/util/virstring.h
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)
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)
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)
When creating a virtual FC HBA with virsh/libvirt API, an error message
will be returned: "error: Node device not found",
also the 'nodedev-dumpxml' shows wrong information of wwpn & wwnn
for the new created device.
Signed-off-by: xschen@tnsoft.com.cn
This reverts f90af69 which switched wwpn & wwwn in the wrong place.
https://www.kernel.org/doc/Documentation/scsi/scsi_fc_transport.txt
(cherry picked from commit 3c0d5e224c71cd85b123cd938eeb360502503cb0)
Conflicts:
src/storage/storage_backend_scsi.c
If networkUnplugBandwidth is called on a network which has
no bandwidth defined, print a warning instead of crashing.
This can happen when destroying a domain with bandwidth if
bandwidth was removed from the network after the domain was
started.
https://bugzilla.redhat.com/show_bug.cgi?id=975359
(cherry picked from commit 658c932ab4aec2222b0ce3840a96748e73b39b3f)
Don't check for '\n' at the end of file if zero bytes were read.
Found by valgrind:
==404== Invalid read of size 1
==404== at 0x529B09F: virCgroupGetValueStr (vircgroup.c:540)
==404== by 0x529AF64: virCgroupMoveTask (vircgroup.c:1079)
==404== by 0x1EB475: qemuSetupCgroupForEmulator (qemu_cgroup.c:1061)
==404== by 0x1D9489: qemuProcessStart (qemu_process.c:3801)
==404== by 0x18557E: qemuDomainObjStart (qemu_driver.c:5787)
==404== by 0x190FA4: qemuDomainCreateWithFlags (qemu_driver.c:5839)
Introduced by 0d0b409.
https://bugzilla.redhat.com/show_bug.cgi?id=978356
(cherry picked from commit 306c49ffd56a1c72b1892d50f2a75531c62f4a1d)
Cherry-picked from 38ada092d1ad5f27a24e511173308d568b6b085f
As a consequence of the cgroup layout changes from commit 'cfed9ad4', the
lxcDomainGetSchedulerParameters[Flags]()' and lxcGetSchedulerType() APIs
failed to return data for a non running domain. This can be seen through
a 'virsh schedinfo <domain>' command which returns:
Scheduler : Unknown
error: Requested operation is not valid: cgroup CPU controller is not mounted
Prior to that change a non running domain would return:
Scheduler : posix
cpu_shares : 0
vcpu_period : 0
vcpu_quota : 0
emulator_period: 0
emulator_quota : 0
This patch will restore the capability to return configuration only data
for a non running domain regardless of whether cgroups are available.
Conflicts:
src/lxc/lxc_driver.c
* Resolved conflict by using former lxcCgroupHasController() rather than
virCgroupHasController()
* Needed to add the code to fetch the 'vm'
vm = virDomainObjListFindByUUID(driver->domains, domain->uuid);
if (vm == NULL) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("No such domain %s"), domain->uuid);
goto cleanup;
}
* Used 'ret = strdup("posix");' rather than VIR_STRDUP(ret, "posix");
and added the virReportOOMError(); on failure.
Cherry-picked from b23754534193fb7a1e31306d94ae5f09759a0aa4
As a consequence of the cgroup layout changes from commit '632f78ca', the
qemuDomainGetSchedulerParameters[Flags]()' and qemuGetSchedulerType() APIs
failed to return data for a non running domain. This can be seen through
a 'virsh schedinfo <domain>' command which returns:
Scheduler : Unknown
error: Requested operation is not valid: cgroup CPU controller is not mounted
Prior to that change a non running domain would return:
Scheduler : posix
cpu_shares : 0
vcpu_period : 0
vcpu_quota : 0
emulator_period: 0
emulator_quota : 0
This patch will restore the capability to return configuration only data
for a non running domain regardless of whether cgroups are available.
Conflicts:
src/qemu/qemu_driver.c
* Resolved conflict by using former qemuCgroupHasController() rather than
virCgroupHasController()
* Needed to add the code to fetch the 'vm'
vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
if (vm == NULL) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("No such domain %s"), dom->uuid);
goto cleanup;
}
* Used 'ret = strdup("posix");' rather than VIR_STRDUP(ret, "posix");
and added the virReportOOMError(); on failure.
Don't free the stream on error if we've successfully added it
to the hash table, since it will be freed by virChrdevHashEntryFree
callback.
Preserve the error message before calling virStreamFree, since it
resets the error.
Introduced by 4716138, crashing since 6921892.
Reported by Sergey Fionov on libvir-list.
(cherry picked from commit a32b41746c4e1a44fb998a93da99c72f6586b359)
Use the host number as the host number when constructing the sysfs path
instead of the variable we are trying to fill.
https://bugzilla.redhat.com/show_bug.cgi?id=973543
(cherry picked from commit 371c15517c919e9d7d2ea3f66e490bd49b6db384)
This should resolve:
https://bugzilla.redhat.com/show_bug.cgi?id=959191
The problem was that qemuUpdateActivePciHostdevs was returning 0
(success) when no hostdevs were present, but would otherwise return -1
(failure) even when it completed successfully. It is only called from
qemuProcessReconnect(), and when qemuProcessReconnect got back an
error, it would not only stop reconnecting, but would terminate the
guest qemu process "to remove danger of it ending up running twice if
user tries to start it again later".
(This bug was introduced in commit 011cf7ad, which was pushed between
v1.0.2 and v1.0.3, so all maintenance branches from v1.0.3 up to 1.0.5
will need this one line patch applied.)
(cherry picked from commit 2ea45647bcde23cff5da48f725561ff5ba3fba39)
https://bugzilla.redhat.com/show_bug.cgi?id=922186
Commit d04916fa introduced a regression in audit quality - even
though the code was computing the proper escaped name for a
path, it wasn't feeding that escaped name on to the audit message.
As a result, /var/log/audit/audit.log would mention a pair of
fields class=path path=/dev/hpet instead of the intended
class=path path="/dev/hpet", which in turn caused ausearch to
format the audit log with path=(null).
* src/conf/domain_audit.c (virDomainAuditCgroupPath): Use
constructed encoding.
Signed-off-by: Eric Blake <eblake@redhat.com>
(cherry picked from commit 31c6bf35b9d9de04158318658f4fbf6a9e54ff28)
By default, libtool builds two .o files for every .lo rule:
src/foo.o - static builds
src/.libs/foo.o - shared library builds
But since commit ad42b34b disabled static builds, src/foo.o is
no longer built by default. On a fresh checkout, this means our
protocol check rules using pdwtags were testing a missing file,
and thanks to a lousy behavior of pdwtags happily giving no output
and 0 exit status (http://bugzilla.redhat.com/949034), we were
merely claiming that "dwarves is too old" and skipping the test.
However, if you swap between branches and do incremental builds,
such as building v0.10.2-maint and then switching back to master,
you end up with src/foo.o being leftover from its 0.10.2 state,
and then 'make check' fails because the .o file does not match
the protocol-structs file due to API additions in the meantime.
A simpler fix would be to always look in .libs for the .o to
be parsed; but since it is possible to pass ./configure options
to tell libtool to do a static-only build with no shared .o,
I went with the approach of finding the newest of the two files,
whenever both exist.
* src/Makefile.am (PDWTAGS): Ensure we test just-built file.
(cherry picked from commit 5899e09e6112ff739cc4b9b9198d904fc88b698c)
The last Viktor's effort to fix the race and memory corruption unfortunately
wasn't complete in the case the close callback was not registered in an
connection. At that time, the trail of event's that I'll describe later could
still happen and corrupt the memory or cause a crash of the client (including
the daemon in case of a p2p migration).
Consider the following prerequisities and trail of events:
Let's have a remote connection to a hypervisor that doesn't have a close
callback registered and the client is using the event loop. The crash happens in
cooperation of 2 threads. Thread E is the event loop and thread W is the worker
that does some stuff. R denotes the remote client.
1.) W - The client finishes everything and sheds the last reference on the client
2.) W - The virObject stuff invokes virConnectDispose that invokes doRemoteClose
3.) W - the remote close method invokes the REMOTE_PROC_CLOSE RPC method.
4.) W - The thread is preempted at this point.
5.) R - The remote side receives the close and closes the socket.
6.) E - poll() wakes up due to the closed socket and invokes the close callback
7.) E - The event loop is preempted right before remoteClientCloseFunc is called
8.) W - The worker now finishes, and frees the conn object.
9.) E - The remoteClientCloseFunc accesses the now-freed conn object in the
attempt to retrieve pointer for the real close callback.
10.) Kaboom, corrupted memory/segfault.
This patch tries to fix this by introducing a new object that survives the
freeing of the connection object. We can't increase the reference count on the
connection object itself or the connection would never be closed, as the
connection is closed only when the reference count reaches zero.
The new object - virConnectCloseCallbackData - is a lockable object that keeps
the pointers to the real user registered callback and ensures that the
connection callback is either not called if the connection was already freed or
that the connection isn't freed while this is being called.
(cherry picked from commit 8ad126e695e5cef5da9d62ccfde7338317041e84)
By adjusting the reference count of the connection object we
prevent races between callback function and virConnectClose.
Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com>
(cherry picked from commit 03a43efa86f5099d3f6df334f73961a535e488b5)
When creating a logical volume with virStorageVolCreateXMLFrom,
"qemu-img convert" is called internally if clonevol is a file volume.
Then, vol->target.format is used as output_fmt parameter but the
target.format of logical volumes is always 0 because logical volumes
haven't the volume format type element.
Fortunately, 0 was treated as RAW file format before commit f772b3d9,
so there was no problem. But now, 0 is treated as the type of none,
qemu-img fails with "Unknown file format 'none'".
This patch fixes this issue by treating output block devices as RAW
file format like for input block devices.
Signed-off-by: Atsushi Kumagai <kumagai-atsushi@mxc.nes.nec.co.jp>
(cherry picked from commit d369e50825b88251d13a489efa2a58919b18a2c2)
By passing the flags -z relro -z now to the linker, we can force
it to resolve all library symbols at startup, instead of on-demand.
This allows it to then make the global offset table (GOT) read-only,
which makes some security attacks harder.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
(cherry picked from commit fc8c1787d8b8eb6c462b8e5b49c2b3ccf3669bc0)
PIE (position independent executable) adds security to executables
by composing them entirely of position-independent code (PIC. The
.so libraries already build with -fPIC. This adds -fPIE which is
the equivalent to -fPIC, but for executables. This for allows Exec
Shield to use address space layout randomization to prevent attackers
from knowing where existing executable code is during a security
attack using exploits that rely on knowing the offset of the
executable code in the binary, such as return-to-libc attacks.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
(cherry picked from commit 1150999ca444d8cb1d906a4948b808125fa209b7)
Mimic the fix done in 02b9097274d1330c2e1dca7f598880e09b5c2aa0 to fix crash by
accessing an already freed structure. Also copy the explaining comment why the
pointer can't be accessed any more.
(cherry picked from commit 43b6f304bce5937f1e3a852b20b52a772b39b95f)
Reported by Anthony Messina in
https://bugzilla.redhat.com/show_bug.cgi?id=904692
Present since introduction of smartcard support in commit f5fd9baa
* src/qemu/qemu_command.c (qemuBuildCommandLine): Match qemu spelling.
* tests/qemuxml2argvdata/qemuxml2argv-smartcard-host-certificates.args:
Fix broken test.
(cherry picked from commit 6f7e4ea359323f9bc413dfb738a5c544d4f9c4f8)
Code added by commit id '523207fe8'
TEST: qemuxml2argvtest
........................................ 40
........................................ 80
........................................ 120
........................................ 160
........................................ 200
........................................ 240
................................. 273 OK
==30993== 39 bytes in 1 blocks are definitely lost in loss record 33 of 87
==30993== at 0x4A0887C: malloc (vg_replace_malloc.c:270)
==30993== by 0x41E501: fakeSecretGetValue (qemuxml2argvtest.c:33)
==30993== by 0x427591: qemuBuildDriveURIString (qemu_command.c:2571)
==30993== by 0x42C502: qemuBuildDriveStr (qemu_command.c:2627)
==30993== by 0x4335FC: qemuBuildCommandLine (qemu_command.c:6443)
==30993== by 0x41E8A0: testCompareXMLToArgvHelper (qemuxml2argvtest.c:154
==30993== by 0x41FE8F: virtTestRun (testutils.c:157)
==30993== by 0x418BE3: mymain (qemuxml2argvtest.c:506)
==30993== by 0x4204CA: virtTestMain (testutils.c:719)
==30993== by 0x38D6821A04: (below main) (in /usr/lib64/libc-2.16.so)
==30993==
==30993== 46 bytes in 1 blocks are definitely lost in loss record 64 of 87
==30993== at 0x4A0887C: malloc (vg_replace_malloc.c:270)
==30993== by 0x38D690A167: __vasprintf_chk (in /usr/lib64/libc-2.16.so)
==30993== by 0x4CB28E7: virVasprintf (stdio2.h:210)
==30993== by 0x4CB29A3: virAsprintf (virutil.c:2017)
==30993== by 0x4275B4: qemuBuildDriveURIString (qemu_command.c:2580)
==30993== by 0x42C502: qemuBuildDriveStr (qemu_command.c:2627)
==30993== by 0x4335FC: qemuBuildCommandLine (qemu_command.c:6443)
==30993== by 0x41E8A0: testCompareXMLToArgvHelper (qemuxml2argvtest.c:154
==30993== by 0x41FE8F: virtTestRun (testutils.c:157)
==30993== by 0x418BE3: mymain (qemuxml2argvtest.c:506)
==30993== by 0x4204CA: virtTestMain (testutils.c:719)
==30993== by 0x38D6821A04: (below main) (in /usr/lib64/libc-2.16.so)
==30993==
==30993== 385 (56 direct, 329 indirect) bytes in 1 blocks are definitely los
==30993== at 0x4A06B6F: calloc (vg_replace_malloc.c:593)
==30993== by 0x4C6B2CF: virAllocN (viralloc.c:152)
==30993== by 0x4C9C7EB: virObjectNew (virobject.c:191)
==30993== by 0x4D21810: virGetSecret (datatypes.c:642)
==30993== by 0x41E5D5: fakeSecretLookupByUsage (qemuxml2argvtest.c:51)
==30993== by 0x4D4BEC5: virSecretLookupByUsage (libvirt.c:15295)
==30993== by 0x4276A9: qemuBuildDriveURIString (qemu_command.c:2565)
==30993== by 0x42C502: qemuBuildDriveStr (qemu_command.c:2627)
==30993== by 0x4335FC: qemuBuildCommandLine (qemu_command.c:6443)
==30993== by 0x41E8A0: testCompareXMLToArgvHelper (qemuxml2argvtest.c:154
==30993== by 0x41FE8F: virtTestRun (testutils.c:157)
==30993== by 0x418BE3: mymain (qemuxml2argvtest.c:506)
==30993==
PASS: qemuxml2argvtest
Interesting side note is that running the test singularly via 'make -C tests
check TESTS=qemuxml2argvtest' didn't trip the valgrind error; however,
running during 'make -C tests valgrind' did cause the error to be seen.
(cherry picked from commit 9a80050e523da636a11d32d507ede11af764b9e0)
Commit 9a3ff01d7f16cc280ce3176620c0714f55511a65 (which was ACKed at
the end of January, but for some reason didn't get pushed until during
the 1.0.4 freeze) fixed the logic in virPCIGetVirtualFunctions().
Unfortunately, a typo in the fix (replacing VIR_REALLOC_N with
VIR_ALLOC_N during code movement) caused not only a memory leak, but
also resulted in most of the elements of the result array being
replaced with NULL. virNetDevGetVirtualFunctions() assumed (and I think
rightly so) that virPCIGetVirtualFunctions() wouldn't return any NULL
elements in the array, so it ended up segfaulting.
This was found when attempting to use a virtual network with an
auto-created pool of SRIOV VFs, e.g.:
<forward mode='hostdev' managed='yes'>
<pf dev='eth4'/>
</forward>
(the pool of PCI addresses is discovered by calling
virNetDevGetVirtualFunctions() on the PF dev).
When logical pool has no PVs associated with itself (user-created),
virCommandFree(cmd) is called twice with the same pointer and that
causes a segfault in daemon.
With my previous patches, we unconditionally appended a seclabel,
even if it wasn't generated but found in array of defined seclabels.
This resulted in double free later when doing virDomainDefFree
and iterating over the array of defined seclabels.
Moreover, there was another possibility of double free, if the
seclabel was generated in the last iteration of the process of
walking trough security managers array.
One of my previous patches manipulated virSecurityLabel* APIs,
some were added to header files, and some were renamed. However,
these changes were not reflected in libvirt_private.syms.
https://bugzilla.redhat.com/show_bug.cgi?id=923946
The <seclabel type='none'/> should be added iff there is no other
seclabel defined within a domain. This bug can be easily reproduced:
1) configure selinux seclabel for a domain
2) disable system's selinux and restart libvirtd
3) observe <seclabel type='none'/> being appended to a domain on its
startup
The virDomainDefGetSecurityLabelDef was modifying the domain XML.
It tried to find a seclabel corresponding to given sec driver. If the
label wasn't found, the function created one which is wrong. In fact
it's security manager which should modify this part of domain XML.