From mboxrd@z Thu Jan 1 00:00:00 1970 X-Msuck: nntp://news.gmane.org/gmane.linux.lib.musl.general/6740 Path: news.gmane.org!not-for-mail From: Rich Felker Newsgroups: gmane.linux.lib.musl.general Subject: Fixing multithreaded set*id() AS-safety Date: Fri, 19 Dec 2014 22:39:18 -0500 Message-ID: <20141220033918.GA3273@brightrain.aerifal.cx> Reply-To: musl@lists.openwall.com NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-Trace: ger.gmane.org 1419046781 31674 80.91.229.3 (20 Dec 2014 03:39:41 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Sat, 20 Dec 2014 03:39:41 +0000 (UTC) To: musl@lists.openwall.com Original-X-From: musl-return-6753-gllmg-musl=m.gmane.org@lists.openwall.com Sat Dec 20 04:39:35 2014 Return-path: Envelope-to: gllmg-musl@m.gmane.org Original-Received: from mother.openwall.net ([195.42.179.200]) by plane.gmane.org with smtp (Exim 4.69) (envelope-from ) id 1Y2AtC-0008U1-9p for gllmg-musl@m.gmane.org; Sat, 20 Dec 2014 04:39:34 +0100 Original-Received: (qmail 7925 invoked by uid 550); 20 Dec 2014 03:39:32 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: Original-Received: (qmail 7914 invoked from network); 20 Dec 2014 03:39:31 -0000 Content-Disposition: inline User-Agent: Mutt/1.5.21 (2010-09-15) Original-Sender: Rich Felker Xref: news.gmane.org gmane.linux.lib.musl.general:6740 Archived-At: Currently multithreaded set*id() functions fail to be AS-safe, despite some of them being required to be AS-safe, due to the need for locking. If they're called from a signal handler that interrupted pthread_create or dlopen (or another set*id() function during a tiny race window), attempting to obtain the lock (lock_ptc.c functions) will deadlock. There are probably other ugly issues too. I see two possible strategies for fixing this: Strategy 1: Block signals during all uses of this lock to preclude the possibility of a thread trying to take a lock it already holds. This would mean adding signal blocking to dlopen (irrelevant to performance) and pthread_create (a potentially serious cost). This feels wrong since the common uses of this lock do not require AS-safety; only the uncommon one (set*id() in a multithreaded program) does. On the other hand, pthread_create already has to manipulate the signal mask in some situations (priority attributes and when pthread_create is called from a cancelled thread), and simply making the signal masking and restoration unconditional would simplify the code at the expense of some performance cost in the common cases. I think some additional synchronization needs to be added to pthread_exit for this strategy too, since the current logic seems to have races where set*id() might deadlock due to miscounting thread when racing with pthread_exit. Strategy 2: Use the kernel, via /proc/self/task, to enumerate threads. Multithreaded set*id() would have to fail on systems with /proc not mounted or non-Linux systems that don't emulate /proc/self/task. The benefit of this approach is that we can eliminate basically all need for synchronization between important, performance-critical operations (pthread_create and pthread_exist) and the ugly set*id() mess. The only requirement seems to be ensuring that unboundedly many new threads can't be created during set*id() (imagine a chain of threads creating new threads and terminating, which reading /proc/self/task might never catch up with), but this can easily be precluded with a global flag that blocks forward progress in pthread_create (via a futex wait) if it's set, and no synchronization would be required to access the flag since it only needs to block unboundedly many new threads. There may also be some complicated logic for handling threads that exit after they're signaled but before the signal is delivered, but I think it's doable. Neither approach is really attractive. Strategy 1 feels less hackish and more elegant (it actually makes the pthread_create code more elegant than it is now by having fewer special cases), but the cost feels wasteful. Strategy 2 is ugly but has the ugliness isolated to synccall.c (the internals for set*id()) where it doesn't interact with other parts of the code in any significant way. Any opinions on which way we should go? I'll probably hold off to do any of this until the next release cycle (or maybe even later), but I want to go ahead and start thinking about and discussing it. Rich