1. 23 1月, 2015 2 次提交
  2. 24 12月, 2014 5 次提交
  3. 13 11月, 2014 1 次提交
    • L
      util: eliminate "use after free" in callers of virNetDevLinkDump · a5314b47
      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
      a5314b47
  4. 07 11月, 2014 1 次提交
    • E
      CVE-2014-7823: dumpxml: security hole with migratable flag · 757292bf
      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>
      757292bf
  5. 02 10月, 2014 1 次提交
  6. 18 9月, 2014 1 次提交
  7. 03 7月, 2014 2 次提交
    • E
      build: fix 'make check' with newer git · 5e556055
      Eric Blake 提交于
      Newer git doesn't like the maint.mk rule 'public-submodule-commit'
      that was associated with the version of maint.mk in our checkout
      of gnulib.  I tracked down that libvirt commit 8531301d picked up
      a gnulib fix that makes git happy.  Basically, that libvirt commit
      uses maint.mk from gnulib.git commit d18d1b802.  Rather than try
      and backport .gnulib submodule changes, this is a downstream
      variant that uses just the newer maint.mk from the known-good
      submodule state, as follows:
      
      mkdir -p gnulib/local/top
      cd .gnulib
      git checkout d18d1b802 top/maint.mk
      git diff HEAD > ../gnulib/local/top/maint.mk.diff
      git reset --hard
      cd ..
      git add gnulib/local/top
      Signed-off-by: NEric Blake <eblake@redhat.com>
      5e556055
    • P
      qemu: copy: Accept 'format' parameter when copying to a non-existing img · a103b53f
      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
      a103b53f
  8. 27 6月, 2014 2 次提交
    • E
      docs: publish correct enum values · a85bb493
      Eric Blake 提交于
      We publish libvirt-api.xml for others to use, and in fact, the
      libvirt-python bindings use it to generate python constants that
      correspond to our enum values.  However, we had an off-by-one bug
      that any enum that relied on C's rules for implicit initialization
      of the first enum member to 0 got listed in the xml as having a
      value of 1 (and all later members of the enum were equally
      botched).
      
      The fix is simple - since we add one to the previous value when
      encountering an enum without an initializer, the previous value
      must start at -1 so that the first enum member is assigned 0.
      
      The python generator code has had the off-by-one ever since DV
      first wrote it years ago, but most of our public enums were immune
      because they had an explicit = 0 initializer.  The only affected
      enums are:
      - virDomainEventGraphicsAddressType (such as
      VIR_DOMAIN_EVENT_GRAPHICS_ADDRESS_IPV4), since commit 987e31ed
      (libvirt v0.8.0)
      - virDomainCoreDumpFormat (such as VIR_DOMAIN_CORE_DUMP_FORMAT_RAW),
      since commit 9fbaff00 (libvirt v1.2.3)
      - virIPAddrType (such as VIR_IP_ADDR_TYPE_IPV4), since commit
      03e0e79e (not yet released)
      
      Thanks to Nehal J Wani for reporting the problem on IRC, and
      for helping me zero in on the culprit function.
      
      * docs/apibuild.py (CParser.parseEnumBlock): Fix implicit enum
      values.
      Signed-off-by: NEric Blake <eblake@redhat.com>
      (cherry picked from commit 9b291bbe)
      a85bb493
    • P
      qemu: blockcopy: Don't remove existing disk mirror info · 69380800
      Peter Krempa 提交于
      When creating a new disk mirror the new struct is stored in a separate
      variable until everything went well. The removed hunk would actually
      remove existing mirror information for example when the api would be run
      if a mirror still exists.
      
      (cherry picked from commit 02b364e1)
      
      This fixes a regression introduced in commit ff5f30b6.
      Signed-off-by: NEric Blake <eblake@redhat.com>
      
      Conflicts:
      	src/qemu/qemu_driver.c - no refactoring of commits 7b7bf001, 4f202266
      69380800
  9. 06 5月, 2014 1 次提交
  10. 01 5月, 2014 1 次提交
  11. 10 4月, 2014 3 次提交
  12. 20 3月, 2014 1 次提交
    • M
      virNetClientSetTLSSession: Restore original signal mask · 57170e15
      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)
      57170e15
  13. 13 3月, 2014 1 次提交
    • P
      qemu: Avoid operations on NULL monitor if VM fails early · fe741707
      Peter Krempa 提交于
      https://bugzilla.redhat.com/show_bug.cgi?id=1047659
      
      If a VM dies very early during an attempted connect to the guest agent
      while the locks are down the domain monitor object will be freed. The
      object is then accessed later as any failure during guest agent startup
      isn't considered fatal.
      
      In the current upstream version this doesn't lead to a crash as
      virObjectLock called when entering the monitor in
      qemuProcessDetectVcpuPIDs checks the pointer before attempting to
      dereference (lock) it. The NULL pointer is then caught in the monitor
      helper code.
      
      Before the introduction of virObjectLockable - observed on 0.10.2 - the
      pointer is locked directly via virMutexLock leading to a crash.
      
      To avoid this problem we need to differentiate between the guest agent
      not being present and the VM quitting when the locks were down. The fix
      reorganizes the code in qemuConnectAgent to add the check and then adds
      special handling to the callers.
      
      (cherry picked from commit b952cbbc)
      fe741707
  14. 10 3月, 2014 1 次提交
    • D
      Add a mutex to serialize updates to firewall · 98069ba7
      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)
      98069ba7
  15. 18 2月, 2014 14 次提交
  16. 05 2月, 2014 1 次提交
    • E
      event: move event filtering to daemon (regression fix) · b19f3269
      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)
      
      Conflicts:
      	daemon/remote.c - not backporting network events
      	src/conf/network_event.c - likewise
      	src/conf/network_event.h - likewise
      	src/network/bridge_driver.c - likewise
      	src/conf/domain_event.c - revert back to pre-CVE state
      	src/conf/domain_event.h - likewise
      	src/libxl/libxl_driver.c - likewise
      	src/lxc/lxc_driver.c - likewise
      	src/remote/remote_driver.c - likewise
      	src/test/test_driver.c - likewise
      	src/uml/uml_driver.c - likewise
      	src/xen/xen_driver.c - likewise
      b19f3269
  17. 04 2月, 2014 2 次提交
    • D
      Push nwfilter update locking up to top level · f3aa956f
      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)
      f3aa956f
    • D
      Add a read/write lock implementation · 4912cccd
      Daniel P. Berrange 提交于
      Add virRWLock backed up by a POSIX rwlock primitive
      Signed-off-by: NDaniel P. Berrange <berrange@redhat.com>
      (cherry picked from commit c065984b)
      4912cccd