nptl: Fix race between pthread_kill and thread exit (bug 12889)

A new thread exit lock and flag are introduced.  They are used to
detect that the thread is about to exit or has exited in
__pthread_kill_internal, and the signal is not sent in this case.

The test sysdeps/pthread/tst-pthread_cancel-select-loop.c is derived
from a downstream test originally written by Marek Polacek.

Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>
This commit is contained in:
Florian Weimer 2021-09-13 11:06:08 +02:00
parent 8af8456004
commit 526c3cf11e
7 changed files with 276 additions and 26 deletions

View File

@ -31,6 +31,7 @@
#include <futex-internal.h>
#include <kernel-features.h>
#include <nptl-stack.h>
#include <libc-lock.h>
/* Default alignment of stack. */
#ifndef STACK_ALIGN
@ -126,6 +127,8 @@ get_cached_stack (size_t *sizep, void **memp)
/* No pending event. */
result->nextevent = NULL;
result->exiting = false;
__libc_lock_init (result->exit_lock);
result->tls_state = (struct tls_internal_t) { 0 };
/* Clear the DTV. */

View File

@ -395,6 +395,12 @@ struct pthread
PTHREAD_CANCEL_ASYNCHRONOUS). */
unsigned char canceltype;
/* Used in __pthread_kill_internal to detected a thread that has
exited or is about to exit. exit_lock must only be acquired
after blocking signals. */
bool exiting;
int exit_lock; /* A low-level lock (for use with __libc_lock_init etc). */
/* Used on strsignal. */
struct tls_internal_t tls_state;

View File

@ -36,6 +36,7 @@
#include <sys/single_threaded.h>
#include <version.h>
#include <clone_internal.h>
#include <futex-internal.h>
#include <shlib-compat.h>
@ -484,6 +485,19 @@ 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);
/* Tell __pthread_kill_internal that this thread is about to exit.
If there is a __pthread_kill_internal in progress, this delays
the thread exit until the signal has been queued by the kernel
(so that the TID used to send it remains valid). */
__libc_lock_lock (pd->exit_lock);
pd->exiting = true;
__libc_lock_unlock (pd->exit_lock);
#ifndef __ASSUME_SET_ROBUST_LIST
/* If this thread has any robust mutexes locked, handle them now. */
# if __PTHREAD_MUTEX_HAVE_PREV

View File

@ -16,6 +16,7 @@
License along with the GNU C Library; if not, see
<https://www.gnu.org/licenses/>. */
#include <libc-lock.h>
#include <unistd.h>
#include <pthreadP.h>
#include <shlib-compat.h>
@ -23,37 +24,51 @@
int
__pthread_kill_internal (pthread_t threadid, int signo)
{
pid_t tid;
struct pthread *pd = (struct pthread *) threadid;
if (pd == THREAD_SELF)
/* It is a special case to handle raise() implementation after a vfork
call (which does not update the PD tid field). */
tid = INLINE_SYSCALL_CALL (gettid);
else
/* Force load of pd->tid into local variable or register. Otherwise
if a thread exits between ESRCH test and tgkill, we might return
EINVAL, because pd->tid would be cleared by the kernel. */
tid = atomic_forced_read (pd->tid);
int val;
if (__glibc_likely (tid > 0))
{
pid_t pid = __getpid ();
val = INTERNAL_SYSCALL_CALL (tgkill, pid, tid, signo);
val = (INTERNAL_SYSCALL_ERROR_P (val)
? INTERNAL_SYSCALL_ERRNO (val) : 0);
/* Use the actual TID from the kernel, so that it refers to the
current thread even if called after vfork. There is no
signal blocking in this case, so that the signal is delivered
immediately, before __pthread_kill_internal returns: a signal
sent to the thread itself needs to be delivered
synchronously. (It is unclear if Linux guarantees the
delivery of all pending signals after unblocking in the code
below. POSIX only guarantees delivery of a single signal,
which may not be the right one.) */
pid_t tid = INTERNAL_SYSCALL_CALL (gettid);
int ret = INTERNAL_SYSCALL_CALL (kill, tid, signo);
return INTERNAL_SYSCALL_ERROR_P (ret) ? INTERNAL_SYSCALL_ERRNO (ret) : 0;
}
else
/* The kernel reports that the thread has exited. POSIX specifies
the ESRCH error only for the case when the lifetime of a thread
ID has ended, but calling pthread_kill on such a thread ID is
undefined in glibc. Therefore, do not treat kernel thread exit
as an error. */
val = 0;
return val;
/* Block all signals, as required by pd->exit_lock. */
sigset_t old_mask;
__libc_signal_block_all (&old_mask);
__libc_lock_lock (pd->exit_lock);
int ret;
if (pd->exiting)
/* The thread is about to exit (or has exited). Sending the
signal is either not observable (the target thread has already
blocked signals at this point), or it will fail, or it might be
delivered to a new, unrelated thread that has reused the TID.
So do not actually send the signal. Do not report an error
because the threadid argument is still valid (the thread ID
lifetime has not ended), and ESRCH (for example) would be
misleading. */
ret = 0;
else
{
/* Using tgkill is a safety measure. pd->exit_lock ensures that
the target thread cannot exit. */
ret = INTERNAL_SYSCALL_CALL (tgkill, __getpid (), pd->tid, signo);
ret = INTERNAL_SYSCALL_ERROR_P (ret) ? INTERNAL_SYSCALL_ERRNO (ret) : 0;
}
__libc_lock_unlock (pd->exit_lock);
__libc_signal_restore_set (&old_mask);
return ret;
}
int

