From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lf1-x12a.google.com (mail-lf1-x12a.google.com [IPv6:2a00:1450:4864:20::12a]) by sourceware.org (Postfix) with ESMTPS id 4CDFC3858425 for ; Thu, 13 Apr 2023 11:58:27 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 4CDFC3858425 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-lf1-x12a.google.com with SMTP id bi41so658048lfb.7 for ; Thu, 13 Apr 2023 04:58:27 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1681387105; x=1683979105; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=PgKAXx+SVYWtGo2ZgUUCvmN1SWQPP2NR+kJI5j3X748=; b=bgWU5g35Fe0iP9koWlvkn19eoX1i+faF2BJxTjV/+xJ0m8irN+iwqf6AMS4m5AhUyK Wxo9kLRcNifXqouTr9A9zmEoLBoDXXWIbJWCShYcwBNS/9olk8TP9t/eCpJO9Dq1NtVa c895jVSrw5WBkPlcFKfglqYshHhGd3HZczY9hlc22u2iQNKIl67sANQrk9pD8+/sO6kr YkhIKbIYhAjN4YN3cSvrbr5ug2hrF92FhDl9ihVxt2pFojmIslC1yQlhXPDVDKdWADDh jFF4nr7gjotQrDLtBLD7rAJHDb5oD+lUwbwA4GyVKUpsKm9WqzGUdQvYE2NPLd3nBHdd rksA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1681387105; x=1683979105; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=PgKAXx+SVYWtGo2ZgUUCvmN1SWQPP2NR+kJI5j3X748=; b=f/XrlCZufEm+COv9HsWin8+8bAF8rWBnYew7IG6A+yhmQH780S1vXcyLQfSwZtqKkj RqAPjR7q5A0eF95z+MyENRCv1KVyTVITz5khzWWXITo1cYAFZHl9OV/AHcpqXRLDqpm8 DW57omCAmliRnP0n9d94VmBM+tKF3liViOA/YzbE9LzE7BH8VTQaCd1/q/l61HU7XJ/w aclleqmHOlNWLgnCC6EMVtR8MaPmhftaqQgo4lXtgJ2uyxOhxWep47+ij9kofRMLXtOd gX0qn0+C2lBOVPOqdztsrQQDzGdZUFgiUuGz0/1y+tqrl52A0c/BCJcR3307ZjzZG2xO 7IQA== X-Gm-Message-State: AAQBX9euRZEubS7kGVuKP2xE4UfqXS+ehI3g+95iXJlF58KyFwsipidu /jdl3aaaAFC4UCB7PRjdlVf8KFGc+9A= X-Google-Smtp-Source: AKy350admqq/JJxJg8CtgEsGFWqH9rJHR8hHXaDj+KfLPixS2mb1SRmCiKGGvLO1873rx6SujKxNWA== X-Received: by 2002:ac2:5614:0:b0:4ea:f617:e994 with SMTP id v20-20020ac25614000000b004eaf617e994mr615615lfd.60.1681387105094; Thu, 13 Apr 2023 04:58:25 -0700 (PDT) Received: from surface-pro-6.. ([194.190.106.50]) by smtp.gmail.com with ESMTPSA id u21-20020ac25195000000b004cb45148027sm280985lfi.203.2023.04.13.04.58.23 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 13 Apr 2023 04:58:24 -0700 (PDT) From: Sergey Bugaev To: libc-alpha@sourceware.org, bug-hurd@gnu.org, Samuel Thibault Cc: Sergey Bugaev Subject: [RFC PATCH glibc v2 26/34] hurd: Remove __hurd_local_reply_port Date: Thu, 13 Apr 2023 14:58:12 +0300 Message-Id: <20230413115812.267158-1-bugaevc@gmail.com> X-Mailer: git-send-email 2.39.2 In-Reply-To: <20230411201845.oias7lryrvm3cck7@begin> References: <20230411201845.oias7lryrvm3cck7@begin> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-11.0 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 autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Now that the signal code no longer accesses it, the only real user of it was mig-reply.c, so move the logic for managing the port there. If we're in SHARED and outside of rtld, we know that __LIBC_NO_TLS () always evaluates to 0, and a TLS reply port will always be used, not __hurd_reply_port0. Still, the compiler does not see that __hurd_reply_port0 is never used due to its address being taken. To deal with this, explicitly compile out __hurd_reply_port0 when we know we won't use it. Also, instead of accessing the port via THREAD_SELF->reply_port, this uses THREAD_GETMEM and THREAD_SETMEM directly, avoiding possible miscompilations. Signed-off-by: Sergey Bugaev --- Changes since v1: * Only deallocate the port in __mig_dealloc_reply_port if it's the same as the stored port, because of signals. * Add a comment detailing the reasoning. * Add two assertions: that ARG and PORT are exactly the same (unless PORT has been invalidated), and that we succeed deallocating PORT. NOTE: Completely untested! Do your own testing! You have been warned! hurd/hurd/threadvar.h | 7 ---- sysdeps/mach/hurd/mig-reply.c | 70 +++++++++++++++++++++++++++++------ 2 files changed, 58 insertions(+), 19 deletions(-) diff --git a/hurd/hurd/threadvar.h b/hurd/hurd/threadvar.h index f5c6a278..c476d988 100644 --- a/hurd/hurd/threadvar.h +++ b/hurd/hurd/threadvar.h @@ -29,11 +29,4 @@ extern unsigned long int __hurd_sigthread_stack_base; extern unsigned long int __hurd_sigthread_stack_end; -/* Store the MiG reply port reply port until we enable TLS. */ -extern mach_port_t __hurd_reply_port0; - -/* This returns either the TLS reply port variable, or a single-thread variable - when TLS is not initialized yet. */ -#define __hurd_local_reply_port (*(__LIBC_NO_TLS () ? &__hurd_reply_port0 : &THREAD_SELF->reply_port)) - #endif /* hurd/threadvar.h */ diff --git a/sysdeps/mach/hurd/mig-reply.c b/sysdeps/mach/hurd/mig-reply.c index 33ff260e..3fdee80e 100644 --- a/sysdeps/mach/hurd/mig-reply.c +++ b/sysdeps/mach/hurd/mig-reply.c @@ -17,22 +17,46 @@ #include #include -#include +#include /* These functions are called by MiG-generated code. */ +#if !defined (SHARED) || IS_IN (rtld) mach_port_t __hurd_reply_port0; +#endif + +static mach_port_t +get_reply_port (void) +{ +#if !defined (SHARED) || IS_IN (rtld) + if (__LIBC_NO_TLS ()) + return __hurd_reply_port0; +#endif + return THREAD_GETMEM (THREAD_SELF, reply_port); +} + +static void +set_reply_port (mach_port_t port) +{ +#if !defined (SHARED) || IS_IN (rtld) + if (__LIBC_NO_TLS ()) + __hurd_reply_port0 = port; + else +#endif + THREAD_SETMEM (THREAD_SELF, reply_port, port); +} /* Called by MiG to get a reply port. */ mach_port_t __mig_get_reply_port (void) { - if (__hurd_local_reply_port == MACH_PORT_NULL - || (&__hurd_local_reply_port != &__hurd_reply_port0 - && __hurd_local_reply_port == __hurd_reply_port0)) - __hurd_local_reply_port = __mach_reply_port (); - - return __hurd_local_reply_port; + mach_port_t port = get_reply_port (); + if (__glibc_unlikely (port == MACH_PORT_NULL)) + { + port = __mach_reply_port (); + set_reply_port (port); + } + return port; } weak_alias (__mig_get_reply_port, mig_get_reply_port) libc_hidden_def (__mig_get_reply_port) @@ -41,12 +65,34 @@ libc_hidden_def (__mig_get_reply_port) void __mig_dealloc_reply_port (mach_port_t arg) { - mach_port_t port = __hurd_local_reply_port; - __hurd_local_reply_port = MACH_PORT_NULL; /* So the mod_refs RPC won't use it. */ + error_t err; + mach_port_t port = get_reply_port (); + + set_reply_port (MACH_PORT_NULL); /* So the mod_refs RPC won't use it. */ + + /* Normally, ARG should be the same as PORT that we store. However, if a + signal has interrupted the RPC, the stored PORT has been deallocated and + reset to MACH_PORT_NULL (or possibly MACH_PORT_DEAD). In this case the + MIG routine still has the old name, which it passes to us here. We must + not deallocate (or otherwise touch) it, since it may be already allocated + to another port right. Fortunately MIG itself doesn't do anything with + the reply port on errors either, other than immediately calling this + function. + + And so: + 1. Assert that things are sane, i.e. and PORT is either invalid or same + as ARG. + 2. Only deallocate the name if our stored PORT still names it. In that + case we're sure the right has not been deallocated / the name reused. + */ + + if (!MACH_PORT_VALID (port)) + return; + assert (port == arg); - if (MACH_PORT_VALID (port)) - __mach_port_mod_refs (__mach_task_self (), port, - MACH_PORT_RIGHT_RECEIVE, -1); + err = __mach_port_mod_refs (__mach_task_self (), port, + MACH_PORT_RIGHT_RECEIVE, -1); + assert_perror (err); } weak_alias (__mig_dealloc_reply_port, mig_dealloc_reply_port) libc_hidden_def (__mig_dealloc_reply_port) -- 2.39.2