mailing list of musl libc
 help / color / mirror / code / Atom feed
* [musl] Potential deadlock in pthread_kill()
@ 2020-06-30  4:19 Hydro Flask
  2020-06-30  4:43 ` Rich Felker
  0 siblings, 1 reply; 13+ messages in thread
From: Hydro Flask @ 2020-06-30  4:19 UTC (permalink / raw)
  To: musl

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.

Hydro

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [musl] Potential deadlock in pthread_kill()
  2020-06-30  4:19 [musl] Potential deadlock in pthread_kill() Hydro Flask
@ 2020-06-30  4:43 ` Rich Felker
  2020-06-30  6:19   ` Hydro Flask
  0 siblings, 1 reply; 13+ messages in thread
From: Rich Felker @ 2020-06-30  4:43 UTC (permalink / raw)
  To: musl

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [musl] Potential deadlock in pthread_kill()
  2020-06-30  4:43 ` Rich Felker
@ 2020-06-30  6:19   ` Hydro Flask
  2020-06-30  9:26     ` Rich Felker
  0 siblings, 1 reply; 13+ messages in thread
From: Hydro Flask @ 2020-06-30  6:19 UTC (permalink / raw)
  To: musl; +Cc: Rich Felker

>>         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.

Just to be clear, this doesn't only occur when calling pthread_kill() 
and using pthread_self() as the target, it can be any target thread, as 
long as it's the same target thread is used in the signal handler and in 
the synchronous context.

Looking at the commit message you references, I think the only fix for 
all cases is to block signals before taking the killlock. If there is a 
way to avoid the killlock altogether that would also fix it. Thanks 
again for confirming the issue.

Hydro

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [musl] Potential deadlock in pthread_kill()
  2020-06-30  6:19   ` Hydro Flask
@ 2020-06-30  9:26     ` Rich Felker
  2020-06-30 14:58       ` Markus Wichmann
  0 siblings, 1 reply; 13+ messages in thread
From: Rich Felker @ 2020-06-30  9:26 UTC (permalink / raw)
  To: musl

On Mon, Jun 29, 2020 at 11:19:39PM -0700, Hydro Flask wrote:
> >>        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.
> 
> Just to be clear, this doesn't only occur when calling
> pthread_kill() and using pthread_self() as the target, it can be any
> target thread, as long as it's the same target thread is used in the
> signal handler and in the synchronous context.

How so? If the target is different, the rest of the pthread_kill,
including the unlock, will proceed concurrently with the signal
handler. However you may be able to construct mutual-signaling
deadlock cases.

> Looking at the commit message you references, I think the only fix
> for all cases is to block signals before taking the killlock. If

This might be the case.

> there is a way to avoid the killlock altogether that would also fix
> it. Thanks again for confirming the issue.

It can't be removed without replacing it with something else to
synchronize against possible thread exit.

Rich

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [musl] Potential deadlock in pthread_kill()
  2020-06-30  9:26     ` Rich Felker
@ 2020-06-30 14:58       ` Markus Wichmann
  2020-06-30 16:28         ` Hydro Flask
  0 siblings, 1 reply; 13+ messages in thread
From: Markus Wichmann @ 2020-06-30 14:58 UTC (permalink / raw)
  To: musl

On Tue, Jun 30, 2020 at 05:26:46AM -0400, Rich Felker wrote:
> On Mon, Jun 29, 2020 at 11:19:39PM -0700, Hydro Flask wrote:
> >
> > Just to be clear, this doesn't only occur when calling
> > pthread_kill() and using pthread_self() as the target, it can be any
> > target thread, as long as it's the same target thread is used in the
> > signal handler and in the synchronous context.
>
> How so? If the target is different, the rest of the pthread_kill,
> including the unlock, will proceed concurrently with the signal
> handler. However you may be able to construct mutual-signaling
> deadlock cases.
>

Thread A calls pthread_kill(thread_c, ...). Thread B calls
(concurrently) pthread_kill(thread_a, ...). Thread B's signal arrives
while thread A holds the killlock. Signal handler calls
pthread_kill(thread_c, ...).

No mutual signalling. Doesn't even need thread B to be calling
pthread_kill, could be an external signal that just happens to arrive at
that time and is taken by thread A by (bad) luck.

Ciao,
Markus

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [musl] Potential deadlock in pthread_kill()
  2020-06-30 14:58       ` Markus Wichmann
@ 2020-06-30 16:28         ` Hydro Flask
  2020-06-30 19:28           ` Dmitry Samersoff
  0 siblings, 1 reply; 13+ messages in thread
From: Hydro Flask @ 2020-06-30 16:28 UTC (permalink / raw)
  To: musl; +Cc: Markus Wichmann

On 2020-06-30 07:58, Markus Wichmann wrote:
> On Tue, Jun 30, 2020 at 05:26:46AM -0400, Rich Felker wrote:
>> On Mon, Jun 29, 2020 at 11:19:39PM -0700, Hydro Flask wrote:
>> >
>> > Just to be clear, this doesn't only occur when calling
>> > pthread_kill() and using pthread_self() as the target, it can be any
>> > target thread, as long as it's the same target thread is used in the
>> > signal handler and in the synchronous context.
>> 
>> How so? If the target is different, the rest of the pthread_kill,
>> including the unlock, will proceed concurrently with the signal
>> handler. However you may be able to construct mutual-signaling
>> deadlock cases.
>> 
> 
> Thread A calls pthread_kill(thread_c, ...). Thread B calls
> (concurrently) pthread_kill(thread_a, ...). Thread B's signal arrives
> while thread A holds the killlock. Signal handler calls
> pthread_kill(thread_c, ...).

You can trigger it much more simply than that. Doesn't require multiple 
threads:

1. pthread_kill(th)
2. LOCK(killlock)
3. <signal arrives after lock>
4. signal handler: pthread_kill(th)
5. signal handler: LOCK(killlock)

In the preceding example "th" can be any pthread_t value, the only 
requirement is that it's the same pthread_t in both the synchronous and 
signal handler context.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [musl] Potential deadlock in pthread_kill()
  2020-06-30 16:28         ` Hydro Flask
@ 2020-06-30 19:28           ` Dmitry Samersoff
  2020-06-30 19:54             ` Rich Felker
  0 siblings, 1 reply; 13+ messages in thread
From: Dmitry Samersoff @ 2020-06-30 19:28 UTC (permalink / raw)
  To: musl, Hydro Flask; +Cc: Markus Wichmann

Hello all,

Does it make sense to trylock and immediately return ESRCH if 
pthread_kill is already in progress?

-Dmitry

On 30.06.2020 19:28, Hydro Flask wrote:
> On 2020-06-30 07:58, Markus Wichmann wrote:
>> On Tue, Jun 30, 2020 at 05:26:46AM -0400, Rich Felker wrote:
>>> On Mon, Jun 29, 2020 at 11:19:39PM -0700, Hydro Flask wrote:
>>> >
>>> > Just to be clear, this doesn't only occur when calling
>>> > pthread_kill() and using pthread_self() as the target, it can be any
>>> > target thread, as long as it's the same target thread is used in the
>>> > signal handler and in the synchronous context.
>>>
>>> How so? If the target is different, the rest of the pthread_kill,
>>> including the unlock, will proceed concurrently with the signal
>>> handler. However you may be able to construct mutual-signaling
>>> deadlock cases.
>>>
>>
>> Thread A calls pthread_kill(thread_c, ...). Thread B calls
>> (concurrently) pthread_kill(thread_a, ...). Thread B's signal arrives
>> while thread A holds the killlock. Signal handler calls
>> pthread_kill(thread_c, ...).
> 
> You can trigger it much more simply than that. Doesn't require multiple 
> threads:
> 
> 1. pthread_kill(th)
> 2. LOCK(killlock)
> 3. <signal arrives after lock>
> 4. signal handler: pthread_kill(th)
> 5. signal handler: LOCK(killlock)
> 
> In the preceding example "th" can be any pthread_t value, the only 
> requirement is that it's the same pthread_t in both the synchronous and 
> signal handler context.


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [musl] Potential deadlock in pthread_kill()
  2020-06-30 19:28           ` Dmitry Samersoff
@ 2020-06-30 19:54             ` Rich Felker
  2020-06-30 21:00               ` Hydro Flask
  0 siblings, 1 reply; 13+ messages in thread
From: Rich Felker @ 2020-06-30 19:54 UTC (permalink / raw)
  To: musl

On Tue, Jun 30, 2020 at 10:28:50PM +0300, Dmitry Samersoff wrote:
> Hello all,
> 
> Does it make sense to trylock and immediately return ESRCH if
> pthread_kill is already in progress?

No, ESRCH is not a valid result for this condition. Moreover killlock
being taken does not tell you that pthread_kill is already in progress
targeting the same thread, just that something that needs the kernel
tid to be valid (kill, cancel, or scheduling changes) or that needs to
change its validity (exiting) is in progress.

Note that for fixing this issue, it won't suffice just to make
pthread_kill block signals. The other places that use the killlock
also need to block signals, to make the lock fully AS-safe.

Rich

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [musl] Potential deadlock in pthread_kill()
  2020-06-30 19:54             ` Rich Felker
@ 2020-06-30 21:00               ` Hydro Flask
  2020-07-06 22:00                 ` Rich Felker
  0 siblings, 1 reply; 13+ messages in thread
From: Hydro Flask @ 2020-06-30 21:00 UTC (permalink / raw)
  To: musl; +Cc: Rich Felker

On 2020-06-30 12:54, Rich Felker wrote:
> Note that for fixing this issue, it won't suffice just to make
> pthread_kill block signals. The other places that use the killlock
> also need to block signals, to make the lock fully AS-safe.
> 
> Rich

Rich did you see the issue I was talking about in the example in my 
previous email? Does it make sense?

I do not mind submitting a patch for this issue if you don't currently 
have the spare cycles, just let me know if we are in agreement in terms 
of understanding the issue and I can submit a patch that blocks "app 
signals" in every place that uses the killlock.

Probably could also remove the pthread_self() special case that you 
referenced earlier in the thread but that would be a second patch.

Thank you,
Hydro

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [musl] Potential deadlock in pthread_kill()
  2020-06-30 21:00               ` Hydro Flask
@ 2020-07-06 22:00                 ` Rich Felker
  2020-07-06 22:14                   ` Hydro Flask
  0 siblings, 1 reply; 13+ messages in thread
From: Rich Felker @ 2020-07-06 22:00 UTC (permalink / raw)
  To: Hydro Flask; +Cc: musl

[-- Attachment #1: Type: text/plain, Size: 967 bytes --]

On Tue, Jun 30, 2020 at 02:00:13PM -0700, Hydro Flask wrote:
> On 2020-06-30 12:54, Rich Felker wrote:
> >Note that for fixing this issue, it won't suffice just to make
> >pthread_kill block signals. The other places that use the killlock
> >also need to block signals, to make the lock fully AS-safe.
> 
> Rich did you see the issue I was talking about in the example in my
> previous email? Does it make sense?
> 
> I do not mind submitting a patch for this issue if you don't
> currently have the spare cycles, just let me know if we are in
> agreement in terms of understanding the issue and I can submit a
> patch that blocks "app signals" in every place that uses the
> killlock.
> 
> Probably could also remove the pthread_self() special case that you
> referenced earlier in the thread but that would be a second patch.

Yes, I see it clearly now. Sorry it took a while. I have prepared the
attached patch which I'll push soon if there are no problems.

Rich

[-- Attachment #2: 0001-make-thread-killlock-async-signal-safe-for-pthread_k.patch --]
[-- Type: text/plain, Size: 4223 bytes --]

From 7cc9496a18c3fa665c286b8be41d790c795e0171 Mon Sep 17 00:00:00 2001
From: Rich Felker <dalias@aerifal.cx>
Date: Mon, 6 Jul 2020 17:56:19 -0400
Subject: [PATCH] make thread killlock async-signal-safe for pthread_kill

pthread_kill is required to be AS-safe. that requirement can't be met
if the target thread's killlock can be taken in contexts where
application-installed signal handlers can run.

block signals around use of this lock in all pthread_* functions which
target a tid, and reorder blocking/unblocking of signals in
pthread_exit so that they're blocked whenever the killlock is held.
---
 src/thread/pthread_create.c        | 11 ++++++-----
 src/thread/pthread_getschedparam.c |  3 +++
 src/thread/pthread_kill.c          |  3 +++
 src/thread/pthread_setschedparam.c |  3 +++
 src/thread/pthread_setschedprio.c  |  3 +++
 5 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/src/thread/pthread_create.c b/src/thread/pthread_create.c
index 6bdfb44f..10f1b7d8 100644
--- a/src/thread/pthread_create.c
+++ b/src/thread/pthread_create.c
@@ -72,12 +72,13 @@ _Noreturn void __pthread_exit(void *result)
 	/* Access to target the exiting thread with syscalls that use
 	 * its kernel tid is controlled by killlock. For detached threads,
 	 * any use past this point would have undefined behavior, but for
-	 * joinable threads it's a valid usage that must be handled. */
+	 * joinable threads it's a valid usage that must be handled.
+	 * Signals must be blocked since pthread_kill must be AS-safe. */
+	__block_app_sigs(&set);
 	LOCK(self->killlock);
 
-	/* The thread list lock must be AS-safe, and thus requires
-	 * application signals to be blocked before it can be taken. */
-	__block_app_sigs(&set);
+	/* The thread list lock must be AS-safe, and thus depends on
+	 * application signals being blocked above. */
 	__tl_lock();
 
 	/* If this is the only thread in the list, don't proceed with
@@ -85,8 +86,8 @@ _Noreturn void __pthread_exit(void *result)
 	 * signal state to prepare for exit to call atexit handlers. */
 	if (self->next == self) {
 		__tl_unlock();
-		__restore_sigs(&set);
 		UNLOCK(self->killlock);
+		__restore_sigs(&set);
 		exit(0);
 	}
 
diff --git a/src/thread/pthread_getschedparam.c b/src/thread/pthread_getschedparam.c
index 1cba073d..c098befb 100644
--- a/src/thread/pthread_getschedparam.c
+++ b/src/thread/pthread_getschedparam.c
@@ -4,6 +4,8 @@
 int pthread_getschedparam(pthread_t t, int *restrict policy, struct sched_param *restrict param)
 {
 	int r;
+	sigset_t set;
+	__block_app_sigs(&set);
 	LOCK(t->killlock);
 	if (!t->tid) {
 		r = ESRCH;
@@ -14,5 +16,6 @@ int pthread_getschedparam(pthread_t t, int *restrict policy, struct sched_param
 		}
 	}
 	UNLOCK(t->killlock);
+	__restore_sigs(&set);
 	return r;
 }
diff --git a/src/thread/pthread_kill.c b/src/thread/pthread_kill.c
index 3d9395cb..446254b6 100644
--- a/src/thread/pthread_kill.c
+++ b/src/thread/pthread_kill.c
@@ -4,9 +4,12 @@
 int pthread_kill(pthread_t t, int sig)
 {
 	int r;
+	sigset_t set;
+	__block_app_sigs(&set);
 	LOCK(t->killlock);
 	r = t->tid ? -__syscall(SYS_tkill, t->tid, sig)
 		: (sig+0U >= _NSIG ? EINVAL : 0);
 	UNLOCK(t->killlock);
+	__restore_sigs(&set);
 	return r;
 }
diff --git a/src/thread/pthread_setschedparam.c b/src/thread/pthread_setschedparam.c
index 038d13d8..76d4d45a 100644
--- a/src/thread/pthread_setschedparam.c
+++ b/src/thread/pthread_setschedparam.c
@@ -4,8 +4,11 @@
 int pthread_setschedparam(pthread_t t, int policy, const struct sched_param *param)
 {
 	int r;
+	sigset_t set;
+	__block_app_sigs(&set);
 	LOCK(t->killlock);
 	r = !t->tid ? ESRCH : -__syscall(SYS_sched_setscheduler, t->tid, policy, param);
 	UNLOCK(t->killlock);
+	__restore_sigs(&set);
 	return r;
 }
diff --git a/src/thread/pthread_setschedprio.c b/src/thread/pthread_setschedprio.c
index 5bf4a019..fc2e13dd 100644
--- a/src/thread/pthread_setschedprio.c
+++ b/src/thread/pthread_setschedprio.c
@@ -4,8 +4,11 @@
 int pthread_setschedprio(pthread_t t, int prio)
 {
 	int r;
+	sigset_t set;
+	__block_app_sigs(&set);
 	LOCK(t->killlock);
 	r = !t->tid ? ESRCH : -__syscall(SYS_sched_setparam, t->tid, &prio);
 	UNLOCK(t->killlock);
+	__restore_sigs(&set);
 	return r;
 }
-- 
2.21.0


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [musl] Potential deadlock in pthread_kill()
  2020-07-06 22:00                 ` Rich Felker
