[hurd,commited] hurd: Fix pselect atomicity

[hurd,commited] hurd: Fix pselect atomicity

[Samuel Thibault]

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)
 {
-- 
2.26.2