[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Tue, 30 Jun 2020 00:43:23 -0400
From: Rich Felker <dalias@...c.org>
To: musl@...ts.openwall.com
Subject: Re: Potential deadlock in pthread_kill()
On Mon, Jun 29, 2020 at 09:19:08PM -0700, Hydro Flask wrote:
> Hello all,
>
> Noticed something while reading some code today. pthread_kill() is
> specified by POSIX to be async signal safe but I noticed that in
> musl's implementation if a signal occurs while the "killlock" is
> held and the signal handler calls pthread_kill() on the same target
> thread, a deadlock will occur. Is this intentional?
>
> int pthread_kill(pthread_t t, int sig)
> {
> int r;
> LOCK(t->killlock);
> r = t->tid ? -__syscall(SYS_tkill, t->tid, sig)
> : (sig+0U >= _NSIG ? EINVAL : 0);
> UNLOCK(t->killlock);
> return r;
> }
>
> Thank you for your attention.
Thanks. It looks like this case was overlooked in the pthread_cancel
fix that was commit 060ed9367337cbbd59a9e5e638a1c2f460192f25. The
possibility of blocking signals was even mentioned there but deemed
unnecessary.
A simpler/lighter fix might be, before the lock,
if (t==__pthread_self())
return -__syscall(SYS_tkill, t->tid, sig);
since no lock is needed if targeting self; t->tid is necessarily valid
in that case.
One concern I just had was interaction with fork (also a nasty AS-safe
function), but if fork is called from a signal handler during
pthread_kill, it's no different from the signal handler running just
before pthread_kill; the result is targeting an invalid (in the child)
pthread_t, which thereby has undefined behavior. So, while ugly, I
think this is ok.
Note that raise() *does* need to block signals here, because there is
no explicit pthread_t argument and thus the interaction with fork is
well-defined.
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.