@ 2020-07-06 22:14                   ` Hydro Flask
  2020-07-06 22:22                     ` Rich Felker
  0 siblings, 1 reply; 13+ messages in thread
From: Hydro Flask @ 2020-07-06 22:14 UTC (permalink / raw)
  To: Rich Felker; +Cc: musl

On 2020-07-06 15:00, Rich Felker wrote:
> Yes, I see it clearly now. Sorry it took a while. I have prepared the
> attached patch which I'll push soon if there are no problems.

Needs one more tiny tweak. I noticed that pthread_cancel() calls 
pthread_kill(). That means pthread_kill() must be async-cancel-safe. If 
an asynchronous cancellation happens after pthread_kill() grabs the 
killlock, then it will deadlock because the asynchronous 
pthread_exit(PTHREAD_CANCELED) call will then recursively try to grab 
killlock.

The solution as far as I can tell is to not just block app signals when 
grabbing killlock, but all signals.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [musl] Potential deadlock in pthread_kill()
  2020-07-06 22:14                   ` Hydro Flask
@ 2020-07-06 22:22                     ` Rich Felker
  2020-07-06 22:37                       ` Hydro Flask
  0 siblings, 1 reply; 13+ messages in thread
From: Rich Felker @ 2020-07-06 22:22 UTC (permalink / raw)
  To: Hydro Flask; +Cc: musl

On Mon, Jul 06, 2020 at 03:14:43PM -0700, Hydro Flask wrote:
> On 2020-07-06 15:00, Rich Felker wrote:
> >Yes, I see it clearly now. Sorry it took a while. I have prepared the
> >attached patch which I'll push soon if there are no problems.
> 
> Needs one more tiny tweak. I noticed that pthread_cancel() calls
> pthread_kill(). That means pthread_kill() must be async-cancel-safe.
> If an asynchronous cancellation happens after pthread_kill() grabs
> the killlock, then it will deadlock because the asynchronous
> pthread_exit(PTHREAD_CANCELED) call will then recursively try to
> grab killlock.
> 
> The solution as far as I can tell is to not just block app signals
> when grabbing killlock, but all signals.

Indeed. It'd also work to disable async cancellation for the duration
of the pthread_cancel call, but that's almost surely more work.

Are you in agreement that it suffices for only pthread_kill to block
all signals? (Still blocking just app signals everywhere else) The
scheduling functions could be changed too, but I'm hesitant to change
pthread_exit without thinking about it further since it has a lot more
subtleties. And I think only pthread_kill needs it since it's the only
one that needs to be AC-safe.

Rich

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [musl] Potential deadlock in pthread_kill()
  2020-07-06 22:22                     ` Rich Felker
