From f9011787497a276f84ef79ae233992692a626dc7 Mon Sep 17 00:00:00 2001 From: Samuel Thibault Date: Wed, 27 May 2020 23:42:24 +0000 Subject: [PATCH] hurd: Fix pselect atomicity 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 b140dab6c30..69a415c02c1 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 a2741bb7c82..4d819d9af23 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) { -- 2.39.5