From patchwork Wed May 17 19:14:32 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sergey Bugaev X-Patchwork-Id: 1782935 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=sourceware.org (client-ip=2620:52:3:1:0:246e:9693:128c; helo=sourceware.org; envelope-from=libc-alpha-bounces+incoming=patchwork.ozlabs.org@sourceware.org; receiver=) Authentication-Results: legolas.ozlabs.org; dkim=pass (1024-bit key; secure) header.d=sourceware.org header.i=@sourceware.org header.a=rsa-sha256 header.s=default header.b=QZ+L8BSG; dkim-atps=neutral Received: from sourceware.org (server2.sourceware.org [IPv6:2620:52:3:1:0:246e:9693:128c]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-384) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4QM2rK4Q4Mz20dq for ; Thu, 18 May 2023 05:15:45 +1000 (AEST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 9879E38346B7 for ; Wed, 17 May 2023 19:15:43 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 9879E38346B7 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1684350943; bh=5dPGJhcZg6lHzRIz+7HN9+JEHTfCzz8yFj8r2ixXrLo=; h=To:Subject:Date:In-Reply-To:References:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=QZ+L8BSGPULK8LRZooGLf53NRFC6mgOxmsmzBSke+lkZYSHsjhNBByTsUOE9QkJ8H YOlK1sciKW1kLcq64PG2B63Jmc3iASIAsKb9J77PyYPkijlmzLSvmyomv0DuNgjaSS kvBTKYpIwOQt2KCDcrcs/a/jGSlCkQNIVsyOXWtU= X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from mail-lf1-x136.google.com (mail-lf1-x136.google.com [IPv6:2a00:1450:4864:20::136]) by sourceware.org (Postfix) with ESMTPS id D0995385696A for ; Wed, 17 May 2023 19:14:48 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org D0995385696A Received: by mail-lf1-x136.google.com with SMTP id 2adb3069b0e04-4ec8eca56cfso1395904e87.0 for ; Wed, 17 May 2023 12:14:48 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1684350887; x=1686942887; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=5dPGJhcZg6lHzRIz+7HN9+JEHTfCzz8yFj8r2ixXrLo=; b=TNUZ1jWg9gvfDdTzOt7/OZTI6RR4FAmNivAaszvj560cVP+LeTATxoUBz4v07hRkLZ 84VGKrM4nv2dzEIO1KMb4DvmlMw6YsawT/oKFocAkYja3tJkuuh2v/kWjsS9ObeXPeUt lwPlsLKoMjCkwA1pCBrZL9taH+wCKvU9QgqVyZSMqZjkLvruY0pVJXdxOkveuG8VH8Z7 nCSu98DpgEYHn6N9A4Gq/pwYSfgvhijXmp9frIJ2YLkCKPQb7QnJ8X3S2QXNCq+CAPPC 7ZZMIhCVBa1aSYWnPMUJ2F3i7uleF7ttqEAxnzfxvQ8vogsfnvHkYn6oIyDXEKXqNhjI RWgQ== X-Gm-Message-State: AC+VfDwq7DS4CZ76pk5isOr+aPRQUFRVyg0Tp7NgO1hJW/MniOQ4+pQ5 dn2QckvWH76z0/7vT9BIBdv468Cv5sw= X-Google-Smtp-Source: ACHHUZ6CelYMAgmTxFxMvgTPwPy+BAoOzA4VSJ//FwFi2hKfuHNR5d/kyF7Mwysgz2mpgNG0ucqqwg== X-Received: by 2002:ac2:4143:0:b0:4f3:895f:f3f8 with SMTP id c3-20020ac24143000000b004f3895ff3f8mr411845lfi.16.1684350886796; Wed, 17 May 2023 12:14:46 -0700 (PDT) Received: from surface-pro-6.. ([194.190.106.50]) by smtp.gmail.com with ESMTPSA id o18-20020ac24352000000b004eed8de597csm3415744lfl.32.2023.05.17.12.14.45 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 17 May 2023 12:14:45 -0700 (PDT) To: libc-alpha@sourceware.org, bug-hurd@gnu.org Subject: [RFC PATCH 06/10] hurd: Make sure to not use tcb->self Date: Wed, 17 May 2023 22:14:32 +0300 Message-Id: <20230517191436.73636-7-bugaevc@gmail.com> X-Mailer: git-send-email 2.40.1 In-Reply-To: <20230517191436.73636-1-bugaevc@gmail.com> References: <20230517191436.73636-1-bugaevc@gmail.com> MIME-Version: 1.0 X-Spam-Status: No, score=-11.1 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) 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: Sergey Bugaev via Libc-alpha From: Sergey Bugaev Reply-To: Sergey Bugaev Errors-To: libc-alpha-bounces+incoming=patchwork.ozlabs.org@sourceware.org Sender: "Libc-alpha" Unlike sigstate->thread, tcb->self did not hold a Mach port reference on the thread port it names. This means that the port can be deallocated, and the name reused for something else, without anyone noticing. Using tcb->self will then lead to port use-after-free. Fortunately nothing was accessing tcb->self, other than it being intially set to then-valid thread port name upon TCB initialization. To assert that this keeps being the case without altering TCB layout, rename self -> self_do_not_use, and stop initializing it. Also, do not (re-)allocate a whole separate and unused stack for the main thread, and just exit __pthread_setup early in this case. Found upon attempting to use tcb->self and getting unexpected crashes. Signed-off-by: Sergey Bugaev --- sysdeps/mach/hurd/i386/tls.h | 3 +-- sysdeps/mach/hurd/x86/htl/pt-setup.c | 34 ++++++++++------------------ sysdeps/mach/hurd/x86_64/tls.h | 3 +-- 3 files changed, 14 insertions(+), 26 deletions(-) diff --git a/sysdeps/mach/hurd/i386/tls.h b/sysdeps/mach/hurd/i386/tls.h index e124fb10..ba283008 100644 --- a/sysdeps/mach/hurd/i386/tls.h +++ b/sysdeps/mach/hurd/i386/tls.h @@ -32,7 +32,7 @@ typedef struct { void *tcb; /* Points to this structure. */ dtv_t *dtv; /* Vector of pointers to TLS data. */ - thread_t self; /* This thread's control port. */ + thread_t self_do_not_use; /* This thread's control port. */ int multiple_threads; uintptr_t sysinfo; uintptr_t stack_guard; @@ -419,7 +419,6 @@ _hurd_tls_new (thread_t child, tcbhead_t *tcb) HURD_TLS_DESC_DECL (desc, tcb); tcb->tcb = tcb; - tcb->self = child; if (HURD_SEL_LDT (sel)) err = __i386_set_ldt (child, sel, &desc, 1); diff --git a/sysdeps/mach/hurd/x86/htl/pt-setup.c b/sysdeps/mach/hurd/x86/htl/pt-setup.c index 3abd92b2..686124d7 100644 --- a/sysdeps/mach/hurd/x86/htl/pt-setup.c +++ b/sysdeps/mach/hurd/x86/htl/pt-setup.c @@ -19,6 +19,7 @@ #include #include #include +#include #include @@ -76,35 +77,24 @@ __pthread_setup (struct __pthread *thread, void *), void *(*start_routine) (void *), void *arg) { - tcbhead_t *tcb; error_t err; - mach_port_t ktid; - thread->mcontext.pc = entry_point; - thread->mcontext.sp = stack_setup (thread, start_routine, arg); - - ktid = __mach_thread_self (); - if (thread->kernel_thread == ktid) + if (thread->kernel_thread == hurd_thread_self ()) /* Fix up the TCB for the main thread. The C library has already installed a TCB, which we want to keep using. This TCB must not be freed so don't register it in the thread structure. On the other hand, it's not yet possible to reliably release a TCB. - Leave the unused one registered so that it doesn't leak. The - only thing left to do is to correctly set the `self' member in - the already existing TCB. */ - tcb = THREAD_SELF; - else - { - err = __thread_set_pcsptp (thread->kernel_thread, - 1, thread->mcontext.pc, - 1, thread->mcontext.sp, - 1, thread->tcb); - assert_perror (err); - tcb = thread->tcb; - } - __mach_port_deallocate (__mach_task_self (), ktid); + Leave the unused one registered so that it doesn't leak. */ + return 0; + + thread->mcontext.pc = entry_point; + thread->mcontext.sp = stack_setup (thread, start_routine, arg); - tcb->self = thread->kernel_thread; + err = __thread_set_pcsptp (thread->kernel_thread, + 1, thread->mcontext.pc, + 1, thread->mcontext.sp, + 1, thread->tcb); + assert_perror (err); return 0; } diff --git a/sysdeps/mach/hurd/x86_64/tls.h b/sysdeps/mach/hurd/x86_64/tls.h index 1274723a..35dcef44 100644 --- a/sysdeps/mach/hurd/x86_64/tls.h +++ b/sysdeps/mach/hurd/x86_64/tls.h @@ -35,7 +35,7 @@ typedef struct { void *tcb; /* Points to this structure. */ dtv_t *dtv; /* Vector of pointers to TLS data. */ - thread_t self; /* This thread's control port. */ + thread_t self_do_no_use; /* This thread's control port. */ int __glibc_padding1; int multiple_threads; int gscope_flag; @@ -158,7 +158,6 @@ _hurd_tls_new (thread_t child, tcbhead_t *tcb) struct i386_fsgs_base_state state; tcb->tcb = tcb; - tcb->self = child; /* Install the TCB address into FS base. */ state.fs_base = (uintptr_t) tcb;