@ 2020-07-06 22:37                       ` Hydro Flask
  0 siblings, 0 replies; 13+ messages in thread
From: Hydro Flask @ 2020-07-06 22:37 UTC (permalink / raw)
  To: musl

On 2020-07-06 15:22, Rich Felker wrote:
> On Mon, Jul 06, 2020 at 03:14:43PM -0700, Hydro Flask wrote:
>> On 2020-07-06 15:00, Rich Felker wrote:
>> >Yes, I see it clearly now. Sorry it took a while. I have prepared the
>> >attached patch which I'll push soon if there are no problems.
>> 
>> Needs one more tiny tweak. I noticed that pthread_cancel() calls
>> pthread_kill(). That means pthread_kill() must be async-cancel-safe.
>> If an asynchronous cancellation happens after pthread_kill() grabs
>> the killlock, then it will deadlock because the asynchronous
>> pthread_exit(PTHREAD_CANCELED) call will then recursively try to
>> grab killlock.
>> 
>> The solution as far as I can tell is to not just block app signals
>> when grabbing killlock, but all signals.
> 
> Are you in agreement that it suffices for only pthread_kill to block
> all signals? (Still blocking just app signals everywhere else) The
> scheduling functions could be changed too, but I'm hesitant to change
> pthread_exit without thinking about it further since it has a lot more
> subtleties. And I think only pthread_kill needs it since it's the only
> one that needs to be AC-safe.

Yes I think that's right, since every other function that grabs the 
killlock can assume that async cancellation won't happen.

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2020-07-06 22:38 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-30  4:19 [musl] Potential deadlock in pthread_kill() Hydro Flask
2020-06-30  4:43 ` Rich Felker
2020-06-30  6:19   ` Hydro Flask
2020-06-30  9:26     ` Rich Felker
2020-06-30 14:58       ` Markus Wichmann
2020-06-30 16:28         ` Hydro Flask
2020-06-30 19:28           ` Dmitry Samersoff
2020-06-30 19:54             ` Rich Felker
2020-06-30 21:00               ` Hydro Flask
2020-07-06 22:00                 ` Rich Felker
2020-07-06 22:14                   ` Hydro Flask
2020-07-06 22:22                     ` Rich Felker
2020-07-06 22:37                       ` Hydro Flask

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).