Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix potential deadlock with libc setgid/setuid #34

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

MarkusBauer
Copy link

I noticed that unit tests based on unshare deadlock from time to time. Apparently libc uses an internal mutex to synchronize setuid and setgid calls, which interferes with our forking. If we replace these calls with a raw syscall, we never take this lock. Given that we call either exec or exit shortly afterwards, the additional synchronization from libc shouldn't be necessary at all.

I could reproduce this bug in a small local project by running the 12 unit tests in a loop, each test spawns one child. Maybe 1 out of 50 attempts deadlocked. With the patch I no longer saw deadlocks.

For the reference: if the bug appeared, lldb reported the following backtrace for the blocking process (either with setgid or setuid):

(lldb) bt
* thread #1, name = 'sandbox::sandbo', stop reason = signal SIGSTOP
  * frame #0: 0x00007c81fa8994bc libc.so.6`setxid_mark_thread [inlined] futex_wait(private=0, expected=4294967294, futex_word=0x00007c81f9600cdc) at futex-internal.h:146:13
    frame #1: 0x00007c81fa8994aa libc.so.6`setxid_mark_thread [inlined] futex_wait_simple(private=0, expected=4294967294, futex_word=0x00007c81f9600cdc) at futex-internal.h:177:3
    frame #2: 0x00007c81fa8994aa libc.so.6`setxid_mark_thread(t=0x00007c81f96006c0, cmdp=<unavailable>) at nptl_setxid.c:105:7
    frame #3: 0x00007c81fa8996f5 libc.so.6`__nptl_setxid at nptl_setxid.c:195:7
    frame #4: 0x00007c81fa90eaee libc.so.6`__setuid [inlined] __setuid(uid=<unavailable>) at setuid.c:28:10
    frame #5: 0x00007c81fa90ead8 libc.so.6`__setuid(uid=<unavailable>) at setuid.c:23:1
    frame #6: 0x00005a9d6b49a5da engine_playground-5b269bc370ebf6a3`unshare::child::child_after_clone::_$u7b$$u7b$closure$u7d$$u7d$::h61991c3b9df4e187((null)=0x00007c81f91ff0ac) at child.rs:181:12
    frame #7: 0x00005a9d6b490f56 engine_playground-5b269bc370ebf6a3`core::option::Option$LT$T$GT$::map::h15c7a0c9e8e0c656(self=Option<&u32> @ 0x00007c81f91fbce8, f={closure_env#6} @ 0x00007c81f91fbcf8) at option.rs:1119:29
    frame #8: 0x00005a9d6b49c088 engine_playground-5b269bc370ebf6a3`unshare::child::child_after_clone::h597a0be69731d412(child=0x00007c81f91fc2d8) at child.rs:180:5
    frame #9: 0x00005a9d6b4a9ae6 engine_playground-5b269bc370ebf6a3`unshare::run::_$LT$impl$u20$unshare..Command$GT$::spawn_inner::_$u7b$$u7b$closure$u7d$$u7d$::h460a95f4c269566e at run.rs:280:13
    frame #10: 0x00005a9d6b4b2116 engine_playground-5b269bc370ebf6a3`_$LT$alloc..boxed..Box$LT$F$C$A$GT$$u20$as$u20$core..ops..function..FnMut$LT$Args$GT$$GT$::call_mut::hdd7b671fc289cce3(self=0x00007c81f91fad60, args=<unavailable>) at boxed.rs:2000:9
    frame #11: 0x00005a9d6b4b1907 engine_playground-5b269bc370ebf6a3`nix::sched::sched_linux_like::clone::callback::h89c0b15dbc89ddd8(data=0x00007c81f91fad60) at sched.rs:187:13
    frame #12: 0x00007c81fa929a34 libc.so.6`__clone at clone.S:100

Availability: If anyone wants to use this and other features before it's merged, check out my temporary fork. It includes #23 #27 #29 #33 #34 #35 and #36 with all conflicts resolved.
Add to your Cargo.toml: unshare = { git = "https://github.com/MarkusBauer/unshare.git" }

(cherry picked from commit 254ca68)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant