[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-ID: <20141220033918.GA3273@brightrain.aerifal.cx>
Date: Fri, 19 Dec 2014 22:39:18 -0500
From: Rich Felker <dalias@...c.org>
To: musl@...ts.openwall.com
Subject: Fixing multithreaded set*id() AS-safety
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
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
