[v2,3/4] nptl: Add internal __pthread_call_with_tid function

Message ID 39f7f2474c4c9e8ae63fc00c60574ee5a657e12d.1629475813.git.fweimer@redhat.com
State Superseded
Headers
Series Fix ESRCH issues in pthread_cancel, pthread_kill |

Checks

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

Commit Message

Florian Weimer Aug. 20, 2021, 4:13 p.m. UTC
  It will be used to address race conditions during thread exit
due to TID reuse.

nptl/tst-pthread_call_with_tid has to be a static test because
__pthread_call_with_tid is not exported from libc.so.
---
v2: New patch, extracted from the previous 3/3 patch.  Added signal
    blocking (but not for the self-thread case), which is required for
    correctness.

 nptl/Makefile                    |   5 +-
 nptl/allocatestack.c             |   1 +
 nptl/descr.h                     |  11 +++
 nptl/pthread_call_with_tid.c     |  66 +++++++++++++
 nptl/pthread_create.c            |  18 ++++
 nptl/tst-pthread_call_with_tid.c | 165 +++++++++++++++++++++++++++++++
 sysdeps/nptl/pthreadP.h          |  20 ++++
 7 files changed, 285 insertions(+), 1 deletion(-)
 create mode 100644 nptl/pthread_call_with_tid.c
 create mode 100644 nptl/tst-pthread_call_with_tid.c
  

Patch

diff --git a/nptl/Makefile b/nptl/Makefile
index ff4d590f11..f6167dcd54 100644
--- a/nptl/Makefile
+++ b/nptl/Makefile
@@ -93,6 +93,7 @@  routines = \
   pthread_barrierattr_getpshared \
   pthread_barrierattr_init \
   pthread_barrierattr_setpshared \
+  pthread_call_with_tid \
   pthread_cancel \
   pthread_cleanup_upto \
   pthread_clockjoin \
@@ -436,7 +437,9 @@  tests-static += tst-stackguard1-static \
 		tst-mutex8-static tst-mutexpi8-static tst-sem11-static \
 		tst-sem12-static tst-cond11-static \
 		tst-pthread-gdb-attach-static \
-		tst-pthread_exit-nothreads-static
+		tst-pthread_exit-nothreads-static \
+		tst-pthread_call_with_tid \
+		# tests-static
 
 tests += tst-cancel24-static
 
diff --git a/nptl/allocatestack.c b/nptl/allocatestack.c
index cfe37a3443..dd9e0b2c6b 100644
--- a/nptl/allocatestack.c
+++ b/nptl/allocatestack.c
@@ -127,6 +127,7 @@  get_cached_stack (size_t *sizep, void **memp)
   /* No pending event.  */
   result->nextevent = NULL;
 
+  result->exit_futex = 0;
   result->tls_state = (struct tls_internal_t) { 0 };
 
   /* Clear the DTV.  */
diff --git a/nptl/descr.h b/nptl/descr.h
index c85778d449..b848b9ecd4 100644
--- a/nptl/descr.h
+++ b/nptl/descr.h
@@ -396,6 +396,17 @@  struct pthread
      PTHREAD_CANCEL_ASYNCHRONOUS).  */
   unsigned char canceltype;
 
+  /* Futex to prevent thread exit during __pthread_call_with_tid.  The
+     least-significant bit is an exit flag.  The remaining bits count
+     the number of __pthread_call_with_tid calls in progress.
+
+     An exiting thread sets the LSB, and waits until exit_futex is 1.
+
+     If the thread is not yet exiting (as indicated by the LSB),
+     __pthread_call_with_tid atomically adds 2 to the futex variable
+     before the callback, and subtracts 2 after it.  */
+  unsigned int exit_futex;
+
   /* Used on strsignal.  */
   struct tls_internal_t tls_state;
 
