From mboxrd@z Thu Jan 1 00:00:00 1970 X-Msuck: nntp://news.gmane.org/gmane.linux.lib.musl.general/13707 Path: news.gmane.org!.POSTED.blaine.gmane.org!not-for-mail From: Alexey Izbyshev Newsgroups: gmane.linux.lib.musl.general Subject: __synccall: deadlock and reliance on racy /proc/self/task Date: Sun, 03 Feb 2019 00:40:39 +0300 Message-ID: <1cc54dbe2e4832d804184f33cda0bdd1@ispras.ru> Reply-To: musl@lists.openwall.com Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=_0348b6719652f0288bebb19ff25d6280" Injection-Info: blaine.gmane.org; posting-host="blaine.gmane.org:195.159.176.226"; logging-data="76242"; mail-complaints-to="usenet@blaine.gmane.org" User-Agent: Roundcube Webmail/1.1.2 To: musl@lists.openwall.com Original-X-From: musl-return-13723-gllmg-musl=m.gmane.org@lists.openwall.com Sat Feb 02 22:53:51 2019 Return-path: Envelope-to: gllmg-musl@m.gmane.org Original-Received: from mother.openwall.net ([195.42.179.200]) by blaine.gmane.org with smtp (Exim 4.89) (envelope-from ) id 1gq3EU-000Jjk-UK for gllmg-musl@m.gmane.org; Sat, 02 Feb 2019 22:53:51 +0100 Original-Received: (qmail 11945 invoked by uid 550); 2 Feb 2019 21:53:48 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Original-Received: (qmail 31986 invoked from network); 2 Feb 2019 21:40:50 -0000 X-Sender: izbyshev@ispras.ru Xref: news.gmane.org gmane.linux.lib.musl.general:13707 Archived-At: --=_0348b6719652f0288bebb19ff25d6280 Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset=US-ASCII; format=flowed Hello! I've discovered that setuid() deadlocks on a simple stress test (attached: test-setuid.c) that creates threads concurrently with setuid(). (Tested on 1.1.21 on x86_64, kernel 4.15.x and 4.4.x). The gdb output: (gdb) info thr Id Target Id Frame * 1 LWP 23555 "a.out" __synccall (func=func@entry=0x402a7d , ctx=ctx@entry=0x7fffea85b17c) at ../../musl/src/thread/synccall.c:144 2 LWP 23566 "a.out" __syscall () at ../../musl/src/internal/x86_64/syscall.s:13 (gdb) bt #0 __synccall (func=func@entry=0x402a7d , ctx=ctx@entry=0x7fffea85b17c) at ../../musl/src/thread/synccall.c:144 #1 0x0000000000402af9 in __setxid (nr=, id=, eid=, sid=) at ../../musl/src/unistd/setxid.c:33 #2 0x00000000004001c8 in main () (gdb) thr 2 (gdb) bt #0 __syscall () at ../../musl/src/internal/x86_64/syscall.s:13 #1 0x00000000004046b7 in __timedwait_cp (addr=addr@entry=0x7fe99023475c, val=val@entry=-1, clk=clk@entry=0, at=at@entry=0x0, priv=) at ../../musl/src/thread/__timedwait.c:31 #2 0x0000000000404591 in sem_timedwait (sem=sem@entry=0x7fe99023475c, at=at@entry=0x0) at ../../musl/src/thread/sem_timedwait.c:23 #3 0x00000000004044e1 in sem_wait (sem=sem@entry=0x7fe99023475c) at ../../musl/src/thread/sem_wait.c:5 #4 0x00000000004037ae in handler (sig=) at ../../musl/src/thread/synccall.c:43 #5 #6 __clone () at ../../musl/src/thread/x86_64/clone.s:17 #7 0x00000000004028ec in __pthread_create (res=0x7fe990234eb8, attrp=0x606260 , entry=0x400300 , arg=0x0) at ../../musl/src/thread/pthread_create.c:286 #8 0x0000000000400323 in thr () The main thread spins in __synccall with futex() always returning ETIMEDOUT (line 139) and "head" is NULL, while handler() in the second thread is blocked on sem_wait() (line 40). So it looks like handler() updated the linked list, but the main thread doesn't see the update. For some reason __synccall accesses the list without a barrier (line 120), though I don't see why one wouldn't be necessary for correct observability of head->next. However, I'm testing on x86_64, so acquire/release semantics works without barriers. I thought that a possible explanation is that handler() got blocked in a *previous* setuid() call, but we didn't notice its list entry at that time and then overwrote "head" with NULL on the current call to setuid(). This seems to be possible because of the following. 1) There is a "presignalling" phase, where we may send a signal to *any* thread. Moreover, the number of signals we sent may be *more* than the number of threads because some threads may exit while we're in the loop. As a result, SIGSYNCCALL may be pending after this loop. /* Initially send one signal per counted thread. But since we can't * synchronize with thread creation/exit here, there could be too * few signals. This initial signaling is just an optimization, not * part of the logic. */ for (i=libc.threads_minus_1; i; i--) __syscall(SYS_kill, pid, SIGSYNCCALL); 2) __synccall relies on /proc/self/task to get the list of *all* threads. However, since new threads can be created concurrently while we read /proc (if some threads were in pthread_thread() after __block_new_threads check when we set it to 1), I thought that /proc may miss some threads (that's actually why I started the whole exercise in the first place). So, if we miss a thread in (2) but it's created and signalled with a pending SIGSYNCCALL shortly after we exit /proc loop (but before we reset the signal handler), "handler()" will run in that thread concurrently with us, and we may miss its list entry if the timing is right. I've checked that if I remove the "presignalling" loop, the deadlock disappears (at least, I could run the test for several minutes without any problem). Of course, the larger problem remains: if we may miss some threads because of /proc, we may fail to call setuid() syscall in those threads. And that's indeed easily happens in my second test (attached: test-setuid-mismatch.c; expected to be run as a suid binary; note that I tested both with and without "presignalling"). Both tests run on glibc (2.27) without any problem. Would it be possible to fix __synccall in musl? Thanks! (Please CC me on answering, I'm not subscribed to the list). Alexey --=_0348b6719652f0288bebb19ff25d6280 Content-Transfer-Encoding: base64 Content-Type: text/x-c; name=test-setuid.c Content-Disposition: attachment; filename=test-setuid.c; size=656 I2RlZmluZSBfR05VX1NPVVJDRQojaW5jbHVkZSA8cHRocmVhZC5oPgojaW5jbHVkZSA8c3RkaW8u aD4KI2luY2x1ZGUgPHN0ZGxpYi5oPgojaW5jbHVkZSA8c3lzL3N5c2NhbGwuaD4KI2luY2x1ZGUg PHVuaXN0ZC5oPgoKI2RlZmluZSBOVU1fVEhSRUFEUyAxCgpzdGF0aWMgcHRocmVhZF9hdHRyX3Qg YXR0cjsKCnZvaWQgKnRocih2b2lkICphcmcpIHsKICBpZiAocHRocmVhZF9jcmVhdGUoJihwdGhy ZWFkX3QpezB9LCAmYXR0ciwgdGhyLCAwKSkKICAgIGFib3J0KCk7CiAgcmV0dXJuIDA7Cn0KCmlu dCBtYWluKHZvaWQpIHsKICBwdGhyZWFkX2F0dHJfaW5pdCgmYXR0cik7CiAgcHRocmVhZF9hdHRy X3NldGRldGFjaHN0YXRlKCZhdHRyLCBQVEhSRUFEX0NSRUFURV9ERVRBQ0hFRCk7CiAgZm9yIChp bnQgaSA9IDA7IGkgPCBOVU1fVEhSRUFEUzsgaSsrKQogICAgaWYgKHB0aHJlYWRfY3JlYXRlKCYo cHRocmVhZF90KXswfSwgJmF0dHIsIHRociwgMCkpCiAgICAgIGFib3J0KCk7CgogIGludCBldWlk ID0gZ2V0ZXVpZCgpOwogIGZvciAoaW50IGl0ID0gMDs7IGl0KyspIHsKICAgIHByaW50ZigiJWRc biIsIGl0KTsKICAgIGlmIChzZXRldWlkKGV1aWQpKSB7CiAgICAgIHBlcnJvcigic2V0ZXVpZCIp OwogICAgICBhYm9ydCgpOwogICAgfQogIH0KfQo= --=_0348b6719652f0288bebb19ff25d6280 Content-Transfer-Encoding: base64 Content-Type: text/x-c; name=test-setuid-mismatch.c Content-Disposition: attachment; filename=test-setuid-mismatch.c; size=1162 I2RlZmluZSBfR05VX1NPVVJDRQojaW5jbHVkZSA8cHRocmVhZC5oPgojaW5jbHVkZSA8c3RkaW8u aD4KI2luY2x1ZGUgPHN0ZGxpYi5oPgojaW5jbHVkZSA8c3lzL3N5c2NhbGwuaD4KI2luY2x1ZGUg PHVuaXN0ZC5oPgoKI2RlZmluZSBOVU1fVEhSRUFEUyAxCgpzdGF0aWMgcHRocmVhZF9hdHRyX3Qg YXR0cjsKc3RhdGljIHZvbGF0aWxlIGludCBldWlkID0gLTEsIGdlbiwgbnVtOwoKdm9pZCAqdGhy KHZvaWQgKmFyZykgewogIHdoaWxlIChldWlkID09IC0xKQogICAgOwogIGludCBjdXJfZ2VuID0g Z2VuOwogIGludCB0ZXVpZCA9IHN5c2NhbGwoU1lTX2dldGV1aWQpOwogIGlmICh0ZXVpZCAhPSBl dWlkKSB7CiAgICBwcmludGYoIm1pc21hdGNoOiAlZCAhPSAlZFxuIiwgdGV1aWQsIGV1aWQpOwog ICAgc2xlZXAoMTAwMDAwMCk7CiAgfQogIF9fc3luY19mZXRjaF9hbmRfYWRkKCZudW0sIDEpOwog IHdoaWxlIChjdXJfZ2VuID09IGdlbikKICAgIDsKCiAgaWYgKHB0aHJlYWRfY3JlYXRlKCYocHRo cmVhZF90KXswfSwgJmF0dHIsIHRociwgMCkpCiAgICBhYm9ydCgpOwogIHJldHVybiAwOwp9Cgpp bnQgbWFpbih2b2lkKSB7CiAgcHRocmVhZF9hdHRyX2luaXQoJmF0dHIpOwogIHB0aHJlYWRfYXR0 cl9zZXRkZXRhY2hzdGF0ZSgmYXR0ciwgUFRIUkVBRF9DUkVBVEVfREVUQUNIRUQpOwogIGZvciAo aW50IGkgPSAwOyBpIDwgTlVNX1RIUkVBRFM7IGkrKykKICAgIGlmIChwdGhyZWFkX2NyZWF0ZSgm KHB0aHJlYWRfdCl7MH0sICZhdHRyLCB0aHIsIDApKQogICAgICBhYm9ydCgpOwoKICBwcmludGYo InVpZCA9ICVkIGV1aWQgPSAlZFxuIiwgZ2V0dWlkKCksIGdldGV1aWQoKSk7CiAgaW50IHJ1aWQg PSBnZXR1aWQoKTsKICBpbnQgY3VyX2V1aWQgPSAwOwogIGZvciAoaW50IGl0ID0gMDs7IGl0Kysp IHsKICAgIHByaW50ZigiJWRcbiIsIGl0KTsKICAgIGN1cl9ldWlkID0gY3VyX2V1aWQgPyAwIDog cnVpZDsKICAgIGlmIChzZXRldWlkKGN1cl9ldWlkKSkgewogICAgICBwZXJyb3IoInNldGV1aWQi KTsKICAgICAgYWJvcnQoKTsKICAgIH0KICAgIG51bSA9IDA7CiAgICBldWlkID0gY3VyX2V1aWQ7 CiAgICB3aGlsZSAobnVtIDwgTlVNX1RIUkVBRFMpCiAgICAgIDsKICAgIGV1aWQgPSAtMTsKICAg IGdlbiA9IDEgLSBnZW47CiAgfQp9Cg== --=_0348b6719652f0288bebb19ff25d6280--