CVE-2016-5008

Setting an empty graphics password is documented as a way to disable
VNC/SPICE access, but QEMU does not always behaves like that. VNC would
happily accept the empty password. Let's enforce the behavior by setting
password expiration to "now".

https://bugzilla.redhat.com/show_bug.cgi?id=1180092Signed-off-by: NJiri Denemark <jdenemar@redhat.com>
(cherry picked from commit bb848fee)

The libvirt file system storage driver determines what file to
act on by concatenating the pool location with the volume name.
If a user is able to pick names like "../../../etc/passwd", then
they can escape the bounds of the pool.  For that matter,
virStoragePoolListVolumes() doesn't descend into subdirectories,
so a user really shouldn't use a name with a slash.

Normally, only privileged users can coerce libvirt into creating
or opening existing files using the virStorageVol APIs; and such
users already have full privilege to create any domain XML (so it
is not an escalation of privilege).  But in the case of
fine-grained ACLs, it is feasible that a user can be granted
storage_vol:create but not domain:write, and it violates
assumptions if such a user can abuse libvirt to access files
outside of the storage pool.

Therefore, prevent all use of volume names that contain "/",
whether or not such a name is actually attempting to escape the
pool.

This changes things from:

$ virsh vol-create-as default ../../../../../../etc/haha --capacity 128
Vol ../../../../../../etc/haha created
$ rm /etc/haha

to:

$ virsh vol-create-as default ../../../../../../etc/haha --capacity 128
error: Failed to create vol ../../../../../../etc/haha
error: Requested operation is not valid: volume name '../../../../../../etc/haha' cannot contain '/'
Signed-off-by: NEric Blake <eblake@redhat.com>
(cherry picked from commit 034e47c3)