diff --git a/nptl/pthread_call_with_tid.c b/nptl/pthread_call_with_tid.c
new file mode 100644
index 0000000000..ea3e801e22
--- /dev/null
+++ b/nptl/pthread_call_with_tid.c
@@ -0,0 +1,66 @@ 
+/* Invoke a callback with the TID of a specific thread.
+   Copyright (C) 2021 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#include <futex-internal.h>
+#include <internal-signals.h>
+#include <pthreadP.h>
+#include <sysdep.h>
+#include <unistd.h>
+
+int
+__pthread_call_with_tid (pthread_t thr,
+                         int (*callback) (struct pthread *thr, pid_t tid,
+                                          void *closure),
+                         void *closure)
+{
+  struct pthread *pd = (struct pthread *) thr;
+
+  if (pd == THREAD_SELF)
+    {
+      /* Use the actual TID from the kernel, so that it refers to the
+         current thread even if called after vfork.  No signal
+         blocking in this case, so that the signal is delivered
+         immediately.  */
+      pid_t tid = INLINE_SYSCALL_CALL (gettid);
+      return callback (pd, tid, closure);
+    }
+
+  /* Block all signals.  The counter increment/decrement is not
+     reentrant.  */
+  sigset_t old_mask;
+  __libc_signal_block_all (&old_mask);
+
+  pid_t tid;
+  if (atomic_fetch_add_acq_rel (&pd->exit_futex, 2) & 1)
+    /* The target thread has exited or is about to.  There is no
+       stable TID value anymore.  */
+    tid = 0;
+  else
+    tid = pd->tid;
+
+  int ret = callback (pd, tid, closure);
+
+  /* A value 3 before the update indicates that the thread is exiting,
+     and this is the last remaining concurrent operation.  */
+  if (atomic_fetch_add_acq_rel (&pd->exit_futex, -2) == 3)
+    futex_wake (&pd->exit_futex, 1, FUTEX_PRIVATE);
+
+  __libc_signal_restore_set (&old_mask);
+
+  return ret;
+}
diff --git a/nptl/pthread_create.c b/nptl/pthread_create.c
index d8ec299cb1..e012d89ecb 100644
--- a/nptl/pthread_create.c
+++ b/nptl/pthread_create.c
@@ -37,6 +37,7 @@ 
 #include <sys/single_threaded.h>
 #include <version.h>
 #include <clone_internal.h>
+#include <futex-internal.h>
 
 #include <shlib-compat.h>
 
@@ -485,6 +486,23 @@  start_thread (void *arg)
     /* This was the last thread.  */
     exit (0);
 
+  /* This prevents sending a signal from this thread to itself during
+     its final stages.  This must come after the exit call above
+     because atexit handlers must not run with signals blocked.  */
+  __libc_signal_block_all (NULL);
+
+  /* Delay exiting this thread until there are no concurrent
+     __pthread_call_with_tid operations in progress.  */
+  {
+    /* Indicate that no further signals should be sent.  */
+    atomic_fetch_or_release (&pd->exit_futex, 1);
+
+    /* Wait until no __pthread_call_with_tid operations are left.  */
+    unsigned int val;
+    while ((val = atomic_load_acquire (&pd->exit_futex)) != 1)
+      futex_wait (&pd->exit_futex, val, FUTEX_PRIVATE);
+  }
+
 #ifndef __ASSUME_SET_ROBUST_LIST
   /* If this thread has any robust mutexes locked, handle them now.  */
 # if __PTHREAD_MUTEX_HAVE_PREV
