diff mbox series

[v2,8/9] nptl: Use pthread_kill on pthread_cancel

Message ID 20210527172823.3461314-9-adhemerval.zanella@linaro.org
State Committed
Commit a6c813d0ad0fd9830f2cd3c3d079af8d2aa50a1f
Delegated to: Florian Weimer
Headers
Series nptl: pthread cancellation refactor |

Checks

Context Check Description
dj/TryBot-apply_patch success Patch applied to master at the time it was sent

Commit Message

Adhemerval Zanella May 27, 2021, 5:28 p.m. UTC 
  It consolidates the tgkill call and it is the first step of making
pthread_cancel async-signal-safe.  It also fix a possible issue
where the 'struct pthread' tid is not read atomically, which might
send an invalid cancellation signal (similar to what
db988e50a87f613cb6b9e98a2fc66a4848bc3546 fixed for pthread_join).

Checked on x86_64-linux-gnu and aarch64-linux-gnu.
---
 nptl/pthreadP.h       |  2 ++
 nptl/pthread_cancel.c |  6 +-----
 nptl/pthread_kill.c   | 18 ++++++++++++------
 3 files changed, 15 insertions(+), 11 deletions(-)

Comments

Florian Weimer June 1, 2021, 12:41 p.m. UTC | #1 
* Adhemerval Zanella via Libc-alpha:

> It consolidates the tgkill call and it is the first step of making
> pthread_cancel async-signal-safe.  It also fix a possible issue
> where the 'struct pthread' tid is not read atomically, which might
> send an invalid cancellation signal (similar to what
> db988e50a87f613cb6b9e98a2fc66a4848bc3546 fixed for pthread_join).

Looks okay to me, thanks.

Florian
diff mbox series

Patch

diff --git a/nptl/pthreadP.h b/nptl/pthreadP.h
index 618922f47a..675d1de753 100644
--- a/nptl/pthreadP.h
+++ b/nptl/pthreadP.h
@@ -515,6 +515,8 @@  libc_hidden_proto (__pthread_detach)
 extern int __pthread_kill (pthread_t threadid, int signo);
 libc_hidden_proto (__pthread_kill)
 extern int __pthread_cancel (pthread_t th);
+extern int __pthread_kill_internal (pthread_t threadid, int signo)
+  attribute_hidden;
 extern void __pthread_exit (void *value) __attribute__ ((__noreturn__));
 libc_hidden_proto (__pthread_exit)
 extern int __pthread_join (pthread_t threadid, void **thread_return);
diff --git a/nptl/pthread_cancel.c b/nptl/pthread_cancel.c
index c6086b6c3c..5fae763861 100644
--- a/nptl/pthread_cancel.c
+++ b/nptl/pthread_cancel.c
@@ -109,11 +109,7 @@  __pthread_cancel (pthread_t th)
       return 0;
     }
 
-  pid_t pid = __getpid ();
-  int val = INTERNAL_SYSCALL_CALL (tgkill, pid, pd->tid, SIGCANCEL);
-  return INTERNAL_SYSCALL_ERROR_P (val)
-	 ? INTERNAL_SYSCALL_ERRNO (val)
-	 : 0;
+  return __pthread_kill_internal (th, SIGCANCEL);
 }
 versioned_symbol (libc, __pthread_cancel, pthread_cancel, GLIBC_2_34);
 
diff --git a/nptl/pthread_kill.c b/nptl/pthread_kill.c
index 0392c3f9e5..93387dd60a 100644
--- a/nptl/pthread_kill.c
+++ b/nptl/pthread_kill.c
@@ -21,13 +21,8 @@ 
 #include <shlib-compat.h>
 
 int
-__pthread_kill (pthread_t threadid, int signo)
+__pthread_kill_internal (pthread_t threadid, int signo)
 {
-  /* Disallow sending the signal we use for cancellation, timers,
-     for the setxid implementation.  */
-  if (__is_internal_signal (signo))
-    return EINVAL;
-
   pid_t tid;
   struct pthread *pd = (struct pthread *) threadid;
 
@@ -53,6 +48,17 @@  __pthread_kill (pthread_t threadid, int signo)
 
   return val;
 }
+
+int
+__pthread_kill (pthread_t threadid, int signo)
+{
+  /* Disallow sending the signal we use for cancellation, timers,
+     for the setxid implementation.  */
+  if (__is_internal_signal (signo))
+    return EINVAL;
+
+  return __pthread_kill_internal (threadid, signo);
+}
 /* Some architectures (for instance arm) might pull raise through libgcc, so
    avoid the symbol version if it ends up being used on ld.so.  */
 #if !IS_IN(rtld)