1. 20 3月, 2014 1 次提交
    • M
      virNetClientSetTLSSession: Restore original signal mask · f1725e60
      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)
      f1725e60
  2. 10 3月, 2014 7 次提交
    • E
      storage: use valid XML for awkward volume names · 45d40bcf
      Eric Blake 提交于
      $ touch /var/lib/libvirt/images/'a<b>c'
      $ virsh pool-refresh default
      $ virsh vol-dumpxml 'a<b>c' default | head -n2
      <volume>
        <name>a<b>c</name>
      
      Oops.  That's not valid XML.  And when we fix the XML
      generation, it fails RelaxNG validation.
      
      I'm also tired of seeing <key>(null)</key> in the example
      output for volume xml; while we used NULLSTR() to avoid
      a NULL deref rather than relying on glibc's printf
      extension behavior, it's even better if we avoid the issue
      in the first place.  But this requires being careful that
      we don't invalidate any storage backends that were relying
      on key being unassigned during virStoragVolCreateXML[From].
      
      I would have split this into two patches (one for escaping,
      one for avoiding <key>(null)</key>), but since they both
      end up touching a lot of the same test files, I ended up
      merging it into one.
      
      Note that this patch allows pretty much any volume name
      that can appear in a directory (excluding . and .. because
      those are special), but does nothing to change the current
      (unenforced) RelaxNG claim that pool names will consist
      only of letters, numbers, _, -, and +.  Tightening the C
      code to match RelaxNG patterns and/or relaxing the grammar
      to match the C code for pool names is a task for another
      day (but remember, we DID recently tighten C code for
      domain names to exclude a leading '.').
      
      * src/conf/storage_conf.c (virStoragePoolSourceFormat)
      (virStoragePoolDefFormat, virStorageVolTargetDefFormat)
      (virStorageVolDefFormat): Escape user-controlled strings.
      (virStorageVolDefParseXML): Parse key, for use in unit tests.
      * src/storage/storage_driver.c (storageVolCreateXML)
      (storageVolCreateXMLFrom): Ensure parsed key doesn't confuse
      volume creation.
      * docs/schemas/basictypes.rng (volName): Relax definition.
      * tests/storagepoolxml2xmltest.c (mymain): Test it.
      * tests/storagevolxml2xmltest.c (mymain): Likewise.
      * tests/storagepoolxml2xmlin/pool-dir-naming.xml: New file.
      * tests/storagepoolxml2xmlout/pool-dir-naming.xml: Likewise.
      * tests/storagevolxml2xmlin/vol-file-naming.xml: Likewise.
      * tests/storagevolxml2xmlout/vol-file-naming.xml: Likewise.
      * tests/storagevolxml2xmlout/vol-*.xml: Fix fallout.
      Signed-off-by: NEric Blake <eblake@redhat.com>
      (cherry picked from commit 6cc4d6a3)
      45d40bcf
    • E
      maint: fix comma style issues: conf · 5fdc3e6e
      Eric Blake 提交于
      Most of our code base uses space after comma but not before;
      fix the remaining uses before adding a syntax check.
      
      * src/conf/capabilities.c: Consistently use commas.
      * src/conf/domain_conf.c: Likewise.
      * src/conf/network_conf.c: Likewise.
      * src/conf/storage_conf.c: Likewise.
      Signed-off-by: NEric Blake <eblake@redhat.com>
      (cherry picked from commit 6f4901e1)
      5fdc3e6e
    • M
      virNetServerRun: Notify systemd that we're accepting clients · b7288926
      Michal Privoznik 提交于
      Systemd does not forget about the cases, where client service needs to
      wait for daemon service to initialize and start accepting new clients.
      Setting a dependency in client is not enough as systemd doesn't know
      when the daemon has initialized itself and started accepting new
      clients. However, it offers a mechanism to solve this. The daemon needs
      to call a special systemd function by which the daemon tells "I'm ready
      to accept new clients". This is exactly what we need with
      libvirtd-guests (client) and libvirtd (daemon). So now, with this
      change, libvirt-guests.service is invoked not any sooner than
      libvirtd.service calls the systemd notify function.
      Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
      (cherry picked from commit 68954fb2)
      b7288926
    • M
      libvirt-guests: Wait for libvirtd to initialize · 73307499
      Michal Privoznik 提交于
      I've noticed that in some cases systemd was quick enough and even
      if libvirt-guests.service is marked to be started after the
      libvirtd.service my guests were not resumed as
      libvirt-guests.sh failed to connect. This is because of a
      simple fact: systemd correctly starts libvirt-guests after it
      execs libvirtd. However, the daemon is not able to accept
      connections right from the start. It's doing some
      initialization which may take ages. This problem is not limited
      to systemd only, indeed. Any init system that is able to startup
      services in parallel (e.g. OpenRC) may run into this situation.
      The fix is to try connecting not only once, but continuously a few
      times with a small sleep in between tries.
      Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
      (cherry picked from commit 4e7fc830)
      73307499
    • M
      virSystemdCreateMachine: Set dependencies for slices · 484cec0b
      Michal Privoznik 提交于
      https://bugzilla.redhat.com/show_bug.cgi?id=1031696
      
      When creating a new domain, we let systemd know about it by calling
      CreateMachine() function via dbus. Systemd then creates a scope and
      places domain into it. However, later when the host is shutting
      down, systemd computes the shutdown order to see what processes can
      be shut down in parallel. And since we were not setting
      dependencies at all, the slices (and thus domains) were most likely
      killed before libvirt-guests.service. So user domains that had to
      be saved, shut off, whatever were in fact killed.  This problem can
      be solved by letting systemd know that scopes we're creating must
      not be killed before libvirt-guests.service.
      Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
      (cherry picked from commit ba79e387)
      484cec0b
    • G
      Add Documentation fields to systemd service files · 901aa960
      Guido Günther 提交于
      We point to the manpages where available and redirect to libvirt's
      homepage as a last resort.
      
      (cherry picked from commit 1b9f5aa7)
      901aa960
    • D
      Add a mutex to serialize updates to firewall · 034a4d7a
      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)
      034a4d7a
  3. 19 2月, 2014 18 次提交
  4. 05 2月, 2014 1 次提交
    • E
      event: move event filtering to daemon (regression fix) · fad8d7df
      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
      fad8d7df
  5. 04 2月, 2014 2 次提交
    • D
      Push nwfilter update locking up to top level · 978648de
      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)
      978648de
    • D
      Add a read/write lock implementation · 7ca05e2f
      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)
      7ca05e2f
  6. 29 1月, 2014 4 次提交
  7. 21 1月, 2014 1 次提交
  8. 17 1月, 2014 3 次提交
    • C
      Prep for release 1.1.3.3 · bc6b8388
      Cole Robinson 提交于
      bc6b8388
    • E
      virt-login-shell: fix regressions in behavior · 2ae5b3e3
      Eric Blake 提交于
      Our fixes for CVE-2013-4400 were so effective at "fixing" bugs
      in virt-login-shell that we ended up fixing it into a useless
      do-nothing program.
      
      Commit 3e2f27e1 picked the name LIBVIRT_SETUID_RPC_CLIENT for
      the witness macro when we are doing secure compilation.  But
      commit 9cd6a57d checked whether the name IN_VIRT_LOGIN_SHELL,
      from an earlier version of the patch series, was defined; with
      the net result that virt-login-shell invariably detected that
      it was setuid and failed virInitialize.
      
      Commit b7fcc799 closed all fds larger than stderr, but in the
      wrong place.  Looking at the larger context, we mistakenly did
      the close in between obtaining the set of namespace fds, then
      actually using those fds to switch namespace, which means that
      virt-login-shell will ALWAYS fail.
      
      This is the minimal patch to fix the regressions, although
      further patches are also worth having to clean up poor
      semantics of the resulting program (for example, it is rude to
      not pass on the exit status of the wrapped program back to the
      invoking shell).
      
      * tools/virt-login-shell.c (main): Don't close fds until after
      namespace swap.
      * src/libvirt.c (virGlobalInit): Use correct macro.
      Signed-off-by: NEric Blake <eblake@redhat.com>
      (cherry picked from commit 3d007cb5)
      2ae5b3e3
    • D
      Fix race leading to crash when setting up dbus watches · 3ba68920
      Daniel P. Berrange 提交于
      Currently the virDBusAddWatch does
      
        virEventAddHandle(fd, flags,
                          virDBusWatchCallback,
                          watch, NULL);
        dbus_watch_set_data(watch, info, virDBusWatchFree);
      
      Unfortunately this is racy - since the event loop is in a
      different thread, the virDBusWatchCallback method may be
      run before we get to calling dbus_watch_set_data. We must
      reverse the order of these calls
      
      See https://bugzilla.redhat.com/show_bug.cgi?id=885445Signed-off-by: NDaniel P. Berrange <berrange@redhat.com>
      (cherry picked from commit 7d3a1c8b)
      3ba68920
  9. 16 1月, 2014 3 次提交
    • E
      event: filter global events by domain:getattr ACL [CVE-2014-0028] · 51afa9a2
      Eric Blake 提交于
      Ever since ACL filtering was added in commit 76397360 (v1.1.1), a
      user could still use event registration to obtain access to a
      domain that they could not normally access via virDomainLookup*
      or virConnectListAllDomains and friends.  We already have the
      framework in the RPC generator for creating the filter, and
      previous cleanup patches got us to the point that we can now
      wire the filter through the entire object event stack.
      
      Furthermore, whether or not domain:getattr is honored, use of
      global events is a form of obtaining a list of networks, which
      is covered by connect:search_domains added in a93cd08f (v1.1.0).
      Ideally, we'd have a way to enforce connect:search_domains when
      doing global registrations while omitting that check on a
      per-domain registration.  But this patch just unconditionally
      requires connect:search_domains, even when no list could be
      obtained, based on the following observations:
      1. Administrators are unlikely to grant domain:getattr for one
      or all domains while still denying connect:search_domains - a
      user that is able to manage domains will want to be able to
      manage them efficiently, but efficient management includes being
      able to list the domains they can access.  The idea of denying
      connect:search_domains while still granting access to individual
      domains is therefore not adding any real security, but just
      serves as a layer of obscurity to annoy the end user.
      2. In the current implementation, domain events are filtered
      on the client; the server has no idea if a domain filter was
      requested, and must therefore assume that all domain event
      requests are global.  Even if we fix the RPC protocol to
      allow for server-side filtering for newer client/server combos,
      making the connect:serach_domains ACL check conditional on
      whether the domain argument was NULL won't benefit older clients.
      Therefore, we choose to document that connect:search_domains
      is a pre-requisite to any domain event management.
      
      Network events need the same treatment, with the obvious
      change of using connect:search_networks and network:getattr.
      
      * src/access/viraccessperm.h
      (VIR_ACCESS_PERM_CONNECT_SEARCH_DOMAINS)
      (VIR_ACCESS_PERM_CONNECT_SEARCH_NETWORKS): Document additional
      effect of the permission.
      * src/conf/domain_event.h (virDomainEventStateRegister)
      (virDomainEventStateRegisterID): Add new parameter.
      * src/conf/network_event.h (virNetworkEventStateRegisterID):
      Likewise.
      * src/conf/object_event_private.h (virObjectEventStateRegisterID):
      Likewise.
      * src/conf/object_event.c (_virObjectEventCallback): Track a filter.
      (virObjectEventDispatchMatchCallback): Use filter.
      (virObjectEventCallbackListAddID): Register filter.
      * src/conf/domain_event.c (virDomainEventFilter): New function.
      (virDomainEventStateRegister, virDomainEventStateRegisterID):
      Adjust callers.
      * src/conf/network_event.c (virNetworkEventFilter): New function.
      (virNetworkEventStateRegisterID): Adjust caller.
      * src/remote/remote_protocol.x
      (REMOTE_PROC_CONNECT_DOMAIN_EVENT_REGISTER)
      (REMOTE_PROC_CONNECT_DOMAIN_EVENT_REGISTER_ANY)
      (REMOTE_PROC_CONNECT_NETWORK_EVENT_REGISTER_ANY): Generate a
      filter, and require connect:search_domains instead of weaker
      connect:read.
      * src/test/test_driver.c (testConnectDomainEventRegister)
      (testConnectDomainEventRegisterAny)
      (testConnectNetworkEventRegisterAny): Update callers.
      * src/remote/remote_driver.c (remoteConnectDomainEventRegister)
      (remoteConnectDomainEventRegisterAny): Likewise.
      * src/xen/xen_driver.c (xenUnifiedConnectDomainEventRegister)
      (xenUnifiedConnectDomainEventRegisterAny): Likewise.
      * src/vbox/vbox_tmpl.c (vboxDomainGetXMLDesc): Likewise.
      * src/libxl/libxl_driver.c (libxlConnectDomainEventRegister)
      (libxlConnectDomainEventRegisterAny): Likewise.
      * src/qemu/qemu_driver.c (qemuConnectDomainEventRegister)
      (qemuConnectDomainEventRegisterAny): Likewise.
      * src/uml/uml_driver.c (umlConnectDomainEventRegister)
      (umlConnectDomainEventRegisterAny): Likewise.
      * src/network/bridge_driver.c
      (networkConnectNetworkEventRegisterAny): Likewise.
      * src/lxc/lxc_driver.c (lxcConnectDomainEventRegister)
      (lxcConnectDomainEventRegisterAny): Likewise.
      Signed-off-by: NEric Blake <eblake@redhat.com>
      (cherry picked from commit f9f56340)
      
      Conflicts:
      	src/conf/object_event.c - not backporting event refactoring
      	src/conf/object_event_private.h - likewise
      	src/conf/network_event.c - not backporting network events
      	src/conf/network_event.h - likewise
      	src/network/bridge_driver.c - likewise
      	src/access/viraccessperm.h - likewise
      	src/remote/remote_protocol.x - likewise
      	src/conf/domain_event.c - includes code that upstream has in object_event
      	src/conf/domain_event.h - context
      	src/libxl/libxl_driver.c - context
      	src/lxc/lxc_driver.c - context
      	src/remote/remote_driver.c - context, not backporting network events
      	src/test/test_driver.c - context, not backporting network events
      	src/uml/uml_driver.c - context
      	src/xen/xen_driver.c - context
      51afa9a2
    • E
      Fix memory leak in virObjectEventCallbackListRemoveID() · 271c0e7b
      Eric Blake 提交于
      While running objecteventtest, it was found that valgrind pointed out the
      following memory leak:
      
      ==13464== 5 bytes in 1 blocks are definitely lost in loss record 7 of 134
      ==13464==    at 0x4A0887C: malloc (vg_replace_malloc.c:270)
      ==13464==    by 0x341F485E21: strdup (strdup.c:42)
      ==13464==    by 0x4CAE28F: virStrdup (virstring.c:554)
      ==13464==    by 0x4CF3CBE: virObjectEventCallbackListAddID (object_event.c:286)
      ==13464==    by 0x4CF49CA: virObjectEventStateRegisterID (object_event.c:729)
      ==13464==    by 0x4CF73FE: virDomainEventStateRegisterID (domain_event.c:1424)
      ==13464==    by 0x4D7358F: testConnectDomainEventRegisterAny (test_driver.c:6032)
      ==13464==    by 0x4D600C8: virConnectDomainEventRegisterAny (libvirt.c:19128)
      ==13464==    by 0x402409: testDomainStartStopEvent (objecteventtest.c:232)
      ==13464==    by 0x403451: virtTestRun (testutils.c:138)
      ==13464==    by 0x402012: mymain (objecteventtest.c:395)
      ==13464==    by 0x403AF2: virtTestMain (testutils.c:593)
      ==13464==
      
      (cherry picked from commit 34d52b34)
      
      Conflicts:
      	src/conf/object_event.c - 1.2.1 refactoring to object_event not
      backported, so change applied directly in older domain_event.c instead
      271c0e7b
    • M
      virDomainEventCallbackListFree: Don't leak @list->callbacks · 4f169b0e
      Michal Privoznik 提交于
      The @list->callbacks is an array that is inflated whenever a new event
      is added, e.g. via virDomainEventCallbackListAddID(). However, when we
      are freeing the array, we free the items within it but forgot to
      actually free it.
      Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
      (cherry picked from commit ea13a759)
      4f169b0e