git-intr-msg

commit 32ad5b3328e0ce53ca27e185a89ca44c1d0acd0c
Author: Samuel Thibault <samuel.thibault@ens-lyon.org>
Date:   Tue Oct 9 23:40:09 2018 +0200

    hurd: Fix race between calling RPC and handling a signal

            * sysdeps/mach/hurd/i386/intr-msg.h (INTR_MSG_TRAP): Make
            _hurd_intr_rpc_msg_about_to global point to start of controlled
            assembly snippet. Make it check canceled flag again.
            * hurd/hurdsig.c (_hurdsig_abort_rpcs): Only mutate thread if it passed the
            _hurd_intr_rpc_msg_about_to point.
            * hurd/intr-msg.c (_hurd_intr_rpc_mach_msg): Remove comment on mutation
            issue.

Gbp-Pq: Topic hurd-i386
Gbp-Pq: Name git-intr-msg.diff

diff --git a/hurd/hurdsig.c b/hurd/hurdsig.c
index dab6fbc9892ec6384c63e742411438c74a1b6ac4..404ead6b61b84656b5adfbafcc9e2a5bb9f12460 100644 (file)
--- a/hurd/hurdsig.c
+++ b/hurd/hurdsig.c
@@ -454,6 +454,7 @@ _hurdsig_abort_rpcs (struct hurd_sigstate *ss, int signo, int sigthread,
                     struct machine_thread_all_state *state, int *state_change,
                     void (*reply) (void))
 {
+  extern const void _hurd_intr_rpc_msg_about_to;
   extern const void _hurd_intr_rpc_msg_in_trap;
   mach_port_t rcv_port = MACH_PORT_NULL;
   mach_port_t intr_port;
@@ -469,7 +470,8 @@ _hurdsig_abort_rpcs (struct hurd_sigstate *ss, int signo, int sigthread,
      receive completes immediately or aborts.  */
   abort_thread (ss, state, reply);
 
-  if (state->basic.PC < (natural_t) &_hurd_intr_rpc_msg_in_trap)
+  if (state->basic.PC >= (natural_t) &_hurd_intr_rpc_msg_about_to &&
+      state->basic.PC <  (natural_t) &_hurd_intr_rpc_msg_in_trap)
     {
       /* The thread is about to do the RPC, but hasn't yet entered
         mach_msg.  Mutate the thread's state so it knows not to try

diff --git a/hurd/intr-msg.c b/hurd/intr-msg.c
index ec8dc5e1fb7593ce85577c8eedc3ee538dd91402..9ddae627c4bb1c5afd55d2d1be73d37e092143d2 100644 (file)
--- a/hurd/intr-msg.c
+++ b/hurd/intr-msg.c
@@ -114,23 +114,10 @@ _hurd_intr_rpc_mach_msg (mach_msg_header_t *msg,
 
  message:
 
-  /* XXX
-     At all points here (once SS->intr_port is set), the signal thread
-     thinks we are "about to enter the syscall", and might mutate our
-     return-value register.  This is bogus.
-   */
-
-  if (ss->cancel)
-    {
-      /* We have been cancelled.  Don't do an RPC at all.  */
-      ss->intr_port = MACH_PORT_NULL;
-      ss->cancel = 0;
-      return EINTR;
-    }
-
   /* Note that the signal trampoline code might modify our OPTION!  */
   err = INTR_MSG_TRAP (msg, option, send_size,
-                      rcv_size, rcv_name, timeout, notify);
+                      rcv_size, rcv_name, timeout, notify,
+                      &ss->cancel, &ss->intr_port);
 
   switch (err)
     {

diff --git a/sysdeps/mach/hurd/i386/intr-msg.h b/sysdeps/mach/hurd/i386/intr-msg.h
index 64f05f8c4e8a086e57e201e796b865cec1407c53..7788c3b8923bf7671825b2aca879d013963bdae2 100644 (file)
--- a/sysdeps/mach/hurd/i386/intr-msg.h
+++ b/sysdeps/mach/hurd/i386/intr-msg.h
@@ -20,21 +20,30 @@
 /* Note that we must mark OPTION and TIMEOUT as outputs of this operation,
    to indicate that the signal thread might mutate them as part
    of sending us to a signal handler.  */
-#define INTR_MSG_TRAP(msg, option, send_size, rcv_size, rcv_name, timeout, notify) \
+
+/* After _hurd_intr_rpc_msg_about_to we need to make a last check of cancel, in
+   case we got interrupted right before _hurd_intr_rpc_msg_about_to.  */
+#define INTR_MSG_TRAP(msg, option, send_size, rcv_size, rcv_name, timeout, notify, cancel_p, intr_port_p) \
 ({                                                                           \
   error_t err;                                                               \
-  asm (".globl _hurd_intr_rpc_msg_do_trap\n"                                 \
-       ".globl _hurd_intr_rpc_msg_in_trap\n"                                 \
+  asm (".globl _hurd_intr_rpc_msg_about_to\n"                                \
        ".globl _hurd_intr_rpc_msg_cx_sp\n"                                   \
+       ".globl _hurd_intr_rpc_msg_do_trap\n"                                 \
+       ".globl _hurd_intr_rpc_msg_in_trap\n"                                 \
        ".globl _hurd_intr_rpc_msg_sp_restored\n"                             \
-       "                               movl %%esp, %%ecx\n"                  \
-       "                               leal %3, %%esp\n"                     \
+       "_hurd_intr_rpc_msg_about_to:   cmpl $0, %5\n"                        \
+       "                               jz _hurd_intr_rpc_msg_do\n"           \
+       "                               movl $0, %3\n"                        \
+       "                               movl %6, %%eax\n"                     \
+       "                               jmp _hurd_intr_rpc_msg_sp_restored\n" \
+       "_hurd_intr_rpc_msg_do:         movl %%esp, %%ecx\n"                  \
+       "                               leal %4, %%esp\n"                     \
        "_hurd_intr_rpc_msg_cx_sp:      movl $-25, %%eax\n"                   \
        "_hurd_intr_rpc_msg_do_trap:    lcall $7, $0 # status in %0\n"        \
        "_hurd_intr_rpc_msg_in_trap:    movl %%ecx, %%esp\n"                  \
        "_hurd_intr_rpc_msg_sp_restored:"                                     \
-       : "=a" (err), "+m" (option), "+m" (timeout)                           \
-       : "m" ((&msg)[-1])                                                    \
+       : "=a" (err), "+m" (option), "+m" (timeout), "=m" (*intr_port_p)              \
+       : "m" ((&msg)[-1]), "m" (*cancel_p), "i" (EINTR)                              \
        : "ecx");                                                             \
   err;                                                                       \
 })