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@xxxxxxxxxx> > (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 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list