From patchwork Fri Oct 1 14:10:44 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Florian Weimer X-Patchwork-Id: 45687 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 E70DA385780B for ; Fri, 1 Oct 2021 14:14:16 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org E70DA385780B DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1633097656; bh=1zFp1ITH/dBkxgOEhaF4fcM0MiDMRlTswN8KUf3iAlQ=; h=To:Subject:Date:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:From; b=bsofN2zN1pOUyFYfkqGUWkhdVKuIayedkrsZvlEx0miF+UUvo0qvAzUhN9dEdKQyR 9fxWUKVYAOv+DE0S9m4+wsmCgMpUrcfnJeBYvSS/6lRSAwHi61dKMIw7tCRrN77TMQ tbnITqkR7RevOBf6/VnTXgSkp7pJcJCMkFdoLUkg= 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 739F93857C7E for ; Fri, 1 Oct 2021 14:10:50 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 739F93857C7E 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-421-Mf1DlKQnPEOo3ev-cTv0yQ-1; Fri, 01 Oct 2021 10:10:47 -0400 X-MC-Unique: Mf1DlKQnPEOo3ev-cTv0yQ-1 Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 4251884A5E0 for ; Fri, 1 Oct 2021 14:10:46 +0000 (UTC) Received: from oldenburg.str.redhat.com (unknown [10.39.192.176]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 98FE710023AE for ; Fri, 1 Oct 2021 14:10:45 +0000 (UTC) To: libc-alpha@sourceware.org Subject: [PATCH] nptl: pthread_kill must send signals to a specific thread [BZ #28407] Date: Fri, 01 Oct 2021 16:10:44 +0200 Message-ID: <87czoox2cb.fsf@oldenburg.str.redhat.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.2 (gnu/linux) MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.84 on 10.5.11.22 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-12.4 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 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" The choice between the kill vs tgkill system calls is not just about the TID reuse race, but also about whether the signal is sent to the whole process (and any thread in it) or to a specific thread. This was caught by the openposix test suite: LTP: openposix test suite - FAIL: SIGUSR1 is member of new thread pendingset. Reviewed-by: Carlos O'Donell Tested-by: Carlos O'Donell --- Tested on i686-linux-gnu, x86_64-linux-gnu. nptl/pthread_kill.c | 4 +- sysdeps/pthread/Makefile | 1 + sysdeps/pthread/tst-pthread-raise-blocked-self.c | 92 ++++++++++++++++++++++++ 3 files changed, 94 insertions(+), 3 deletions(-) diff --git a/nptl/pthread_kill.c b/nptl/pthread_kill.c index a44dc8f2d9..35bf1f973e 100644 --- a/nptl/pthread_kill.c +++ b/nptl/pthread_kill.c @@ -40,7 +40,7 @@ __pthread_kill_implementation (pthread_t threadid, int signo, int no_tid) 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); + int ret = INTERNAL_SYSCALL_CALL (tgkill, __getpid (), tid, signo); return INTERNAL_SYSCALL_ERROR_P (ret) ? INTERNAL_SYSCALL_ERRNO (ret) : 0; } @@ -59,8 +59,6 @@ __pthread_kill_implementation (pthread_t threadid, int signo, int no_tid) ret = no_tid; 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; } diff --git a/sysdeps/pthread/Makefile b/sysdeps/pthread/Makefile index d4bd2d4e3e..0af9c59b42 100644 --- a/sysdeps/pthread/Makefile +++ b/sysdeps/pthread/Makefile @@ -121,6 +121,7 @@ tests += tst-cnd-basic tst-mtx-trylock tst-cnd-broadcast \ tst-pthread-setuid-loop \ tst-pthread_cancel-exited \ tst-pthread_cancel-select-loop \ + tst-pthread-raise-blocked-self \ tst-pthread_kill-exited \ tst-pthread_kill-exiting \ # tests diff --git a/sysdeps/pthread/tst-pthread-raise-blocked-self.c b/sysdeps/pthread/tst-pthread-raise-blocked-self.c new file mode 100644 index 0000000000..128e1a6071 --- /dev/null +++ b/sysdeps/pthread/tst-pthread-raise-blocked-self.c @@ -0,0 +1,92 @@ +/* Test that raise sends signal to current thread even if blocked. + 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 + +/* Used to create a dummy thread ID distinct from all other thread + IDs. */ +static void * +noop (void *ignored) +{ + return NULL; +} + +static volatile pthread_t signal_thread; + +static void +signal_handler (int signo) +{ + signal_thread = pthread_self (); +} + +/* Used to ensure that waiting_thread has launched and can accept + signals. */ +static pthread_barrier_t barrier; + +static void * +waiting_thread (void *ignored) +{ + xpthread_barrier_wait (&barrier); + pause (); + return NULL; +} + +static int +do_test (void) +{ + xsignal (SIGUSR1, signal_handler); + xpthread_barrier_init (&barrier, NULL, 2); + + /* Distinct thread ID value to */ + pthread_t dummy = xpthread_create (NULL, noop, NULL); + signal_thread = dummy; + + pthread_t helper = xpthread_create (NULL, waiting_thread, NULL); + + /* Make sure that the thread is running. */ + xpthread_barrier_wait (&barrier); + + /* Block signals on this thread. */ + sigset_t set; + sigfillset (&set); + xpthread_sigmask (SIG_BLOCK, &set, NULL); + + /* Send the signal to this thread. It must not be delivered. */ + raise (SIGUSR1); + TEST_VERIFY (signal_thread == dummy); + + /* Wait a bit to give a chance for signal delivery (increases + chances of failure with bug 28407). */ + usleep (50 * 1000); + + /* Unblocking should cause synchronous delivery of the signal. */ + xpthread_sigmask (SIG_UNBLOCK, &set, NULL); + TEST_VERIFY (signal_thread == pthread_self ()); + + xpthread_cancel (helper); + xpthread_join (helper); + xpthread_join (dummy); + return 0; +} + +#include