From patchwork Thu May 28 09:16:23 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Samuel Thibault X-Patchwork-Id: 39374 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 08E83386F82F; Thu, 28 May 2020 09:18:43 +0000 (GMT) X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from hera.aquilenet.fr (hera.aquilenet.fr [IPv6:2a0c:e300::1]) by sourceware.org (Postfix) with ESMTPS id 7CCDD386F430 for ; Thu, 28 May 2020 09:18:39 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 7CCDD386F430 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=ens-lyon.org Authentication-Results: sourceware.org; spf=fail smtp.mailfrom=samuel.thibault@ens-lyon.org Received: from localhost (localhost [127.0.0.1]) by hera.aquilenet.fr (Postfix) with ESMTP id A64864133; Thu, 28 May 2020 11:18:36 +0200 (CEST) X-Virus-Scanned: Debian amavisd-new at aquilenet.fr Received: from hera.aquilenet.fr ([127.0.0.1]) by localhost (hera.aquilenet.fr [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id YMOuyXRbVHrF; Thu, 28 May 2020 11:18:35 +0200 (CEST) Received: from function (lfbn-bor-1-797-11.w86-234.abo.wanadoo.fr [86.234.239.11]) by hera.aquilenet.fr (Postfix) with ESMTPSA id 18B8840FA; Thu, 28 May 2020 11:18:35 +0200 (CEST) Received: from samy by function with local (Exim 4.93) (envelope-from ) id 1jeEeF-004Dey-K4; Thu, 28 May 2020 11:16:23 +0200 From: Samuel Thibault To: libc-alpha@sourceware.org Subject: [hurd,commited] hurd: Fix pselect atomicity Date: Thu, 28 May 2020 11:16:23 +0200 Message-Id: <20200528091623.1005664-1-samuel.thibault@ens-lyon.org> X-Mailer: git-send-email 2.26.2 MIME-Version: 1.0 X-Spam-Status: No, score=-15.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, JMQ_SPF_NEUTRAL, KAM_DMARC_STATUS, SPF_HELO_PASS, SPF_NEUTRAL, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) 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: , Cc: commit-hurd@gnu.org Errors-To: libc-alpha-bounces@sourceware.org Sender: "Libc-alpha" In case the signal arrives before the __mach_msg call, we need to catch between the sigprocmask call and the __mach_msg call. Let's just reuse the support for sigsuspend to make the signal send a message that our __mach_msg call will just receive. * hurd/hurdselect.c (_hurd_select): Add sigport and ss variables. When sigmask is not NULL, create a sigport port and register as ss->suspended. Add it to the portset. When we receive a message on it, set error to EINTR. Clean up sigport and portset appropriately. * hurd/hurdsig.c (wake_sigsuspend): Note that pselect also uses it. --- hurd/hurdselect.c | 76 ++++++++++++++++++++++++++++++++++++++++++----- hurd/hurdsig.c | 4 +-- 2 files changed, 70 insertions(+), 10 deletions(-) diff --git a/hurd/hurdselect.c b/hurd/hurdselect.c index b140dab6c3..69a415c02c 100644 --- a/hurd/hurdselect.c +++ b/hurd/hurdselect.c @@ -48,7 +48,7 @@ _hurd_select (int nfds, const struct timespec *timeout, const sigset_t *sigmask) { int i; - mach_port_t portset; + mach_port_t portset, sigport; int got, ready; error_t err; fd_set rfds, wfds, xfds; @@ -66,6 +66,7 @@ _hurd_select (int nfds, int error; } d[nfds]; sigset_t oset; + struct hurd_sigstate *ss; union typeword /* Use this to avoid unkosher casts. */ { @@ -115,8 +116,30 @@ _hurd_select (int nfds, reply_msgid = IO_SELECT_TIMEOUT_REPLY_MSGID; } - if (sigmask && __sigprocmask (SIG_SETMASK, sigmask, &oset)) - return -1; + if (sigmask) + { + /* Add a port to the portset for the case when we get the signal even + before calling __mach_msg. */ + + sigport = __mach_reply_port (); + + ss = _hurd_self_sigstate (); + _hurd_sigstate_lock (ss); + /* And tell the signal thread to message us when a signal arrives. */ + ss->suspended = sigport; + _hurd_sigstate_unlock (ss); + + if (__sigprocmask (SIG_SETMASK, sigmask, &oset)) + { + _hurd_sigstate_lock (ss); + ss->suspended = MACH_PORT_NULL; + _hurd_sigstate_unlock (ss); + __mach_port_destroy (__mach_task_self (), sigport); + return -1; + } + } + else + sigport = MACH_PORT_NULL; if (pollfds) { @@ -188,6 +211,8 @@ _hurd_select (int nfds, d[i].io_port); __mutex_unlock (&_hurd_dtable_lock); HURD_CRITICAL_END; + if (sigmask) + __sigprocmask (SIG_SETMASK, &oset, NULL); errno = err; return -1; } @@ -277,9 +302,14 @@ _hurd_select (int nfds, /* Send them all io_select request messages. */ if (firstfd == -1) - /* But not if there were no ports to deal with at all. - We are just a pure timeout. */ - portset = __mach_reply_port (); + { + if (sigport == MACH_PORT_NULL) + /* But not if there were no ports to deal with at all. + We are just a pure timeout. */ + portset = __mach_reply_port (); + else + portset = sigport; + } else { portset = MACH_PORT_NULL; @@ -298,7 +328,7 @@ _hurd_select (int nfds, ts, type); if (!err) { - if (firstfd == lastfd) + if (firstfd == lastfd && sigport == MACH_PORT_NULL) /* When there's a single descriptor, we don't need a portset, so just pretend we have one, but really use the single reply port. */ @@ -329,6 +359,16 @@ _hurd_select (int nfds, } _hurd_port_free (&d[i].cell->port, &d[i].ulink, d[i].io_port); } + + if (got == 0 && sigport != MACH_PORT_NULL) + { + if (portset == MACH_PORT_NULL) + /* Create the portset to receive the signal message on. */ + __mach_port_allocate (__mach_task_self (), MACH_PORT_RIGHT_PORT_SET, + &portset); + /* Put the signal reply port in the port set. */ + __mach_port_move_member (__mach_task_self (), sigport, portset); + } } /* GOT is the number of replies (or errors), while READY is the number of @@ -404,6 +444,16 @@ _hurd_select (int nfds, { MACH_MSG_TYPE_INTEGER_T, sizeof (integer_t) * 8, 1, 1, 0, 0 } }; #endif + + if (sigport != MACH_PORT_NULL && sigport == msg.head.msgh_local_port) + { + /* We actually got interrupted by a signal before + __mach_msg; poll for further responses and then + return quickly. */ + err = EINTR; + goto poll; + } + if (msg.head.msgh_id == reply_msgid && msg.head.msgh_size >= sizeof msg.error && !(msg.head.msgh_bits & MACH_MSGH_BITS_COMPLEX) @@ -492,7 +542,17 @@ _hurd_select (int nfds, for (i = firstfd; i <= lastfd; ++i) if (d[i].reply_port != MACH_PORT_NULL) __mach_port_destroy (__mach_task_self (), d[i].reply_port); - if (firstfd == -1 || (firstfd != lastfd && portset != MACH_PORT_NULL)) + + if (sigport != MACH_PORT_NULL) + { + _hurd_sigstate_lock (ss); + ss->suspended = MACH_PORT_NULL; + _hurd_sigstate_unlock (ss); + __mach_port_destroy (__mach_task_self (), sigport); + } + + if ((firstfd == -1 && sigport == MACH_PORT_NULL) + || ((firstfd != lastfd || sigport != MACH_PORT_NULL) && portset != MACH_PORT_NULL)) /* Destroy PORTSET, but only if it's not actually the reply port for a single descriptor (in which case it's destroyed in the previous loop; not doing it here is just a bit more efficient). */ diff --git a/hurd/hurdsig.c b/hurd/hurdsig.c index a2741bb7c8..4d819d9af2 100644 --- a/hurd/hurdsig.c +++ b/hurd/hurdsig.c @@ -564,8 +564,8 @@ abort_all_rpcs (int signo, struct machine_thread_all_state *state, int live) } } -/* Wake up any sigsuspend call that is blocking SS->thread. SS must be - locked. */ +/* Wake up any sigsuspend or pselect call that is blocking SS->thread. SS must + be locked. */ static void wake_sigsuspend (struct hurd_sigstate *ss) {