1. 30 6月, 2016 1 次提交
  2. 16 12月, 2015 6 次提交
    • E
      CVE-2015-5313: storage: don't allow '/' in filesystem volume names · 02d365da
      Eric Blake 提交于
      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)
      02d365da
    • M
      util: Prepare URI formatting for libxml2 >= 2.9.2 · 614c8d4c
      Martin Kletzander 提交于
      Since commit 8eb55d782a2b9afacc7938694891cc6fad7b42a5 libxml2 removes
      two slashes from the URI when there is no server part.  This is fixed
      with beb7281055dbf0ed4d041022a67c6c5cfd126f25, but only if the calling
      application calls xmlSaveUri() on URI that xmlURIParse() parsed.  And
      that is not the case in virURIFormat().  virURIFormat() accepts
      virURIPtr that can be created without parsing it and we do that when we
      format network storage paths for gluster for example.  Even though
      virStorageSourceParseBackingURI() uses virURIParse(), it throws that data
      structure right away.
      
      Since we want to format URIs as URIs and not absolute URIs or opaque
      URIs (see RFC 3986), we can specify that with a special hack thanks to
      commit beb7281055dbf0ed4d041022a67c6c5cfd126f25, by setting port to -1.
      
      This fixes qemuxml2argvtest test where the disk-drive-network-gluster
      case was failing.
      Signed-off-by: NMartin Kletzander <mkletzan@redhat.com>
      (cherry picked from commit 8f17d0ea)
      614c8d4c
    • D
      avoid using deprecated udev logging functions · 73174ca1
      Daniel P. Berrange 提交于
      In systemd >= 218, the udev_set_log_fn method has been marked
      deprecated and turned into a no-op. Nothing in the udev client
      library will print to stderr by default anymore, so we can
      just stop installing a logging hook for new enough udev.
      
      (cherry picked from commit a93a3b97)
      73174ca1
    • J
      qemu_driver: Resolve Coverity CONSTANT_EXPRESSION_RESULT · 871da174
      John Ferlan 提交于
      The call to virDomainSnapshotRedefinePrep() had a spurrious ! in front of
      it which caused Coverity to complan that the expression is always false.
      
      (cherry picked from commit 9d7254de)
      871da174
    • J
      Properly check the return value of CCWAddressAsString · bc52fd9d
      Ján Tomko 提交于
      It returns NULL on failure. Checking if the negation of it
      is less than zero makes no sense. (Found by coverity after moving
      the code)
      
      In another case, the return value wasn't checked at all.
      
      (cherry picked from commit 3fe9d75a)
      
      Conflicts:
      	src/conf/domain_addr.c - no code movement from commit b2626755
      bc52fd9d
    • D
      libxl: don't break the build on Xen>=4.5 because of libxl_vcpu_setaffinity() · 7f6b0960
      Dario Faggioli 提交于
      libxl interface for vcpu pinning is changing in Xen 4.5. Basically,
      libxl_set_vcpuaffinity() now wants one more parameter. That is
      representative of 'VCPU soft affinity', which libvirt does not use.
      
      To mark such change, the macro LIBXL_HAVE_VCPUINFO_SOFT_AFFINITY is
      defined. Use it as a gate and, if present, re-#define the calls from
      the old to the new interface, to avoid breaking the build.
      Signed-off-by: NDario Faggioli <dario.faggioli@citrix.com>
      Cc: Jim Fehlig <jfehlig@suse.com>
      Cc: Ian Campbell <Ian.Campbell@citrix.com>
      Cc: Ian Jackson <Ian.Jackson@eu.citrix.com>
      (cherry picked from commit bfc72e99)
      7f6b0960
  3. 03 9月, 2015 1 次提交
    • M
      remoteClientCloseFunc: Don't mangle connection object refcount · c735ddf5
      Michal Privoznik 提交于
      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>
      c735ddf5
  4. 23 1月, 2015 2 次提交
  5. 23 12月, 2014 1 次提交
  6. 13 11月, 2014 1 次提交
    • L
      util: eliminate "use after free" in callers of virNetDevLinkDump · fe4fc3fd
      Laine Stump 提交于
      virNetDevLinkDump() gets a message from netlink into "resp", then
      calls nlmsg_parse() to fill the table "tb" with pointers into resp. It
      then returns tb to its caller, but not before freeing the buffer at
      resp. That means that all the callers of virNetDevLinkDump() are
      examining memory that has already been freed. This can be verified by
      filling the buffer at resp with garbage prior to freeing it (or, I
      suppose, just running libvirtd under valgrind) then performing some
      operation that calls virNetDevLinkDump().
      
      The upstream commit log incorrectly states that the code has been like
      this ever since virNetDevLinkDump() was written. In reality, the
      problem was introduced with commit e95de74d, first in libvirt-1.0.5,
      which was attempting to eliminate a typecast that caused compiler
      warnings. It has only been pure luck (or maybe a lack of heavy load,
      and/or maybe an allocation algorithm in malloc() that delays re-use of
      just-freed memory) that has kept this from causing errors, for example
      when configuring a PCI passthrough or macvtap passthrough network
      interface.
      
      The solution taken in this patch is the simplest - just return resp to
      the caller along with tb, then have the caller free it after they are
      finished using the data (pointers) in tb. I alternately could have
      made a cleaner interface by creating a new struct that put tb and resp
      together along with a vir*Free() function for it, but this function is
      only used in a couple places, and I'm not sure there will be
      additional new uses of virNetDevLinkDump(), so the value of adding a
      new type, extra APIs, etc. is dubious.
      
      (cherry picked from commit f9f9699f)
      
      Conflicts:
      	src/util/virnetdevvportprofile.c - change in cleanup label indentation
      fe4fc3fd
  7. 07 11月, 2014 1 次提交
    • E
      CVE-2014-7823: dumpxml: security hole with migratable flag · 3adae530
      Eric Blake 提交于
      Commit 28f8dfdc (v1.0.0) introduced a security hole: in at least
      the qemu implementation of virDomainGetXMLDesc, the use of the
      flag VIR_DOMAIN_XML_MIGRATABLE (which is usable from a read-only
      connection) triggers the implicit use of VIR_DOMAIN_XML_SECURE
      prior to calling qemuDomainFormatXML.  However, the use of
      VIR_DOMAIN_XML_SECURE is supposed to be restricted to read-write
      clients only.  This patch treats the migratable flag as requiring
      the same permissions, rather than analyzing what might break if
      migratable xml no longer includes secret information.
      
      Fortunately, the information leak is low-risk: all that is gated
      by the VIR_DOMAIN_XML_SECURE flag is the VNC connection password;
      but VNC passwords are already weak (FIPS forbids their use, and
      on a non-FIPS machine, anyone stupid enough to trust a max-8-byte
      password sent in plaintext over the network deserves what they
      get).  SPICE offers better security than VNC, and all other
      secrets are properly protected by use of virSecret associations
      rather than direct output in domain XML.
      
      * src/remote/remote_protocol.x (REMOTE_PROC_DOMAIN_GET_XML_DESC):
      Tighten rules on use of migratable flag.
      * src/libvirt-domain.c (virDomainGetXMLDesc): Likewise.
      Signed-off-by: NEric Blake <eblake@redhat.com>
      (cherry picked from commit b1674ad5)
      
      Conflicts:
      	src/libvirt-domain.c - file split from older src/libvirt.c; context with older virLibConnError
      Signed-off-by: NEric Blake <eblake@redhat.com>
      3adae530
  8. 02 10月, 2014 1 次提交
  9. 18 9月, 2014 1 次提交
  10. 03 7月, 2014 1 次提交
    • P
      qemu: copy: Accept 'format' parameter when copying to a non-existing img · a73122a4
      Peter Krempa 提交于
      We have the following matrix of possible arguments handled by the logic
      statement touched by this patch:
             | flags & _REUSE_EXT | !(flags & _REUSE_EXT)
      -------+--------------------+----------------------
       format| (1)                | (2)
      -------+--------------------+----------------------
      !format| (3)                | (4)
      -------+--------------------+----------------------
      
      In cases 1 and 2 the user provided a format, in cases 3 and 4 not. The
      user requests to use a pre-existing image in 1 and 3 and libvirt will
      create a new image in 2 and 4.
      
      The difference between cases 3 and 4 is that for 3 the format is probed
      from the user-provided image, whereas in 4 we just use the existing disk
      format.
      
      The current code would treat cases 1,3 and 4 correctly but in case 2 the
      format provided by the user would be ignored.
      
      The particular piece of code was broken in commit 35c7701c
      but since it was introduced a few commits before that it was never
      released as working.
      
      (cherry picked from commit 42619ed0)
      Signed-off-by: NEric Blake <eblake@redhat.com>
      
      Conflicts:
      	src/qemu/qemu_driver.c - no refactoring of commits 7b7bf001, 4f202266
      a73122a4
  11. 27 6月, 2014 1 次提交
  12. 06 5月, 2014 1 次提交
  13. 01 5月, 2014 1 次提交
  14. 10 4月, 2014 3 次提交
  15. 20 3月, 2014 1 次提交
    • M
      virNetClientSetTLSSession: Restore original signal mask · 93394f56
      Michal Privoznik 提交于
      Currently, we use pthread_sigmask(SIG_BLOCK, ...) prior to calling
      poll(). This is okay, as we don't want poll() to be interrupted.
      However, then - immediately as we fall out from the poll() - we try to
      restore the original sigmask - again using SIG_BLOCK. But as the man
      page says, SIG_BLOCK adds signals to the signal mask:
      
      SIG_BLOCK
            The set of blocked signals is the union of the current set and the set argument.
      
      Therefore, when restoring the original mask, we need to completely
      overwrite the one we set earlier and hence we should be using:
      
      SIG_SETMASK
            The set of blocked signals is set to the argument set.
      Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
      (cherry picked from commit 3d4b4f5a)
      93394f56
  16. 10 3月, 2014 1 次提交
    • D
      Add a mutex to serialize updates to firewall · d5a14a1a
      Daniel P. Berrange 提交于
      The nwfilter conf update mutex previously serialized
      updates to the internal data structures for firewall
      rules, and updates to the firewall itself. The latter
      was recently turned into a read/write lock, and filter
      instantiation allowed to proceed in parallel. It was
      believed that this was ok, since each filter is created
      on a separate iptables/ebtables chain.
      
      It turns out that there is a subtle lock ordering problem
      on virNWFilterObjPtr instances. __virNWFilterInstantiateFilter
      will hold a lock on the virNWFilterObjPtr it is instantiating.
      This in turn invokes virNWFilterInstantiate which then invokes
      virNWFilterDetermineMissingVarsRec which then invokes
      virNWFilterObjFindByName. This iterates over every single
      virNWFilterObjPtr in the list, locking them and checking their
      name. So if 2 or more threads try to instantiate a filter in
      parallel, they'll all hold 1 lock at the top level in the
      __virNWFilterInstantiateFilter method which will cause the
      other thread to deadlock in virNWFilterObjFindByName.
      
      The fix is to add an exclusive mutex to serialize the
      execution of __virNWFilterInstantiateFilter.
      Signed-off-by: NDaniel P. Berrange <berrange@redhat.com>
      (cherry picked from commit 925de19e)
      d5a14a1a
  17. 18 2月, 2014 14 次提交
  18. 05 2月, 2014 1 次提交
    • E
      event: move event filtering to daemon (regression fix) · e48e414e
      Eric Blake 提交于
      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)
      e48e414e
  19. 04 2月, 2014 1 次提交
    • D
      Push nwfilter update locking up to top level · c5d10b7a
      Daniel P. Berrange 提交于
      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)
      c5d10b7a