diff --git a/nptl/tst-pthread_call_with_tid.c b/nptl/tst-pthread_call_with_tid.c
new file mode 100644
index 0000000000..07f8e7f0fa
--- /dev/null
+++ b/nptl/tst-pthread_call_with_tid.c
@@ -0,0 +1,165 @@ 
+/* Whitebox test for __pthread_call_with_tid.
+   Copyright (C) 2021 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#include <pthreadP.h>
+#include <stddef.h>
+#include <stdint.h>
+#include <support/check.h>
+#include <support/support.h>
+#include <support/xsignal.h>
+#include <support/xthread.h>
+#include <support/xunistd.h>
+#include <unistd.h>
+
+/* Check if SIGUSR1 is BLOCKED.  */
+static void
+check_sigusr1 (bool blocked)
+{
+  sigset_t signals;
+  xpthread_sigmask (SIG_BLOCK, NULL, &signals);
+  TEST_COMPARE (!sigismember (&signals, SIGUSR1), !blocked);
+}
+
+/* Direct call to this function.  */
+static int
+callback_self_direct (struct pthread *pd, pid_t tid, void *closure)
+{
+  TEST_VERIFY (pd == THREAD_SELF);
+  TEST_COMPARE (getpid (), gettid ()); /* No other threads.  */
+  TEST_COMPARE (tid, gettid());
+  TEST_COMPARE (tid, pd->tid);
+
+  check_sigusr1 (false);
+
+  return *(int *) closure;
+}
+
+/* Callback for call with pthread_self after vfork.  */
+static int
+callback_self_vfork (struct pthread *pd, pid_t tid, void *closure)
+{
+  TEST_VERIFY (pd == THREAD_SELF);
+
+  /* The stored TID refers to the parent process.  */
+  TEST_COMPARE (getppid (), pd->tid);
+
+  TEST_COMPARE (getpid (), gettid ()); /* No other threads.  */
+  TEST_COMPARE (tid, gettid());
+
+  check_sigusr1 (false);
+  return *(int *) closure;
+}
+
+/* Callback for call on other thread that is running.  Returns TID of
+   the other thread.  */
+static int
+callback_other_running (struct pthread *pd, pid_t tid, void *closure)
+{
+  TEST_COMPARE (tid, pd->tid);
+  TEST_VERIFY (closure == NULL);
+  check_sigusr1 (true);
+  return tid;
+}
+
+/* Thread function that goes with callback_other_running.  Returns
+   TID.  */
+static void *
+threadfunc_other_running (void *closure)
+{
+  pthread_barrier_t *barrier = closure;
+
+  /* Do not exit until main thread says so. */
+  xpthread_barrier_wait (barrier);
+
+  return (void *) (intptr_t) gettid ();
+}
+
+/* Callback for call on other thread that has exited.  */
+static int
+callback_other_exited (struct pthread *pd, pid_t tid, void *closure)
+{
+  TEST_COMPARE (tid, 0);        /* No TID available.  */
+  check_sigusr1 (true);
+  return *(int *) closure;
+}
+
+/* Thread function that goes with callback_other_exited.  */
+static void *
+threadfunc_other_exited (void *closure)
+{
+  return NULL;
+}
+
+static int
+do_test (void)
+{
+  check_sigusr1 (false);
+
+  /* Same thread, direct call.  */
+  int ret = 5;
+  TEST_COMPARE (5, __pthread_call_with_tid (pthread_self (),
+                                            callback_self_direct, &ret));
+  check_sigusr1 (false);
+
+  /* Same thread, after vfork.  */
+  {
+    pid_t pid = vfork ();
+    if (pid == 0)
+      {
+        ret = 6;
+        TEST_COMPARE (6, __pthread_call_with_tid (pthread_self (),
+                                                  callback_self_vfork, &ret));
+        check_sigusr1 (false);
+        _exit (0);
+      }
+    if (pid < 0)
+      FAIL_EXIT1 ("vfork: %m");
+    int status;
+    xwaitpid (pid, &status, 0);
+    TEST_COMPARE (status, 0);
+  }
+
+  /* Other thread, still running.  */
+  {
+    pthread_barrier_t barrier;
+    xpthread_barrier_init (&barrier, NULL, 2);
+    pthread_t thr = xpthread_create (NULL, threadfunc_other_running, &barrier);
+    pid_t tid1 = __pthread_call_with_tid (thr,
+                                          callback_other_running, NULL);
+    check_sigusr1 (false);
+    xpthread_barrier_wait (&barrier);
+    pid_t tid2 = (intptr_t) xpthread_join (thr);
+    xpthread_barrier_destroy (&barrier);
+    TEST_COMPARE (tid1, tid2);
+  }
+
+  /* Other thread, exited.  */
+  {
+    pthread_t thr = xpthread_create (NULL, threadfunc_other_exited, NULL);
+    support_wait_for_thread_exit ();
+    ret = 7;
+    TEST_COMPARE (7, __pthread_call_with_tid (thr,
+                                              callback_other_exited, &ret));
+    check_sigusr1 (false);
+    xpthread_join (thr);
+  }
+
+  return 0;
+}
+
+#include <support/test-driver.c>
diff --git a/sysdeps/nptl/pthreadP.h b/sysdeps/nptl/pthreadP.h
index 374657a2fd..fe28389163 100644
--- a/sysdeps/nptl/pthreadP.h
+++ b/sysdeps/nptl/pthreadP.h
@@ -673,6 +673,26 @@  extern void __wait_lookup_done (void) attribute_hidden;
 int __pthread_attr_extension (struct pthread_attr *attr) attribute_hidden
   __attribute_warn_unused_result__;
 
+/* __pthread_call_with_tid CALLBACK (THR, TID, CLOSURE), where TID is
+   either the current TID of THR, or zero if THR has exited or is
+   about to exit.  If TID is not zero, the thread denoted by it is
+   guaranteed not to exit before CALLBACK returns.
+
+   __pthread_call_with_tid returns the result of CALLBACK.
+
+   If THR refers to the current thread, the actual TID is used (to
+   support vfork).  CALLBACK may unwind in this case (e.g., siglongjmp
+   from user-supplied signal handler).
+
+   If THR is not the current thread, then all signals are blocked
+   until CALLBACK returns.  CALLBACK is assumed to return normally (no
+   unwinding).  This makes the __pthread_call_with_tid
+   async-signal-safe as long as CALLBACK is async-signal-safe. */
+int __pthread_call_with_tid (pthread_t thr,
+			     int (*callback) (struct pthread *thr, pid_t tid,
+					      void *closure),
+			     void *closure) attribute_hidden;
+
 #ifdef SHARED
 # define PTHREAD_STATIC_FN_REQUIRE(name)
 #else