1. 30 7月, 2013 3 次提交
  2. 23 7月, 2013 5 次提交
    • E
      security: fix deadlock with prefork · 65985101
      Eric Blake 提交于
      https://bugzilla.redhat.com/show_bug.cgi?id=964358
      
      Attempts to start a domain with both SELinux and DAC security
      modules loaded will deadlock; latent problem introduced in commit
      fdb3bde3 and exposed in commit 29fe5d74.  Basically, when recursing
      into the security manager for other driver's prefork, we have to
      undo the asymmetric lock taken at the manager level.
      
      Reported by Jiri Denemark, with diagnosis help from Dan Berrange.
      
      * src/security/security_stack.c (virSecurityStackPreFork): Undo
      extra lock grabbed during recursion.
      Signed-off-by: NEric Blake <eblake@redhat.com>
      (cherry picked from commit bfc183c1)
      65985101
    • E
      security_dac: compute supplemental groups before fork · 00c2003e
      Eric Blake 提交于
      https://bugzilla.redhat.com/show_bug.cgi?id=964358
      
      Commit 75c12564 states that virGetGroupList must not be called
      between fork and exec, then commit ee777e99 promptly violated
      that for lxc's use of virSecurityManagerSetProcessLabel.  Hoist
      the supplemental group detection to the time that the security
      manager needs to fork.  Qemu is safe, as it uses
      virSecurityManagerSetChildProcessLabel which in turn uses
      virCommand to determine supplemental groups.
      
      This does not fix the fact that virSecurityManagerSetProcessLabel
      calls virSecurityDACParseIds calls parseIds which eventually
      calls getpwnam_r, which also violates fork/exec async-signal-safe
      safety rules, but so far no one has complained of hitting
      deadlock in that case.
      
      * src/security/security_dac.c (_virSecurityDACData): Track groups
      in private data.
      (virSecurityDACPreFork): New function, to set them.
      (virSecurityDACClose): Clean up new fields.
      (virSecurityDACGetIds): Alter signature.
      (virSecurityDACSetSecurityHostdevLabelHelper)
      (virSecurityDACSetChardevLabel, virSecurityDACSetProcessLabel)
      (virSecurityDACSetChildProcessLabel): Update callers.
      Signed-off-by: NEric Blake <eblake@redhat.com>
      (cherry picked from commit 29fe5d74)
      
      Conflicts:
      	src/security/security_dac.c - virSecurityDACSetSecurityUSBLabel needed similar treatment
      00c2003e
    • E
      security: framework for driver PreFork handler · 689d0146
      Eric Blake 提交于
      https://bugzilla.redhat.com/show_bug.cgi?id=964358
      
      A future patch wants the DAC security manager to be able to safely
      get the supplemental group list for a given uid, but at the time
      of a fork rather than during initialization so as to pick up on
      live changes to the system's group database.  This patch adds the
      framework, including the possibility of a pre-fork callback
      failing.
      
      For now, any driver that implements a prefork callback must be
      robust against the possibility of being part of a security stack
      where a later element in the chain fails prefork.  This means
      that drivers cannot do any action that requires a call to postfork
      for proper cleanup (no grabbing a mutex, for example).  If this
      is too prohibitive in the future, we would have to switch to a
      transactioning sequence, where each driver has (up to) 3 callbacks:
      PreForkPrepare, PreForkCommit, and PreForkAbort, to either clean
      up or commit changes made during prepare.
      
      * src/security/security_driver.h (virSecurityDriverPreFork): New
      callback.
      * src/security/security_manager.h (virSecurityManagerPreFork):
      Change signature.
      * src/security/security_manager.c (virSecurityManagerPreFork):
      Optionally call into driver, and allow returning failure.
      * src/security/security_stack.c (virSecurityDriverStack):
      Wrap the handler for the stack driver.
      * src/qemu/qemu_process.c (qemuProcessStart): Adjust caller.
      Signed-off-by: NEric Blake <eblake@redhat.com>
      (cherry picked from commit fdb3bde3)
      689d0146
    • E
      util: make virSetUIDGID async-signal-safe · 7d24a0a2
      Eric Blake 提交于
      https://bugzilla.redhat.com/show_bug.cgi?id=964358
      
      POSIX states that multi-threaded apps should not use functions
      that are not async-signal-safe between fork and exec, yet we
      were using getpwuid_r and initgroups.  Although rare, it is
      possible to hit deadlock in the child, when it tries to grab
      a mutex that was already held by another thread in the parent.
      I actually hit this deadlock when testing multiple domains
      being started in parallel with a command hook, with the following
      backtrace in the child:
      
       Thread 1 (Thread 0x7fd56bbf2700 (LWP 3212)):
       #0  __lll_lock_wait ()
           at ../nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.S:136
       #1  0x00007fd5761e7388 in _L_lock_854 () from /lib64/libpthread.so.0
       #2  0x00007fd5761e7257 in __pthread_mutex_lock (mutex=0x7fd56be00360)
           at pthread_mutex_lock.c:61
       #3  0x00007fd56bbf9fc5 in _nss_files_getpwuid_r (uid=0, result=0x7fd56bbf0c70,
           buffer=0x7fd55c2a65f0 "", buflen=1024, errnop=0x7fd56bbf25b8)
           at nss_files/files-pwd.c:40
       #4  0x00007fd575aeff1d in __getpwuid_r (uid=0, resbuf=0x7fd56bbf0c70,
           buffer=0x7fd55c2a65f0 "", buflen=1024, result=0x7fd56bbf0cb0)
           at ../nss/getXXbyYY_r.c:253
       #5  0x00007fd578aebafc in virSetUIDGID (uid=0, gid=0) at util/virutil.c:1031
       #6  0x00007fd578aebf43 in virSetUIDGIDWithCaps (uid=0, gid=0, capBits=0,
           clearExistingCaps=true) at util/virutil.c:1388
       #7  0x00007fd578a9a20b in virExec (cmd=0x7fd55c231f10) at util/vircommand.c:654
       #8  0x00007fd578a9dfa2 in virCommandRunAsync (cmd=0x7fd55c231f10, pid=0x0)
           at util/vircommand.c:2247
       #9  0x00007fd578a9d74e in virCommandRun (cmd=0x7fd55c231f10, exitstatus=0x0)
           at util/vircommand.c:2100
       #10 0x00007fd56326fde5 in qemuProcessStart (conn=0x7fd53c000df0,
           driver=0x7fd55c0dc4f0, vm=0x7fd54800b100, migrateFrom=0x0, stdin_fd=-1,
           stdin_path=0x0, snapshot=0x0, vmop=VIR_NETDEV_VPORT_PROFILE_OP_CREATE,
           flags=1) at qemu/qemu_process.c:3694
       ...
      
      The solution is to split the work of getpwuid_r/initgroups into the
      unsafe portions (getgrouplist, called pre-fork) and safe portions
      (setgroups, called post-fork).
      
      * src/util/virutil.h (virSetUIDGID, virSetUIDGIDWithCaps): Adjust
      signature.
      * src/util/virutil.c (virSetUIDGID): Add parameters.
      (virSetUIDGIDWithCaps): Adjust clients.
      * src/util/vircommand.c (virExec): Likewise.
      * src/util/virfile.c (virFileAccessibleAs, virFileOpenForked)
      (virDirCreate): Likewise.
      * src/security/security_dac.c (virSecurityDACSetProcessLabel):
      Likewise.
      * src/lxc/lxc_container.c (lxcContainerSetID): Likewise.
      * configure.ac (AC_CHECK_FUNCS_ONCE): Check for setgroups, not
      initgroups.
      Signed-off-by: NEric Blake <eblake@redhat.com>
      (cherry picked from commit ee777e99)
      
      Conflicts:
      	src/lxc/lxc_container.c - did not use setUIDGID before 1.1.0
      	src/util/virutil.c - oom handling changes not backported
      	src/util/virfile.c - functions still lived in virutil.c this far back
      	configure.ac - context with previous commit
      7d24a0a2
    • E
      util: add virGetGroupList · fcdaa3df
      Eric Blake 提交于
      https://bugzilla.redhat.com/show_bug.cgi?id=964358
      
      Since neither getpwuid_r() nor initgroups() are safe to call in
      between fork and exec (they obtain a mutex, but if some other
      thread in the parent also held the mutex at the time of the fork,
      the child will deadlock), we have to split out the functionality
      that is unsafe.  At least glibc's initgroups() uses getgrouplist
      under the hood, so the ideal split is to expose getgrouplist for
      use before a fork.  Gnulib already gives us a nice wrapper via
      mgetgroups; we wrap it once more to look up by uid instead of name.
      
      * bootstrap.conf (gnulib_modules): Add mgetgroups.
      * src/util/virutil.h (virGetGroupList): New declaration.
      * src/util/virutil.c (virGetGroupList): New function.
      * src/libvirt_private.syms (virutil.h): Export it.
      Signed-off-by: NEric Blake <eblake@redhat.com>
      (cherry picked from commit 75c12564)
      
      Conflicts:
      	bootstrap.conf - not updating gnulib submodule...
      	configure.ac - ...so checking for getgrouplist by hand...
      	src/util/virutil.c - ...and copying only the getgrouplist implementation rather than calling the gnulib function
      fcdaa3df
  3. 20 7月, 2013 1 次提交
    • E
      util: improve user lookup helper · 3a2d5e7e
      Eric Blake 提交于
      https://bugzilla.redhat.com/show_bug.cgi?id=964358
      
      A future patch needs to look up pw_gid; but it is wasteful
      to crawl through getpwuid_r twice for two separate pieces
      of information, and annoying to copy that much boilerplate
      code for doing the crawl.  The current internal-only
      virGetUserEnt is also a rather awkward interface; it's easier
      to just design it to let callers request multiple pieces of
      data as needed from one traversal.
      
      And while at it, I noticed that virGetXDGDirectory could deref
      NULL if the getpwuid_r lookup fails.
      
      * src/util/virutil.c (virGetUserEnt): Alter signature.
      (virGetUserDirectory, virGetXDGDirectory, virGetUserName): Adjust
      callers.
      Signed-off-by: NEric Blake <eblake@redhat.com>
      (cherry picked from commit c1983ba4)
      
      Conflicts:
      	src/util/virutil.c - oom reporting/strdup changes not backported
      3a2d5e7e
  4. 12 7月, 2013 27 次提交
  5. 11 7月, 2013 2 次提交
  6. 01 7月, 2013 2 次提交