[PATCH hurd 2/2] libshouldbeinlibc: add safe port handling macros

[Justus Winter]

* libshouldbeinlibc/Makefile (SRCS, installhdrs): Add new file.
* libshouldbeinlibc/machx.h: New file.
---
 libshouldbeinlibc/Makefile |  2 ++
 libshouldbeinlibc/machx.h  | 73 ++++++++++++++++++++++++++++++++++++++++++++++
 trans/crash.c              | 29 +++++++++---------
 3 files changed, 89 insertions(+), 15 deletions(-)
 create mode 100644 libshouldbeinlibc/machx.h

diff --git a/libshouldbeinlibc/Makefile b/libshouldbeinlibc/Makefile
index 04c085b..76076f2 100644
--- a/libshouldbeinlibc/Makefile
+++ b/libshouldbeinlibc/Makefile
@@ -30,11 +30,13 @@ SRCS = termsize.c timefmt.c exec-reauth.c maptime-funcs.c \
        ugids-verify-auth.c nullauth.c \
        refcount.c \
        assert-backtrace.c \
+       machx.h \
 
 installhdrs = idvec.h timefmt.h maptime.h \
              wire.h portinfo.h portxlate.h cacheq.h ugids.h nullauth.h \
              refcount.h \
              assert-backtrace.h \
+             machx.h \
 
 installhdrsubdir = .
 
diff --git a/libshouldbeinlibc/machx.h b/libshouldbeinlibc/machx.h
new file mode 100644
index 0000000..6e727de
--- /dev/null
+++ b/libshouldbeinlibc/machx.h
@@ -0,0 +1,73 @@
+/* Safe right handling routines.
+
+   Copyright (C) 2016 Free Software Foundation, Inc.
+
+   This file is part of the GNU Hurd.
+
+   The GNU Hurd is free software; you can redistribute it and/or
+   modify it under the terms of the GNU General Public License as
+   published by the Free Software Foundation; either version 2, or (at
+   your option) any later version.
+
+   The GNU Hurd is distributed in the hope that it will be useful, but
+   WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.         See the GNU
+   General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with the GNU Hurd.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#ifndef __MACHX__
+#define __MACHX__
+
+#include <errno.h>
+#include <mach.h>
+#include <assert-backtrace.h>
+
+#define Mach_port_deallocate(TASK, NAME)                                \
+  ({                                                                    \
+    if (MACH_PORT_VALID (NAME))                                         \
+      {                                                                 \
+        error_t _Xerr = mach_port_deallocate ((TASK), (NAME));          \
+        if (_Xerr)                                                      \
+          {                                                             \
+            char _Xmessage[1024];                                       \
+            snprintf (_Xmessage, sizeof _Xmessage,                      \
+                      "error deallocating "#NAME": %s", strerror (_Xerr)); \
+            __print_fail_backtrace (_Xmessage, __FILE__, __LINE__,      \
+                                    __PRETTY_FUNCTION__);               \
+          }                                                             \
+        NAME = MACH_PORT_NULL;                                          \
+      }                                                                 \
+  })
+
+#define Mach_port_move(NAME)                    \
+  ({                                            \
+    mach_port_t _Xport = (NAME);                \
+    NAME = MACH_PORT_NULL;                      \
+    _Xport;                                     \
+  })
+
+#define Mach_port_check(NAME)                                           \
+  void _Mach_port_check_##NAME(char *_unused[] __attribute__ ((unused))) \
+  {                                                                     \
+  if (MACH_PORT_VALID (NAME))                                           \
+    __print_fail_backtrace (#NAME " leaked",                            \
+                            __FILE__, __LINE__, "Port leak detector");  \
+  }                                                                     \
+  char _Mach_port_check_x_##NAME[0]                                     \
+  __attribute__ ((unused, cleanup (_Mach_port_check_##NAME)))
+
+#define Mach_server_success(ERR)       ((ERR) == 0 || (ERR) == MIG_NO_REPLY)
+
+#define Mach_server_port_check(ERR, NAME)                               \
+  void _Mach_port_check_##NAME(char *_unused[] __attribute__ ((unused))) \
+  {                                                                     \
+    if (Mach_server_success (ERR) && MACH_PORT_VALID (NAME))            \
+      __print_fail_backtrace (#NAME " leaked",                          \
+                              __FILE__, __LINE__, "Port leak detector"); \
+  }                                                                     \
+  char _Mach_port_check_x_##NAME[0]                                     \
+  __attribute__ ((unused, cleanup (_Mach_port_check_##NAME)))
+
+#endif /* __MACHX__ */
diff --git a/trans/crash.c b/trans/crash.c
index 0fe9b24..d911da3 100644
--- a/trans/crash.c
+++ b/trans/crash.c
@@ -21,6 +21,7 @@
    along with the GNU Hurd; see the file COPYING.  If not, write to
    the Free Software Foundation, 675 Mass Ave, Cambridge, MA 02139, USA.  */
 
