If OOM occurs in qemuParseCommandLineDisk some intermediate
variables will be leaked when parsing Sheepdog or RBD disks.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The qemuBuildCommandLine code for parsing sound cards will leak
an intermediate variable if an OOM occurs. Move the free'ing of
the variable earlier to avoid the leak.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
In qemuParseNBDString, if the virURIParse fails, the
error is not reported to the caller. Instead execution
falls through to the non-URI codepath causing memory
leaks later on.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
If qemuAddRBDHost fails due to parsing problems or OOM, then
qemuParseRBDString cleanup is skipped causing a memory leak.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
qemuDomainPCIAddressGetNextSlot has a loop for finding
compatible PCI buses. In the loop body it creates a
PCI address string, but never frees this. This causes
a leak if the loop executes more than one iteration,
or if a call in the loop body fails.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
If virDomainSoundCodecDefParseXML returns an error (eg due
to OOM), then the xml nodeset codecNodes is leaked.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
If virDomainVcpuPinDefArrayFree is called with def != NULL,
but nvcpupin == 0, then it leaks memory for 'def'. This is
an unusual scenario, but it hits when cleaning up after an
OOM during parsing of XML.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
This resolves one of the issues listed in:
https://bugzilla.redhat.com/show_bug.cgi?id=1003983
00:1E.0 is the location of this controller on at least some actual Q35
hardware, so we try to replicate the placement. The bridge should work
just as well in any other location though, so if 00:1E.0 isn't
available, just allow it to be auto-assigned anywhere appropriate.
This resolves one of the issues in:
https://bugzilla.redhat.com/show_bug.cgi?id=1003983
This device is identical to qemu's "intel-hda" device (known as "ich6"
in libvirt), but has a different PCI device ID (which matches the ID
of the hda audio built into the ich9 chipset, of course). It's not
supported in earlier versions of qemu, so it requires a capability
bit.
I'm not sure why this code was written to compare the strings that it
had just retrieved from an enum->string conversion, rather than just
look at the original enum values, but this yields the same results,
and is much more efficient (especially as you add more devices).
This is a prerequisite for patches to resolve:
https://bugzilla.redhat.com/show_bug.cgi?id=1003983
Part of the resolution to:
https://bugzilla.redhat.com/show_bug.cgi?id=1003983
Although most devices available in qemu area defined as PCI devices,
and strictly speaking should only be attached via a PCI slot, in
practice qemu allows them to be attached to a PCIe slot and sometimes
this makes sense.
For example, The UHCI and EHCI USB controllers are usually attached
directly to the PCIe "root complex" (i.e. PCIe slots) on real
hardware, so that should be possible for a Q35-based qemu virtual
machine as well.
We still want to prefer a standard PCI slot when auto-assigning
addresses, though, and in general to disallow attaching PCI devices
via PCIe slots.
This patch makes that possible by adding a new
QEMU_PCI_CONNECT_TYPE_EITHER_IF_CONFIG flag. Three things are done
with this flag:
1) It is set for the "pcie-root" controller
2) qemuCollectPCIAddress() now has a set of nested switches that set
this "EITHER" flag for devices that we want to allow connecting to
pcie-root when specifically requested in the config.
3) qemuDomainPCIAddressFlagsCompatible() adds this new flag to the
"flagsMatchMask" if the address being checked came from config rather
than being newly auto-allocated by libvirt (this knowledge is
conveniently already available in the "fromConfig" arg).
Now any device having the EITHER flag set can be connected to
pcie-root if explicitly requested, but auto-allocated addresses for
those devices will still be standard PCI slots instead.
This patch only loosens the restrictions on devices that have been
specifically requested, but the setup is such that it should be fairly
easy to add new devices.
Replace them with switch cases. This will make it more efficient when
we add exceptions for more controller types, and other device types.
This is a prerequisite for patches to resolve:
https://bugzilla.redhat.com/show_bug.cgi?id=1003983
Packets sent by guests on virbrN, *or* by dnsmasq on the same, to
- 255.255.255.255/32 (netmask-independent local network broadcast
address), or to
- 224.0.0.0/24 (local subnetwork multicast range)
are never forwarded, hence it is not necessary to masquerade them.
In fact we must not masquerade them: translating their source addresses or
source ports (where applicable) may confuse receivers on virbrN.
One example is the DHCP client in OVMF (= UEFI firmware for virtual
machines):
http://thread.gmane.org/gmane.comp.bios.tianocore.devel/1506/focus=2640
It expects DHCP replies to arrive from remote source port 67. Even though
dnsmasq conforms to that, the destination address (255.255.255.255) and
the source address (eg. 192.168.122.1) in the reply allow the UDP
masquerading rule to match, which rewrites the source port to or above
1024. This prevents the DHCP client in OVMF from accepting the packet.
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=709418
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
The functions
- iptablesAddForwardDontMasquerade(),
- iptablesRemoveForwardDontMasquerade
handle exceptions in the masquerading implemented in the POSTROUTING chain
of the "nat" table. Such exceptions should be added as chronologically
latest, logically top-most rules.
The bridge driver will call these functions beginning with the next patch:
some special destination IP addresses always refer to the local
subnetwork, even though they don't match any practical subnetwork's
netmask. Packets from virbrN targeting such IP addresses are never routed
outwards, but the current rules treat them as non-virbrN-destined packets
and masquerade them. This causes problems for some receivers on virbrN.
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
The previous patches added infrastructure to report better errors from
monitor in some cases. This patch finalizes this "feature" by enabling
this enhanced error reporting on early phases of VM startup. In these
phases the possibility of qemu producing a useful error message is
really high compared to running it during the whole life cycle. After
the start up is complete, the feature is disabled to provide the usual
error messages so that users are not confused by possibly irrelevant
messages that may be in the domain log.
The original motivation to do this enhancement is to capture errors when
using VFIO device passthrough, where qemu reports errors after the
monitor is initialized and the existing error catching code couldn't
catch this producing a unhelpful message:
# virsh start test
error: Failed to start domain test
error: Unable to read from monitor: Connection reset by peer
With this change, the message is changed to:
# virsh start test
error: Failed to start domain test
error: internal error: early end of file from monitor: possible problem:
qemu-system-x86_64: -device vfio-pci,host=00:1a.0,id=hostdev0,bus=pci.0,addr=0x5: vfio: error, group 8 is not viable, please ensure all devices within the iommu_group are bound to their vfio bus driver.
qemu-system-x86_64: -device vfio-pci,host=00:1a.0,id=hostdev0,bus=pci.0,addr=0x5: vfio: failed to get group 8
qemu-system-x86_64: -device vfio-pci,host=00:1a.0,id=hostdev0,bus=pci.0,addr=0x5: Device 'vfio-pci' could not be initialized
Change the monitor error code to add the ability to access the qemu log
file using a file descriptor so that we can dig in it for a more useful
error message. The error is now logged on monitor hangups and overwrites
a possible lesser error. A hangup on the monitor usualy means that qemu
has crashed and there's a significant chance it produced a useful error
message.
The functionality will be latent until the next patch.
Early VM startup errors usually produce a better error message in the
machine log file. Currently we were accessing it only when the process
exited during certain phases of startup. This will help adding a more
comprehensive error extraction for early qemu startup phases.
This patch adds infrastructure to keep a file descriptor for the machine
log file that will be used in case an error happens.
Teach the function to skip character device definitions printed by qemu
at startup in addition to libvirt log messages and make it usable from
outside of qemu_process.c. Also add documentation about the func.
The parsing of '-usb' did not check for failure of the
virDomainControllerInsert method. As a result on OOM, the
parser mistakenly attached USB disks to the IDE controller.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The code formatting NUMA args was ignoring the return value
of virBitmapFormat, so on OOM, it would silently drop the
NUMA cpumask arg.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
When building boot menu args, if OOM occurred the CLI args
would end up containing 'order=(null)' due to a missing
call to 'virBufferError'.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The testutils.c file had some fprintfs which had not been
converted from %d to %zu, when 'testCounter' change to be
a size_t. This was a build breaker if --enable-test-oom
was enabled
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Since we're about to freeze, it's time to pick up the latest
upstream gnulib. Among other changes, gnulib now guarantees the
use of some -f flags that we were previously manually adding.
* .gnulib: Update to latest, in part for warning improvements.
* m4/virt-compile-warnings.m4 (LIBVIRT_COMPILE_WARNINGS): Drop
flags that are now guaranteed by gnulib.
* bootstrap: Resync to gnulib.
Signed-off-by: Eric Blake <eblake@redhat.com>
On win32, using text mode for binary files might result in short
reads since ASCII character 0x1A is interpreted as EOF. Also, it
could lead to problems using the seek functions because of the \r
handling.
Signed-off-by: Claudio Bley <cbley@av-test.de>
N.B. This had no ill effects as long as O_RDONLY is defined to
to be 0, such that the expression (O_RDONLY < 0) yielded 0
again.
Signed-off-by: Claudio Bley <cbley@av-test.de>
The virCommandAddEnvPassCommon method ignored the failure to
pre-allocate the env variable array with VIR_RESIZE_N. While
this is harmless, it confuses the test harness which is trying
to validate OOM handling of every individual allocation call.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
When the various viralloc.c functions were changed to use the
normal error reporting code, the OOM injection code paths
were not updated to report errors.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The qemuParseCommandLine method did not check the return value of
virStringSplit to see if OOM had occurred. This lead to dereference
of a NULL pointer on OOM.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Most callers of qemuParseKeywords were assigning its return
value to a 'size_t' variable. Then then also checked '< 0'
for error condition, but this will never be true with the
unsigned size_t variable. Rather than using 'ssize_t', change
qemuParseKeywords so that the element count is returned via
an output parameter, leaving the return value solely as an
error indicator.
This avoids a crash accessing beyond the end of an error
upon OOM.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
In
commit 41b5505679
Author: Eric Blake <eblake@redhat.com>
Date: Wed Aug 28 15:01:23 2013 -0600
qemu: simplify list cleanup
The qemuStringToArgvEnv method was changed to use virStringFreeList
to free the 'arglist' array. This method assumes the string list
array is NULL terminated, however, qemuStringToArgvEnv was not
ensuring this when populating 'arglist'. This caused an out of
bounds access by virStringFreeList when OOM occured in the initial
loop of qemuStringToArgvEnv
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
When parsing the RBD hosts, it increments the 'nhosts' counter
before increasing the 'hosts' array allocation. If an OOM then
occurs when increasing the array allocation, the cleanup block
will attempt to access beyond the end of the array. Switch
to using VIR_EXPAND_N instead of VIR_REALLOC_N to protect against
this mistake
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
If OOM occurs in qemuDomainCCWAddressSetCreate, it jumps to
a cleanup block and frees the partially initialized object.
It then mistakenly returns the address of the just free'd
pointer instead of NULL.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
If a OOM error occurs in virGetConnect, this may cause the
virConnectDispose method to de-reference a NULL pointer,
since the close callback will not have been initialized.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The virDomainDefParseXML method did not check the return value
of the virBitmapNew API call for NULL. This lead to a crash on
OOM
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
If an OOM error occurs in virSecurityDeviceLabelDefParseXML the
cleanup code may free an uninitialized pointer, causing a crash
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Start a test case for the virNetServerClient object, which
initially checks the creation of a virIdentityPtr object.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
To allow creation of a virNetSocketPtr instance from a pre-opened
socketpair FD, add a virNetSocketNewConnectSockFD method.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The new function virConnectGetCPUModelNames allows to retrieve the list
of CPU models known by the hypervisor for a specific architecture.
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
Nehal J. Wani reported on IRC a rather interesting build failure:
In file included from util/virnetdevbridge.c:53:0:
/usr/include/linux/in6.h:30:8: error: redefinition of 'struct in6_addr'
struct in6_addr {
^
I traced it to the fact that he ran 'git pull; make check' across
commit e62e0094. What happened is that the configure changes
result in a new variable that was set to be defined on his system,
but config.h was not regenerated to contain the value of that
variable. Running 'make' instead of 'make check' cleaned up the
problem. A bit more investigation, and I see that in Makefile.am,
automake sticks rules that rebuild config.h as part of 'make all',
and that we also had a dependency 'check-local: all'; BUT the
rule for check-local is run only at the point when the top-level
directory is visited. Automake documents that SUBDIRS should
contain an explicit '.' at the point the top-level should be
visited (defaulting to last, if it doesn't appear). Sure enough,
with this patch, 'make check' now does the top-level 'all' rules,
which regenerates 'config.h' BEFORE compiling any code that might
depend on changed content of that file.
* Makefile.am (SUBDIRS): Put '.' first, not last.
Signed-off-by: Eric Blake <eblake@redhat.com>
The fix for CVE-2013-4311 had a pre-requisite enhancement
to the identity code
commit db7a5688c0
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>
