From patchwork Fri Aug 20 16:13:58 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Florian Weimer X-Patchwork-Id: 44729 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id BEE2F39C0005 for ; Fri, 20 Aug 2021 16:15:03 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org BEE2F39C0005 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1629476103; bh=hkrcdLXOf4YmpaC3wWkcr00mWUSSKSvFJfFLdTQd9V4=; h=To:Subject:In-Reply-To:References:Date:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=B7gjmYVdARl74tEtX3btFPq95Kb8wwKXgNElpK1p0xfqF59E7VNRa0lqb+sHb0LJY xjcsI6hmnRf9NYkWC8284xgVaLN2sMIPp4YJw2FqVbvutuHNVLIiY2wqjf9kB9Y53r UGDKWxYnSCnCQ67VLicUdGjM8Lk7KY7u004+fwvo= X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.205.24.124]) by sourceware.org (Postfix) with ESMTP id AE7C23898023 for ; Fri, 20 Aug 2021 16:14:05 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org AE7C23898023 Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-469-Tg74QQ4uN0CpJIhz9eAw_A-1; Fri, 20 Aug 2021 12:14:02 -0400 X-MC-Unique: Tg74QQ4uN0CpJIhz9eAw_A-1 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 5CE5E760C0 for ; Fri, 20 Aug 2021 16:14:01 +0000 (UTC) Received: from oldenburg.str.redhat.com (unknown [10.39.194.2]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 58D265D9FC for ; Fri, 20 Aug 2021 16:14:00 +0000 (UTC) To: libc-alpha@sourceware.org Subject: [PATCH v2 3/4] nptl: Add internal __pthread_call_with_tid function In-Reply-To: References: X-From-Line: 39f7f2474c4c9e8ae63fc00c60574ee5a657e12d Mon Sep 17 00:00:00 2001 Message-Id: <39f7f2474c4c9e8ae63fc00c60574ee5a657e12d.1629475813.git.fweimer@redhat.com> Date: Fri, 20 Aug 2021 18:13:58 +0200 User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.2 (gnu/linux) MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-12.2 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_NONE, TXREP, URIBL_BLACK autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Florian Weimer via Libc-alpha From: Florian Weimer Reply-To: Florian Weimer Errors-To: libc-alpha-bounces+patchwork=sourceware.org@sourceware.org Sender: "Libc-alpha" 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 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 + . */ + +#include +#include +#include +#include +#include + +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 #include #include +#include #include @@ -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 + . */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +/* 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 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