Well, in 8ad126e6 we tried to fix a memory corruption problem.
However, the fix was not as good as it could be. I mean, the
commit has one line more than it should. I've noticed this output
just recently:

  # ./run valgrind --leak-check=full --show-reachable=yes ./tools/virsh domblklist gentoo
  ==17019== Memcheck, a memory error detector
  ==17019== Copyright (C) 2002-2013, and GNU GPL'd, by Julian Seward et al.
  ==17019== Using Valgrind-3.10.1 and LibVEX; rerun with -h for copyright info
  ==17019== Command: /home/zippy/work/libvirt/libvirt.git/tools/.libs/virsh domblklist gentoo
  ==17019==
  Target     Source
  ------------------------------------------------
  fda        /var/lib/libvirt/images/fd.img
  vda        /var/lib/libvirt/images/gentoo.qcow2
  hdc        /home/zippy/tmp/install-amd64-minimal-20150402.iso

  ==17019== Thread 2:
  ==17019== Invalid read of size 4
  ==17019==    at 0x4EFF5B4: virObjectUnref (virobject.c:258)
  ==17019==    by 0x5038CFF: remoteClientCloseFunc (remote_driver.c:552)
  ==17019==    by 0x5069D57: virNetClientCloseLocked (virnetclient.c:685)
  ==17019==    by 0x506C848: virNetClientIncomingEvent (virnetclient.c:1852)
  ==17019==    by 0x5082136: virNetSocketEventHandle (virnetsocket.c:1913)
  ==17019==    by 0x4ECD64E: virEventPollDispatchHandles (vireventpoll.c:509)
  ==17019==    by 0x4ECDE02: virEventPollRunOnce (vireventpoll.c:658)
  ==17019==    by 0x4ECBF00: virEventRunDefaultImpl (virevent.c:308)
  ==17019==    by 0x130386: vshEventLoop (vsh.c:1864)
  ==17019==    by 0x4F1EB07: virThreadHelper (virthread.c:206)
  ==17019==    by 0xA8462D3: start_thread (in /lib64/libpthread-2.20.so)
  ==17019==    by 0xAB441FC: clone (in /lib64/libc-2.20.so)
  ==17019==  Address 0x139023f4 is 4 bytes inside a block of size 240 free'd
  ==17019==    at 0x4C2B1F0: free (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
  ==17019==    by 0x4EA8949: virFree (viralloc.c:582)
  ==17019==    by 0x4EFF6D0: virObjectUnref (virobject.c:273)
  ==17019==    by 0x4FE74D6: virConnectClose (libvirt.c:1390)
  ==17019==    by 0x13342A: virshDeinit (virsh.c:406)
  ==17019==    by 0x134A37: main (virsh.c:950)

The problem is, when registering remoteClientCloseFunc(), it's
conn->closeCallback which is ref'd. But in the function itself
it's conn->closeCallback->conn what is unref'd. This is causing
imbalance in reference counting. Moreover, there's no need for
the remote driver to increase/decrease conn refcount since it's
not used anywhere. It's just merely passed to client registered
callback. And for that purpose it's correctly ref'd in
virConnectRegisterCloseCallback() and then unref'd in
virConnectUnregisterCloseCallback().
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
(cherry picked from commit e6893007)
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

Commit id '7c2d65dd' changed the default value of mode to be -1 if not
supplied in the XML, which should cause creation of the volume using the
default mode of VIR_STORAGE_DEFAULT_VOL_PERM_MODE; however, the check
made was whether mode was '0' or not to use default or provided value.

This patch fixes the issue to check if the 'mode' was provided in the XML
and use that value.

(cherry picked from commit 691dd388)

Commit id '155ca616' added the 'refreshVol' API. In an NFS root-squash
environment it was possible that if the just created volume from XML wasn't
properly created with the right uid/gid and/or mode, then the followup
refreshVol will fail to open the volume in order to get the allocation/
capacity values. This would leave the volume still on the server and
cause a libvirtd crash because 'voldef' would be in the pool list, but
the cleanup code would free it.

(cherry picked from commit db9277a3)

In an NFS root-squashed environment the 'vol-delete' command will fail to
'unlink' the target volume since it was created under a different uid:gid.

This code continues the concepts introduced in virFileOpenForked and
virDirCreate[NoFork] with respect to running the unlink command under
the uid/gid of the child. Unlike the other two, don't retry on EACCES
(that's why we're here doing this now).

(cherry picked from commit 35847860)

This reverts commit 1ce7c1d2,
which introduced a significant semantic change to the
virDomainGetInfo() API. Additionally, the change was only
made to 2 of the 15 virt drivers.

Conflicts:
	src/qemu/qemu_driver.c
Signed-off-by: NJim Fehlig <jfehlig@suse.com>
(cherry picked from commit 60acb38a)

Currently, when trying to virsh pool-define/virsh pool-build a new
'dir' pool, if the target directory already exists, virsh
pool-build/virStoragePoolBuild will error out. This is a change of
behaviour compared to eg libvirt 1.2.13

This is caused by the wrong type being used for the dir_create_flags
variable in virStorageBackendFileSystemBuild , it's defined as a bool
but is used as a flag bit field so should be unsigned int (this matches
the type virDirCreate expects for this variable).

This should fix https://bugzilla.gnome.org/show_bug.cgi?id=752417 (GNOME
Boxes) and https://bugzilla.redhat.com/show_bug.cgi?id=1244080
(downstream virt-manager).

Daemon used false logic for determining whether there were any clients.
When the timer was inactive, it was activated if at least one of the
servers did not have clients.  So the bool was being flipped there and
back all the time in case there was one client, for example.

Initially introduced by fa142073.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1240283Signed-off-by: NMartin Kletzander <mkletzan@redhat.com>
(cherry picked from commit b7ea58c2)
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

So callers don't have to iterate over each server.
Signed-off-by: NMartin Kletzander <mkletzan@redhat.com>
(cherry picked from commit 699faeac)
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

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

Commit id 'cd490086' added a VIR_FORCE_CLOSE of the 'sock', but it
was after the VIR_FREE() of phyp_driver, resulting in a possible/likely
NULL dereference.
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>

Convert virPCIDriverDir to return the buffer allocated (or not) and make the
appropriate check in the caller.
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>

Convert virPCIDriverFile to return the buffer allocated (or not) and make the
appropriate check in the caller.
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>

Convert virPCIFile to return the buffer allocated (or not) and make the
appropriate check in the caller.
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>