+#include <machx.h>
 #include <hurd.h>
 #include <fcntl.h>
 #include <hurd/trivfs.h>
@@ -250,8 +251,12 @@ S_crash_dump_task (mach_port_t port,
                   mach_port_t ctty_id)
 {
   error_t err;
+  Mach_server_port_check (err, task);
+  Mach_server_port_check (err, core_file);
+  Mach_server_port_check (err, ctty_id);
   struct trivfs_protid *cred;
   mach_port_t user_proc = MACH_PORT_NULL;
+  Mach_port_check (user_proc);
   enum crash_action how;
 
   cred = ports_lookup_port (port_bucket, port, trivfs_protid_class);
@@ -303,7 +308,7 @@ S_crash_dump_task (mach_port_t port,
                 We will wait for signals to resume (crash) it.  */
              msgport = ports_get_send_right (c);
              err = proc_setmsgport (user_proc, msgport, &c->original_msgport);
-             mach_port_deallocate (mach_task_self (), msgport);
+             Mach_port_deallocate (mach_task_self (), msgport);
 
              c->reply_port = reply_port;
              c->reply_type = reply_type;
@@ -314,10 +319,8 @@ S_crash_dump_task (mach_port_t port,
              /* Tell the proc server the crasher stopped.  */
              proc_mark_stop (user_proc, signo, sigcode);
 
-             c->task = task;
-             task = MACH_PORT_NULL;
-             c->core_file = core_file;
-             core_file = MACH_PORT_NULL;
+             c->task = Mach_port_move (task);
+             c->core_file = Mach_port_move (core_file);
              c->core_limit = (off_t) -1; /* XXX should core limit in RPC */
              c->signo = signo;
              c->sigcode = sigcode;
@@ -364,7 +367,7 @@ S_crash_dump_task (mach_port_t port,
 
          if (sink != core_file)
            {
-             mach_port_deallocate (mach_task_self (), sink);
+             Mach_port_deallocate (mach_task_self (), sink);
 
              /* We return an error so that the libc discards
                 CORE_FILE.  */
@@ -386,16 +389,12 @@ S_crash_dump_task (mach_port_t port,
       }
     }
 
-  if (user_proc != MACH_PORT_NULL)
-    mach_port_deallocate (mach_task_self (), user_proc);
-  if (err == 0 || err = MIG_NO_REPLY)
+  Mach_port_deallocate (mach_task_self (), user_proc);
+  if (Mach_server_success (err))
     {
-      if (MACH_PORT_VALID (task))
-       mach_port_deallocate (mach_task_self (), task);
-      if (MACH_PORT_VALID (core_file))
-       mach_port_deallocate (mach_task_self (), core_file);
-      if (MACH_PORT_VALID (ctty_id))
-       mach_port_deallocate (mach_task_self (), ctty_id);
+      Mach_port_deallocate (mach_task_self (), task);
+      Mach_port_deallocate (mach_task_self (), core_file);
+      Mach_port_deallocate (mach_task_self (), ctty_id);
     }
 
   ports_port_deref (cred);
-- 
2.1.4