mailing list of musl libc
 help / color / mirror / code / Atom feed
From: Rich Felker <dalias@libc.org>
To: musl@lists.openwall.com
Subject: Fixing multithreaded set*id() AS-safety
Date: Fri, 19 Dec 2014 22:39:18 -0500	[thread overview]
Message-ID: <20141220033918.GA3273@brightrain.aerifal.cx> (raw)

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


             reply	other threads:[~2014-12-20  3:39 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-12-20  3:39 Rich Felker [this message]
2014-12-20  8:48 ` Jens Gustedt
2014-12-20 19:24   ` Rich Felker
2015-01-10  5:33 ` Rich Felker
2015-01-10 22:52   ` stephen Turner
2015-01-11  4:07     ` Rich Felker
2015-01-11 16:31       ` stephen Turner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20141220033918.GA3273@brightrain.aerifal.cx \
    --to=dalias@libc.org \
    --cc=musl@lists.openwall.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.vuxu.org/mirror/musl/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).