So, recently I was testing the LXC driver. You know, startup some
domains. But to my surprise, I was not able to start a single one:

  virsh # start --console test
  error: Reconnected to the hypervisor
  error: Failed to start domain test
  error: internal error: guest failed to start: unexpected exit status 125

So I've start digging. It turns out, that in virExec(), when I printed
out the @cmd, I got strange values: *(cmd->outfdptr) was certainly not
valid FD number: it has random value of several millions. This
obviously made prepareStdFd(childout, STDOUT_FILENO) fail (line 611).
But outfdptr is set in virCommandSetOutputFD(). The only place within
LXC driver where the function is called is in
virLXCProcessBuildControllerCmd(). If you take a closer look at the
function it looks like this:

static virCommandPtr
virLXCProcessBuildControllerCmd(virLXCDriverPtr driver,
                                ..
                                int logfd,
                                const char *pidfile)
{
    ...
    virCommandSetOutputFD(cmd, &logfd);
    virCommandSetErrorFD(cmd, &logfd);
    ...
}

Yes, you guessed it. @logfd is passed into the function by value.
However, in the function we try to get its address (an address of a
local variable) which is no longer valid once function is finished and
stack is cleaned. Therefore when cmd->outfdptr is evaluated at any
point after this function, we may get a random number, depending on
what's currently on the stack. Of course, this may work sometimes too
- it depends on the compiler how it arranges the code, when the stack
is wiped out.

In order to fix this, lets pass a pointer to @logfd instead of
figuring out (wrong) its value in a function.

The bug was introduced in e1de5521.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

Commit id 'f967e7a6' didn't place the closing parentheses quite right
causing DEADCODE errors since the rc setting/comparison was wrong.

Using a custom device tree image may cause unexpected behavior in
architectures that use this approach to detect platform devices. Since
usually the device tree is generated by qemu and thus it's not normally
used let's taint VMs using it to make it obvious as a possible source of
problems.

The memory device hot(un)plug was missing calls to the auditing code.

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

Since the balloon driver does not guarantee that it returns memory to
the host, using the value in the audit message is not a good idea.

This patch removes auditing from updating the balloon size and reports
the total physical size at startup.

The code which generates paths for UNIX socket blindly used target name
without checking if it was set. Thus for the following device XML

    <channel type='unix'>
      <source mode='bind'/>
      <target type='virtio'/>
    </channel>

we would generate "/var/lib/libvirt/qemu/channel/target/NAME.(null)"
path which works but is not really correct. Let's not use the
".target_name" suffix at all if target name is not set.

https://bugzilla.redhat.com/show_bug.cgi?id=1226854Signed-off-by: NJiri Denemark <jdenemar@redhat.com>

While CPU0 was made unpluggable in Linux a while ago it's not desirable
to unplug it since some parts of the kernel (suspend-to-ram) still
depend on it.

This patch fixes the vCPU selection code in libvirt so that it will not
be disabled.

Signed-off-by: NLuyao Huang <lhuang@redhat.com>

The target type comparison in qemuDomainDetachChrDevice
used the VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE enum, so virtio-serial
addresses were not freed properly for channel devices.

Call qemuDomainReleaseDeviceAddress uncoditionally and decide
based on the address type instead of the target/device types.

Also check the device type when deciding what type the address should
be. Commit 9807c471 (aiming to fix another error in address allocation)
only checked the target type, but its value is different for different
device types. This resulted in an error when trying to attach
a channel with target type 'virtio':

error: Failed to attach device from channel-file.xml
error: internal error: virtio serial device has invalid address type

Make the logic for releasing the address dependent only on
* the address type
* whether it was allocated earlier
to avoid copying the device and target type checks.

https://bugzilla.redhat.com/show_bug.cgi?id=1230039Signed-off-by: NLuyao Huang <lhuang@redhat.com>
Signed-off-by: NJán Tomko <jtomko@redhat.com>

def->vcpus was never updated after successfully changing the live
vcpu count of a domain. Subsequent queries for vcpu info would
return incorrect results.  E.g.:

virsh vcpucount test
maximum      config         4
maximum      live           4
current      config         4
current      live           4

virsh setvcpus test 2

virsh vcpucount test
maximum      config         4
maximum      live           4
current      config         4
current      live           4

After patch, live current config is reported correctly:

