From patchwork Tue Sep 21 16:42:27 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Florian Weimer X-Patchwork-Id: 45242 X-Patchwork-Delegate: carlos@redhat.com 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 77620385802A for ; Tue, 21 Sep 2021 16:43:01 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 77620385802A DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1632242581; bh=/7mA0qmXwY1i6IK28E9z1ZYSjrQDnfK0yn6HKFZXK0I=; h=To:Subject:Date:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:From; b=gLN4kY3fL1ZikXxzEFuy7iV3DcW2rr5xIVQyYZUBMN/Xi9yIyeVVXJFi4gJoVzmLB V1P0KBkzJj6jkhYqki3vQWPRU9fy76YfxZ1UqQDFPdGm7BHfX35X6Uv1CXe2LBOsVH WbQhBYEQXtV0uS5TLzGiCiv8+VUwZ6y7KA5cY9zE= 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 C05C23858402 for ; Tue, 21 Sep 2021 16:42:38 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org C05C23858402 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-299-z34FTIQVMlSWPMqDj8etRQ-1; Tue, 21 Sep 2021 12:42:35 -0400 X-MC-Unique: z34FTIQVMlSWPMqDj8etRQ-1 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 78A641966351 for ; Tue, 21 Sep 2021 16:42:29 +0000 (UTC) Received: from oldenburg.str.redhat.com (unknown [10.39.192.176]) by smtp.corp.redhat.com (Postfix) with ESMTPS id C209418761 for ; Tue, 21 Sep 2021 16:42:28 +0000 (UTC) To: libc-alpha@sourceware.org Subject: [PATCH v2] nptl: Avoid setxid deadlock with blocked signals in thread exit [BZ #28361] Date: Tue, 21 Sep 2021 18:42:27 +0200 Message-ID: <87tuidx4lo.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.79 on 10.5.11.16 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-13.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 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" As part of the fix for bug 12889, signals are blocked during thread exit, so that application code cannot run on the thread that is about to exit. This would cause problems if the application expected signals to be delivered after the signal handler revealed the thread to still exist, despite pthread_kill can no longer be used to send signals to it. However, glibc internally uses the SIGSETXID signal in a way that is incompatible with signal blocking, due to the way the setxid handshake delays thread exit until the setxid operation has completed. With a blocked SIGSETXID, the handshake can never complete, causing a deadlock. As a band-aid, restore the previous handshake protocol by not blocking SIGSETXID during thread exit. The new test sysdeps/pthread/tst-pthread-setuid-loop.c is based on a downstream test by Martin Osvald. Reviewed-by: Carlos O'Donell Tested-by: Carlos O'Donell --- v2: Use __sigfillset instead of memset. nptl/pthread_create.c | 12 +++++- sysdeps/pthread/Makefile | 1 + sysdeps/pthread/tst-pthread-setuid-loop.c | 61 +++++++++++++++++++++++++++++++ 3 files changed, 72 insertions(+), 2 deletions(-) diff --git a/nptl/pthread_create.c b/nptl/pthread_create.c index a559f86cc2..d6ea43a754 100644 --- a/nptl/pthread_create.c +++ b/nptl/pthread_create.c @@ -487,8 +487,16 @@ start_thread (void *arg) /* 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); + because atexit handlers must not run with signals blocked. + + Do not block SIGSETXID. The setxid handshake below expects the + signal to be delivered. (SIGSETXID cannot run application code, + nor does it use pthread_kill.) Reuse the pd->sigmask space for + computing the signal mask, to save stack space. */ + __sigfillset (&pd->sigmask); + __sigdelset (&pd->sigmask, SIGSETXID); + INTERNAL_SYSCALL_CALL (rt_sigprocmask, SIG_BLOCK, &pd->sigmask, NULL, + __NSIG_BYTES); /* Tell __pthread_kill_internal that this thread is about to exit. If there is a __pthread_kill_internal in progress, this delays diff --git a/sysdeps/pthread/Makefile b/sysdeps/pthread/Makefile index 48dba717a1..d4bd2d4e3e 100644 --- a/sysdeps/pthread/Makefile +++ b/sysdeps/pthread/Makefile @@ -118,6 +118,7 @@ tests += tst-cnd-basic tst-mtx-trylock tst-cnd-broadcast \ tst-unload \ tst-unwind-thread \ tst-pt-vfork1 tst-pt-vfork2 tst-vfork1x tst-vfork2x \ + tst-pthread-setuid-loop \ tst-pthread_cancel-exited \ tst-pthread_cancel-select-loop \ tst-pthread_kill-exited \ diff --git a/sysdeps/pthread/tst-pthread-setuid-loop.c b/sysdeps/pthread/tst-pthread-setuid-loop.c new file mode 100644 index 0000000000..fda2a49b7f --- /dev/null +++ b/sysdeps/pthread/tst-pthread-setuid-loop.c @@ -0,0 +1,61 @@ +/* Test that setuid, pthread_create, thread exit do not deadlock (bug 28361). + 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 + +/* How many threads to launch during each iteration. */ +enum { threads = 4 }; + +/* How many iterations to perform. This value seems to reproduce + bug 28361 in a bout one in three runs. */ +enum { iterations = 5000 }; + +/* Cache of the real user ID used by setuid_thread. */ +static uid_t uid; + +/* Start routine for the threads. */ +static void * +setuid_thread (void *closure) +{ + TEST_COMPARE (setuid (uid), 0); + return NULL; +} + +static int +do_test (void) +{ + /* The setxid machinery is still invoked even if the UID is + unchanged. (The kernel might reset other credentials as part of + the system call.) */ + uid = getuid (); + + for (int i = 0; i < iterations; ++i) + { + pthread_t thread_ids[threads]; + for (int j = 0; j < threads; ++j) + thread_ids[j] = xpthread_create (NULL, setuid_thread, NULL); + for (int j = 0; j < threads; ++j) + xpthread_join (thread_ids[j]); + } + + return 0; +} + +#include