View File

@ -119,7 +119,9 @@ tests += tst-cnd-basic tst-mtx-trylock tst-cnd-broadcast \
tst-unwind-thread \
tst-pt-vfork1 tst-pt-vfork2 tst-vfork1x tst-vfork2x \
tst-pthread_cancel-exited \
tst-pthread_cancel-select-loop \
tst-pthread_kill-exited \
tst-pthread_kill-exiting \
# tests
tests-time64 := \

View File

@ -0,0 +1,87 @@
/* Test that pthread_cancel succeeds during thread exit.
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/>. */
/* This test tries to trigger an internal race condition in
pthread_cancel, where the cancellation signal is sent after the
thread has begun the cancellation process. This can result in a
spurious ESRCH error. For the original bug 12889, the window is
quite small, so the bug was not reproduced in every run. */
#include <stdbool.h>
#include <stddef.h>
#include <support/check.h>
#include <support/xthread.h>
#include <support/xunistd.h>
#include <sys/select.h>
#include <unistd.h>
/* Set to true by timeout_thread_function when the test should
terminate. */
static bool timeout;
static void *
timeout_thread_function (void *unused)
{
usleep (5 * 1000 * 1000);
__atomic_store_n (&timeout, true, __ATOMIC_RELAXED);
return NULL;
}
/* Used for blocking the select function below. */
static int pipe_fds[2];
static void *
canceled_thread_function (void *unused)
{
while (true)
{
fd_set rfs;
fd_set wfs;
fd_set efs;
FD_ZERO (&rfs);
FD_ZERO (&wfs);
FD_ZERO (&efs);
FD_SET (pipe_fds[0], &rfs);
/* If the cancellation request is recognized early, the thread
begins exiting while the cancellation signal arrives. */
select (FD_SETSIZE, &rfs, &wfs, &efs, NULL);
}
return NULL;
}
static int
do_test (void)
{
xpipe (pipe_fds);
pthread_t thr_timeout = xpthread_create (NULL, timeout_thread_function, NULL);
while (!__atomic_load_n (&timeout, __ATOMIC_RELAXED))
{
pthread_t thr = xpthread_create (NULL, canceled_thread_function, NULL);
xpthread_cancel (thr);
TEST_VERIFY (xpthread_join (thr) == PTHREAD_CANCELED);
}
xpthread_join (thr_timeout);
xclose (pipe_fds[0]);
xclose (pipe_fds[1]);
return 0;
}
#include <support/test-driver.c>

View File

@ -0,0 +1,123 @@
/* Test that pthread_kill succeeds during thread exit.
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/>. */
/* This test verifies that pthread_kill for a thread that is exiting
succeeds (with or without actually delivering the signal). */
#include <array_length.h>
#include <stdbool.h>
#include <stddef.h>
#include <support/xsignal.h>
#include <support/xthread.h>
#include <unistd.h>
/* Set to true by timeout_thread_function when the test should
terminate. */
static bool timeout;
static void *
timeout_thread_function (void *unused)
{
usleep (1000 * 1000);
__atomic_store_n (&timeout, true, __ATOMIC_RELAXED);
return NULL;
}
/* Used to synchronize the sending threads with the target thread and
main thread. */
static pthread_barrier_t barrier_1;
static pthread_barrier_t barrier_2;
/* The target thread to which signals are to be sent. */
static pthread_t target_thread;
/* Set by the main thread to true after timeout has been set to
true. */
static bool exiting;
static void *
sender_thread_function (void *unused)
{
while (true)
{
/* Wait until target_thread has been initialized. The target
thread and main thread participate in this barrier. */
xpthread_barrier_wait (&barrier_1);
if (exiting)
break;
xpthread_kill (target_thread, SIGUSR1);
/* Communicate that the signal has been sent. The main thread
participates in this barrier. */
xpthread_barrier_wait (&barrier_2);
}
return NULL;
}
static void *
target_thread_function (void *unused)
{
target_thread = pthread_self ();
xpthread_barrier_wait (&barrier_1);
return NULL;
}
static int
do_test (void)
{
xsignal (SIGUSR1, SIG_IGN);
pthread_t thr_timeout = xpthread_create (NULL, timeout_thread_function, NULL);
pthread_t threads[4];
xpthread_barrier_init (&barrier_1, NULL, array_length (threads) + 2);
xpthread_barrier_init (&barrier_2, NULL, array_length (threads) + 1);
for (int i = 0; i < array_length (threads); ++i)
threads[i] = xpthread_create (NULL, sender_thread_function, NULL);
while (!__atomic_load_n (&timeout, __ATOMIC_RELAXED))
{
xpthread_create (NULL, target_thread_function, NULL);
/* Wait for the target thread to be set up and signal sending to
start. */
xpthread_barrier_wait (&barrier_1);
/* Wait for signal sending to complete. */
xpthread_barrier_wait (&barrier_2);
xpthread_join (target_thread);
}
exiting = true;
/* Signal the sending threads to exit. */
xpthread_create (NULL, target_thread_function, NULL);
xpthread_barrier_wait (&barrier_1);
for (int i = 0; i < array_length (threads); ++i)
xpthread_join (threads[i]);
xpthread_join (thr_timeout);
return 0;
}
#include <support/test-driver.c>