virsh vcpucount test
maximum      config         4
maximum      live           4
current      config         4
current      live           2

While fixing this, noticed that the live config was not saved
to cfg->stateDir via virDomainSaveStatus. Save the live config
and change error handling of virDomainSave{Config,Status} to
log a message via VIR_WARN, instead of failing the entire
DomainSetVcpusFlags operation.
Signed-off-by: NJim Fehlig <jfehlig@suse.com>

The libxl driver always uses virDomainObj->def when formatting
the domain XML description.  Use virDomainObj->newDef when
--inactive flag is set.
Signed-off-by: NJim Fehlig <jfehlig@suse.com>

libxlDomainCreateXML() would remove a persistent domain if
libxlDomainStart() failed.  Check if domain is persistent
before removing.
Signed-off-by: NJim Fehlig <jfehlig@suse.com>

When restarting libvirtd and reconnecting to running domains,
libxlReconnectDomain() would unconditionally set the domain state
to VIR_DOMAIN_RUNNING, overwriting the state maintained in
$statedir/<domname>.xml.  A domain in a paused state would have
the state changed to running, even though it was actually in a
paused state.
Signed-off-by: NJim Fehlig <jfehlig@suse.com>

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

When the domain "<on_crash>coredump-destroy</on_crash>" is set, the
domain wasn't being destroyed, rather it was being rebooted.

Add VIR_DOMAIN_LIFECYCLE_CRASH_COREDUMP_DESTROY to the list of
on_crash types that cause "-no-reboot" to be added to the qemu
command line.

Although defined the same way, fortunately there hadn't been any deviation.
Ensure any assignments to onCrash use VIR_DOMAIN_LIFECYCLE_CRASH_* defs and
not VIR_DOMAIN_LIFECYCLE_* defs

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

Since an mpath pool contains all the Multipath devices on a host, allowing
more than one defined on a host at a time should be disallowed under the
policy of disallowing duplicate source pools for the host.

Adjust to docs to clarify the Multipath target path value usage for both
the storage driver (only 1 pool per host) and formatstorage references
(ignore the target element in favor of the default target mapping of
/dev/mapper).

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

Per the devmapper docs, use "/dev/mapper" or "/dev/dm-n" in order to
determine if a device is under control of DM Multipath.

So add "/dev/mapper" to the virFileExists, leaving the "/dev/mpath"
as a "legacy" option since it appears for a while it was the preferred
mechanism, but is no longer maintained

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

The formatdomain.html description for <disk> device 'lun' indicates that
it must be either a type 'block' or type 'network' with protocol 'iscsi';
however, we did not make that check until domain startup.

This caused issues for virt-manager which had an unexpected failure at
run time rather config time.

This patch adds a check in post part disk device checking for the specific
and supported lun types as well as adjusting the test failure to be for
parse config rather than run time.

Libvirt periodically refreshes all volumes in a storage pool, including
the volumes being cloned.
While cloning a storage volume from parent, we drop pool locks. Subsequent
volume refresh sometimes changes allocation for an ongoing copy, and leads
to corrupt images.
Fix: Introduce a shadow volume that isolates the volume object under refresh
from the base which has a copy ongoing.
Signed-off-by: NPrerna Saxena <prerna@linux.vnet.ibm.com>
Signed-off-by: NJán Tomko <jtomko@redhat.com>

The pool name has to be the same too to warrant rejecting a pool
definition as duplicate. This regression was introduced in commit
2184ade3.

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

Get rid of spice specific stuff from the handler func and save a few
lines by reflowing the conditions.

Spice events have mostly similar information present in the event JSON
but they differ in the name of the element containing the port.

The JSON event also provides connection ID which might be useful in the
future.

This patch splits up the event parser code into two functions and the
SPICE reimplements the event parsing with correct names and drops the
VNC only stuff.

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

The capability was not used up to the feature freeze.

This reverts commit 7f3515b4.

processSerialChangedEvent processes events for all channels. Commit
2af51483 broke all agent interaction if a channel other than the agent
closes since it did not check that the event actually originated from
the guest agent channel.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1236924
Fixes up: https://bugzilla.redhat.com/show_bug.cgi?id=890648