2000-03-13 Roland McGrath <roland@baalperazim.frob.com>

	* stubs.c (struct msg_sig_post_request): New type, broken out of
	send_signal, where it was anonymous.
	(struct msg_spec): Type removed.
	(blocking_message_send): Change type to any_t -> any_t.
	Don't call cthread_wire; it's a no-op in GNU anyway.
	Just return instead of calling cthread_exit.
	Argument is now a pointer to struct msg_sig_post_request.
	Check mach_msg return value for sanity assert.
	(send_signal): Make MESSAGE auto instead of static, use new type name.
	This problem tracked down by Mark Kettenis <kettenis@gnu.org>.

1 files changed, 120 insertions, 88 deletions

diff --git a/proc/stubs.c b/proc/stubs.c
index f95bdfa5..de3a9b11 100644
--- a/proc/stubs.c
+++ b/proc/stubs.c
@@ -1,5 +1,5 @@
 /* By-hand stubs for some RPC calls
-   Copyright (C) 1994, 1996, 1999 Free Software Foundation
+   Copyright (C) 1994,96,99,2000 Free Software Foundation, Inc.
 
    This program is free software; you can redistribute it and/or
    modify it under the terms of the GNU General Public License as
@@ -20,28 +20,55 @@
 #include <hurd/hurd_types.h>
 #include <mach/message.h>
 #include <string.h>
+#include <assert.h>
 
 #include "proc.h"
 
 /* From hurd/msg.defs: */
 #define RPCID_SIG_POST 23000
 
-struct msg_spec
+
+struct msg_sig_post_request
 {
-  int len;
-  void *contents;
+  mach_msg_header_t head;
+  mach_msg_type_t signaltype;
+  int signal;
+  mach_msg_type_t sigcode_type;
+  natural_t sigcode;
+  mach_msg_type_t refporttype;
+  mach_port_t refport;
 };
 
-/* Send the Mach message indicated by msg_spec; call cthread_exit
+/* Send the Mach message indicated by msg_spec.   call cthread_exit
    when it has been delivered. */
