[libvirt] [PATCH 3/7] util: make virSetUIDGID async-signal-safe

[Cole Robinson]

On 07/23/2013 11:03 AM, Eric Blake wrote:
> 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: Eric Blake <eblake at redhat.com>
> (cherry picked from commit ee777e994927ed5f2d427fbc5a53cbe8b5969bda)
> 
> Conflicts:
> 	src/lxc/lxc_container.c - did not use setUIDGID before 1.1.0
> 	src/util/virutil.c - oom handling changes not backported; no virSetUIDGIDWithCaps
> 	src/util/virfile.c - functions still lived in virutil.c this far back
> 	configure.ac - context with previous commit
> 	src/util/command.c - no UID/GID handling in vircommand.c...
> 	src/storage/storage_backend.c - ...so do it in the one hook user instead

ACK

- Cole