Bug 964359
| Summary: | virSetUIDGID can deadlock due to unsafe use of getpwuid_r | ||
|---|---|---|---|
| Product: | Red Hat Enterprise Linux 6 | Reporter: | Eric Blake <eblake> |
| Component: | libvirt | Assignee: | Eric Blake <eblake> |
| Status: | CLOSED ERRATA | QA Contact: | Virtualization Bugs <virt-bugs> |
| Severity: | high | Docs Contact: | |
| Priority: | high | ||
| Version: | 6.5 | CC: | acathrow, ahadas, ajia, berrange, bharrington, bili, clalancette, dallan, dyuan, eblake, gickowic, gsun, itamar, jdenemar, jforbes, jyang, kcleveng, laine, libvirt-maint, lsu, lyarwood, mjenner, shyu, veillard, ydu, zhwang |
| Target Milestone: | rc | ||
| Target Release: | --- | ||
| Hardware: | Unspecified | ||
| OS: | Unspecified | ||
| Whiteboard: | |||
| Fixed In Version: | libvirt-0.10.2-24.el6 | Doc Type: | Bug Fix |
| Doc Text: |
Cause: Libvirtd was setting up supplemental groups of child processes by making a call between fork and exec to a function (getpwuid_r) that could grab a mutex.
Consequence: If another thread already held the getpwuid_r mutex at the time libvirtd forked, the forked child would deadlock, which in turn hung libvirtd.
Fix: Code to compute the set of supplemental groups was refactored so that no mutex is required after fork.
Result: The deadlock scenario is no longer possible.
|
Story Points: | --- |
| Clone Of: | 964358 | Environment: | |
| Last Closed: | 2013-11-21 09:01:16 UTC | Type: | Bug |
| Regression: | --- | Mount Type: | --- |
| Documentation: | --- | CRM: | |
| Verified Versions: | Category: | --- | |
| oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
| Cloudforms Team: | --- | Target Upstream Version: | |
| Embargoed: | |||
| Bug Depends On: | |||
| Bug Blocks: | 1006509, 1008328 | ||
|
Description
Eric Blake
2013-05-18 03:12:27 UTC
getpwuid_r() has been called by libvirt between fork() and exec() since commit b7bd75c, which was in v0.8.5. (this code was in a different location, but was later moved into the general-purpose virSetUIDGID() function, and called by even more places. I haven't looked, but this code movement could make backporting a fix to older branches a bit tedious. I hit this again today while testing a fix for bug 903433; I'm surprised that no one else has seemed to hit this issue. Testing notes: in /etc/libvirt/qemu.conf, set 'user' and 'group' to qemu (not root), and restart libvirtd. As a sanity check, inspect /proc/nnn/status of a qemu process started by this libvirt; assuming default installation, it should have: Uid: 107 107 107 107 Gid: 107 107 107 107 ... Groups: 36 107 both pre- and post-patch (that is, this validates that we are correctly assigning supplemental group 36 (kvm) to the qemu process). I have not yet played with root-squash NFS in the context of this bug, but suspect it should be possible to write a test case where a file that is owned by the kvm group but NOT the qemu uid or gid should still be usable by a guest (ie. using root-squash NFS as proof that the cases where we have to fork a child to work around root-squash limitations still adds the right supplementary groups to mimic what the real qemu user can access). So far, I haven't found a RELIABLE way to trigger the deadlock pre-patch; I'm still messing around with running libvirtd under gdb, and trying to figure out where to put a breakpoint in glibc's getpwuid_r, followed by appropriate steps to cause one thread of libvirtd to do a uid lookup; the idea is that once the breakpoint is hit, I then let another thread run that forks and does a uid lookup in the child, to reliably prove that the pre-patch deadlock exists. If I come up with a reproducible formula (rather than just starting and stopping multiple domains in parallel while running a hook script in /etc/libvirt/hooks/qemu), I'll add it as another note. v1 of potential upstream patch has been posted upstream; it will need a v2 depending on what gnulib gives us, but I plan on doing a scratch build soon. https://www.redhat.com/archives/libvir-list/2013-May/msg01600.html Upstream is now in place with this series of commits: bfc183c 29fe5d7 fdb3bde ee777e9 75c1256 c1983ba and optionally a gnulib submodule update (I'll have to figure out how to avoid backporting that, as it messes with the build process) An additional issue was identified upstream that affects Debian's choice of using a non-primary group for running qemu, while leaving the files only owned by the primary group: https://www.redhat.com/archives/libvir-list/2013-August/msg00209.html But since RHEL doesn't use Debian's uid setup, it should not matter if that patch is not backported to RHEL. HI Eric, I'm trying to verify this bug with latest libvirt, but not clear about how to trigger this bug, could u give some suggestion so i can verify it? Thanks! Positive verification is easy - both pre-patch and post-patch, run with /etc/libvirt/qemu.conf set to a non-root user that belongs to more than one group, and ensure that qemu child processes are associated with all of the supplementary groups of that user rather than just the primary group. Negative verification (triggering the pre-patch race) is much harder. I happened to hit the race while running a hook script associated with domain startup, where all the hook script did was sleep 2 seconds (http://libvirt.org/hooks.html), then attempting to start multiple domains in parallel. That was enough to make multiple threads of libvirt be probing the group database while other threads were trying to fork; but even then, I only saw the deadlock twice across hundreds of attempts. I think you will only get reliable tests if you run libvirtd under gdb and put a breakpoint inside glibc's mutex within getpwuid_r, although I have not attempted to figure out how to set up such a test. I'm also trying to work on an LD_PRELOAD addition to libvirt/tests that could serve a similar purpose, but don't have that finished yet. Hi, Eric Test with libvirt-0.10.2-22.el6.x86_64, the qemu processes groups always 0, this maybe a problem, details as following: Positive test with libvirt-0.10.2-21.el6.x86_64 1. Set user/group to "qemu", restart libvirt, then start a guest 2. Check the qemu child processes: # ps -eaL|grep qemu 10695 10695 ? 00:00:00 qemu-kvm 10695 10697 ? 00:00:00 qemu-kvm 10695 10698 ? 00:00:00 qemu-kvm # cat /proc/10698/status ... 0 Uid: 107 107 107 107 Gid: 107 107 107 107 Utrace: 0 FDSize: 64 Groups: 36 107 # cat /proc/10697/status ... 0 Uid: 107 107 107 107 Gid: 107 107 107 107 Utrace: 0 FDSize: 64 Groups: 36 107 # cat /proc/10695/status ... Uid: 107 107 107 107 Gid: 107 107 107 107 Utrace: 0 FDSize: 64 Groups: 36 107 ------------ Then update libvirt to libvirt-0.10.2-22.el6.x86_64, repeat step1&2: # ps -eaL|grep qemu 11006 11006 ? 00:00:00 qemu-kvm 11006 11008 ? 00:00:00 qemu-kvm 11006 11009 ? 00:00:00 qemu-kvm # cat /proc/11009/status ... 0 Uid: 107 107 107 107 Gid: 107 107 107 107 Utrace: 0 FDSize: 64 Groups: 0 # cat /proc/11008/status ... Uid: 107 107 107 107 Gid: 107 107 107 107 Utrace: 0 FDSize: 64 Groups: 0 # cat /proc/11006/status ... Uid: 107 107 107 107 Gid: 107 107 107 107 Utrace: 0 FDSize: 64 Groups: 0 (In reply to yanbing du from comment #14) > Hi, Eric > Test with libvirt-0.10.2-22.el6.x86_64, the qemu processes groups always 0, > this maybe a problem, details as following: Yes, it IS a problem. Reopening this bug. > ------------ > Then update libvirt to libvirt-0.10.2-22.el6.x86_64, repeat step1&2: > # ps -eaL|grep qemu > 11006 11006 ? 00:00:00 qemu-kvm > 11006 11008 ? 00:00:00 qemu-kvm > 11006 11009 ? 00:00:00 qemu-kvm > > # cat /proc/11009/status > ... 0 > Uid: 107 107 107 107 > Gid: 107 107 107 107 > Utrace: 0 > FDSize: 64 > Groups: 0 That should not be happening. I'm investigating... *** Bug 997350 has been marked as a duplicate of this bug. *** Now upstream as of commit 745aa55fb, and v2 is in POST on rhvirt-archives (would post the URL, but rhvirt-archives appears to be stuck since the 27th...) (In reply to Eric Blake from comment #20) > Now upstream as of commit 745aa55fb, and v2 is in POST on rhvirt-archives > (would post the URL, but rhvirt-archives appears to be stuck since the > 27th...) here's the message id to look for, if the archives ever get unstuck <1377804152-29348-1-git-send-email-eblake> *** Bug 1002115 has been marked as a duplicate of this bug. *** *** Bug 1003488 has been marked as a duplicate of this bug. *** Hi, Eric I just try to verify this bug with libvirt-0.10.2-24.el6.x86_64, the following was my verify steps, please help check that whether was it enough to verify this bug, thanks. Reproduce bug with libvirt-0.10.2-22.el6.x86_64 1. create new group "abc" and add the user "qemu" to it # id qemu uid=107(qemu) gid=107(qemu) groups=107(qemu),36(kvm),500(abc) 2. Set user/group to "qemu" in qemu.conf, restart libvirt, then start a guest 3. Check the qemu child processes: # ps -eaL|grep qemu 29517 29517 ? 00:00:03 qemu-kvm 29517 29543 ? 00:00:40 qemu-kvm 29517 29544 ? 00:00:00 qemu-kvm 29517 30038 ? 00:00:00 qemu-kvm # cat /proc/29544/status Name: qemu-kvm State: S (sleeping) Tgid: 29517 Pid: 29544 PPid: 1 TracerPid: 0 Uid: 107 107 107 107 Gid: 107 107 107 107 Utrace: 0 FDSize: 64 Groups: 0 492 Then update libvirt to libvirt-0.10.2-24.el6.x86_64, repeat step2&3, qemu child processes are associated with all of the supplementary groups of that user : # ps -eaL|grep qemu 30736 30736 ? 00:00:00 qemu-kvm 30736 30758 ? 00:00:00 qemu-kvm 30736 30759 ? 00:00:02 qemu-kvm 30736 30760 ? 00:00:00 qemu-kvm # cat /proc/30760/status Name: qemu-kvm State: S (sleeping) Tgid: 30736 Pid: 30760 PPid: 1 TracerPid: 0 Uid: 107 107 107 107 Gid: 107 107 107 107 Utrace: 0 FDSize: 64 Groups: 36 107 500 # cat /proc/30759/status Name: qemu-kvm State: S (sleeping) Tgid: 30736 Pid: 30759 PPid: 1 TracerPid: 0 Uid: 107 107 107 107 Gid: 107 107 107 107 Utrace: 0 FDSize: 64 Groups: 36 107 500 *** Bug 1005781 has been marked as a duplicate of this bug. *** *** Bug 999230 has been marked as a duplicate of this bug. *** Since the problem described in this bug report should be resolved in a recent advisory, it has been closed with a resolution of ERRATA. For information on the advisory, and where to find the updated files, follow the link below. If the solution does not work for you, open a new bug report. http://rhn.redhat.com/errata/RHBA-2013-1581.html |