-static void
-blocking_message_send (struct msg_spec *message)
+static any_t
+blocking_message_send (any_t arg)
 {
-  cthread_wire ();
-  mach_msg ((mach_msg_header_t *)message->contents, MACH_SEND_MSG, 
-	    message->len, 0, MACH_PORT_NULL, MACH_MSG_TIMEOUT_NONE, 
-	    MACH_PORT_NULL);
-  cthread_exit (0);
+  struct msg_sig_post_request *const req = arg;
+  error_t err;
+
+  err = mach_msg (&req->head, MACH_SEND_MSG, sizeof *req, 0,
+		  MACH_PORT_NULL, MACH_MSG_TIMEOUT_NONE, MACH_PORT_NULL);
+
+  switch (err)
+    {
+      /* These are the codes that mean a pseudo-receive modified
+	 the message buffer and we might need to clean up the send rights.
+	 None of them should be possible in our usage.  */
+    case MACH_SEND_TIMED_OUT:
+    case MACH_SEND_INTERRUPTED:
+    case MACH_SEND_INVALID_NOTIFY:
+    case MACH_SEND_NO_NOTIFY:
+    case MACH_SEND_NOTIFY_IN_PROGRESS:
+      assert_perror (err);
+      break;
+
+    default:			/* Other errors are safe to ignore.  */
+      break;
+    }
+
+
+  return 0;
 }
 
 /* Send signal SIGNO to MSGPORT with REFPORT as reference.  Don't
@@ -53,84 +80,89 @@ send_signal (mach_port_t msgport,
 {
   error_t err;
 
-  static struct
+  /* This message buffer might be modified by mach_msg in some error cases,
+     so we cannot safely use a shared static buffer.  */
+  struct msg_sig_post_request message =
+  {
     {
-      mach_msg_header_t head;
-      mach_msg_type_t signaltype;
-      int signal;
-      mach_msg_type_t sigcode_type;
-      natural_t sigcode;
-      mach_msg_type_t refporttype;
-      mach_port_t refport;
-    }
-  message = 
+      /* Message header: */
+      (MACH_MSGH_BITS_COMPLEX
+       | MACH_MSGH_BITS (MACH_MSG_TYPE_COPY_SEND,
+			 MACH_MSG_TYPE_MAKE_SEND_ONCE)), /* msgh_bits */
+      sizeof message,		/* msgh_size */
+      msgport,			/* msgh_remote_port */
+      MACH_PORT_NULL,		/* msgh_local_port */
+      0,			/* msgh_seqno */
+      RPCID_SIG_POST,		/* msgh_id */
+    },
     {
-      {
-	/* Message header: */
-	(MACH_MSGH_BITS_COMPLEX 
-	 | MACH_MSGH_BITS (MACH_MSG_TYPE_COPY_SEND,
-			  MACH_MSG_TYPE_MAKE_SEND_ONCE)), /* msgh_bits */
-	sizeof message,		/* msgh_size */
-	0,			/* msgh_remote_port */
-	MACH_PORT_NULL,		/* msgh_local_port */
-	0,			/* msgh_seqno */
-	RPCID_SIG_POST,		/* msgh_id */
-      },
-      {
-	/* Type descriptor for signo */
-	MACH_MSG_TYPE_INTEGER_32, /* msgt_name */
-	32,			/* msgt_size */
-	1,			/* msgt_number */
-	1,			/* msgt_inline */
-	0,			/* msgt_longform */
-	0,			/* msgt_deallocate */
-	0,			/* msgt_unused */
-      },
-      /* Signal number */
-      0,
-      /* Type descriptor for sigcode */
-      {
-	MACH_MSG_TYPE_INTEGER_32, /* msgt_name */
-	32,			/* msgt_size */
-	1,			/* msgt_number */
-	1,			/* msgt_inline */
-	0,			/* msgt_longform */
-	0,			/* msgt_deallocate */
-	0,			/* msgt_unused */
-      },
-      /* Sigcode */
-      0,
-      {
-	/* Type descriptor for refport */
-	MACH_MSG_TYPE_COPY_SEND, /* msgt_name */
-	32,			/* msgt_size */
-	1,			/* msgt_number */
-	1,			/* msgt_inline */
-	0,			/* msgt_longform */
-	0,			/* msgt_deallocate */
-	0,			/* msgt_unused */
-      },
-      /* Reference port */
-      MACH_PORT_NULL,
-    };
-  
-  message.head.msgh_remote_port = msgport;
-  message.signal = signal;
-  message.refport = refport;
-
-  err = mach_msg((mach_msg_header_t *)&message, 
-		 MACH_SEND_MSG|MACH_SEND_TIMEOUT, sizeof message, 0,
-		 MACH_PORT_NULL, 0, MACH_PORT_NULL);
-
-  if (err == MACH_SEND_TIMEOUT)
+      /* Type descriptor for signo */
+      MACH_MSG_TYPE_INTEGER_32, /* msgt_name */
+      32,			/* msgt_size */
+      1,			/* msgt_number */
+      1,			/* msgt_inline */
+      0,			/* msgt_longform */
+      0,			/* msgt_deallocate */
+      0,			/* msgt_unused */
+    },
+    /* Signal number */
+    signal,
+    /* Type descriptor for sigcode */
+    {
+      MACH_MSG_TYPE_INTEGER_32, /* msgt_name */
+      32,			/* msgt_size */
+      1,			/* msgt_number */
+      1,			/* msgt_inline */
+      0,			/* msgt_longform */
+      0,			/* msgt_deallocate */
+      0,			/* msgt_unused */
+    },
+    /* Sigcode */
+    0,
     {
-      struct msg_spec *msg_spec = malloc (sizeof (struct msg_spec));
-      
-      msg_spec->len = sizeof message;
-      msg_spec->contents = malloc (sizeof message);
-      bcopy (&message, msg_spec->contents, sizeof message);
-      
-      cthread_detach (cthread_fork ((cthread_fn_t) blocking_message_send,
-				    msg_spec));
+      /* Type descriptor for refport */
+      MACH_MSG_TYPE_COPY_SEND, /* msgt_name */
+      32,			/* msgt_size */
+      1,			/* msgt_number */
+      1,			/* msgt_inline */
+      0,			/* msgt_longform */
+      0,			/* msgt_deallocate */
+      0,			/* msgt_unused */
+    },
+    refport
+  };
+
+  err = mach_msg ((mach_msg_header_t *)&message,
+		  MACH_SEND_MSG|MACH_SEND_TIMEOUT, sizeof message, 0,
+		  MACH_PORT_NULL, 0, MACH_PORT_NULL);
+  switch (err)
+    {
+    case MACH_SEND_TIMED_OUT:
+      /* The send could not complete immediately, and we do not want to
+	 block.  We'll fork off another thread to do the blocking send.
+	 The message buffer was modified by a pseudo-receive operation, so
+	 we need to copy its modified contents into a malloc'd buffer.  */
+      {
+	struct msg_sig_post_request *copy = malloc (sizeof *copy);
+	if (copy)
+	  {
+	    memcpy (copy, &message, sizeof message);
+	    cthread_detach (cthread_fork (blocking_message_send, copy));
+	  }
+	break;
+      }
+
+      /* These are the other codes that mean a pseudo-receive modified
+	 the message buffer and we might need to clean up the send rights.
+	 None of them should be possible in our usage.  */
+    case MACH_SEND_INTERRUPTED:
+    case MACH_SEND_INVALID_NOTIFY:
+    case MACH_SEND_NO_NOTIFY:
+    case MACH_SEND_NOTIFY_IN_PROGRESS:
+      assert_perror (err);
+      break;
+
+    default:			/* Other errors are safe to ignore.  */
+      break;
     }
 }