[libvirt] [PATCH 3/7] util: make virSetUIDGID async-signal-safe
Cole Robinson
crobinso at redhat.com
Thu Jul 25 22:31:22 UTC 2013
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
More information about the libvir-list
mailing list