diff mbox series

[hurd,commited,3/3] hurd: clean fd and port on thread cancel

Message ID 20200628014526.310138-4-samuel.thibault@ens-lyon.org
State New
Headers show
Series More cancel work | expand

Commit Message

Samuel Thibault June 28, 2020, 1:45 a.m. UTC
HURD_*PORT_USE link fd and port with a stack-stored structure, so on
thread cancel we need to cleanup this.

* hurd/fd-cleanup.c: New file.
* hurd/port-cleanup.c (_hurd_port_use_cleanup): New function.
* hurd/Makefile (routines): Add fd-cleanup.
* sysdeps/hurd/include/hurd.h (__USEPORT_CANCEL): New macro.
* sysdeps/hurd/include/hurd/fd.h (_hurd_fd_port_use_data): New
structure.
(_hurd_fd_port_use_cleanup): New prototype.
(HURD_DPORT_USE_CANCEL, HURD_FD_PORT_USE_CANCEL): New macros.
* sysdeps/hurd/include/hurd/port.h (_hurd_port_use_data): New structure.
(_hurd_port_use_cleanup): New prototype.
(HURD_PORT_USE_CANCEL): New macro.
* hurd/hurd/fd.h (HURD_FD_PORT_USE): Also refer to HURD_FD_PORT_USE_CANCEL.
* hurd/hurd.h (__USEPORT): Also refer to __USEPORT_CANCEL.
* hurd/hurd/port.h (HURD_PORT_USE): Also refer to HURD_PORT_USE_CANCEL.

* hurd/fd-read.c (_hurd_fd_read): Call HURD_FD_PORT_USE_CANCEL instead
of HURD_FD_PORT_USE.
* hurd/fd-write.c (_hurd_fd_write): Likewise.
* sysdeps/mach/hurd/send.c (__send): Call HURD_DPORT_USE_CANCEL instead
of HURD_DPORT_USE.
* sysdeps/mach/hurd/sendmsg.c (__libc_sendmsg): Likewise.
* sysdeps/mach/hurd/sendto.c (__sendto): Likewise.
* sysdeps/mach/hurd/recv.c (__recv): Likewise.
* sysdeps/mach/hurd/recvfrom.c (__recvfrom): Likewise.
* sysdeps/mach/hurd/recvmsg.c (__libc_recvmsg): Call __USEPORT_CANCEL
instead of __USEPORT, and HURD_DPORT_USE_CANCEL instead of
HURD_DPORT_USE.
---
 hurd/Makefile                    |  3 ++-
 hurd/fd-cleanup.c                | 33 +++++++++++++++++++++++++
 hurd/fd-read.c                   |  2 +-
 hurd/fd-write.c                  |  2 +-
 hurd/hurd.h                      |  1 +
 hurd/hurd/fd.h                   |  1 +
 hurd/hurd/port.h                 |  1 +
 hurd/port-cleanup.c              | 13 +++++++++-
 sysdeps/hurd/include/hurd.h      |  5 ++++
 sysdeps/hurd/include/hurd/fd.h   | 41 ++++++++++++++++++++++++++++++++
 sysdeps/hurd/include/hurd/port.h | 24 +++++++++++++++++++
 sysdeps/mach/hurd/recv.c         | 12 +++++-----
 sysdeps/mach/hurd/recvfrom.c     | 12 +++++-----
 sysdeps/mach/hurd/recvmsg.c      | 12 +++++-----
 sysdeps/mach/hurd/send.c         |  8 +++----
 sysdeps/mach/hurd/sendmsg.c      |  2 +-
 sysdeps/mach/hurd/sendto.c       |  2 +-
 17 files changed, 146 insertions(+), 28 deletions(-)
 create mode 100644 hurd/fd-cleanup.c
diff mbox series

Patch

diff --git a/hurd/Makefile b/hurd/Makefile
index 1ace90d10b..861bbf7842 100644
--- a/hurd/Makefile
+++ b/hurd/Makefile
@@ -56,7 +56,8 @@  routines = hurdstartup hurdinit \
 	   ports-get ports-set hurdports hurdmsg \
 	   errno-loc \
 	   hurdlock \
-	   $(sig) $(dtable) $(inlines) port-cleanup report-wait xattr
+	   $(sig) $(dtable) $(inlines) \
+	   fd-cleanup port-cleanup report-wait xattr
 sig	= hurdsig hurdfault siginfo hurd-raise preempt-sig \
 	  trampoline longjmp-ts catch-exc exc2signal hurdkill sigunwind \
 	  thread-self thread-cancel intr-msg catch-signal
diff --git a/hurd/fd-cleanup.c b/hurd/fd-cleanup.c
new file mode 100644
index 0000000000..83bde6ff44
--- /dev/null
+++ b/hurd/fd-cleanup.c
@@ -0,0 +1,33 @@ 
+/* Cleanup function for `struct hurd_fd' users.
+   Copyright (C) 1995-2020 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library 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
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#include <mach.h>
+#include <hurd/fd.h>
+
+/* We were cancelled while using an fd, and called from the cleanup unwinding.
+ */
+
+void
+_hurd_fd_port_use_cleanup (void *arg)
+{
+  struct _hurd_fd_port_use_data *data = arg;
+
+  _hurd_port_free (&data->d->port, &data->ulink, data->port);
+  if (data->ctty != MACH_PORT_NULL)
+    _hurd_port_free (&data->d->ctty, &data->ctty_ulink, data->ctty);
+}
diff --git a/hurd/fd-read.c b/hurd/fd-read.c
index 11c435f491..1a8f20301e 100644
--- a/hurd/fd-read.c
+++ b/hurd/fd-read.c
@@ -35,7 +35,7 @@  _hurd_fd_read (struct hurd_fd *fd, void *buf, size_t *nbytes, loff_t offset)
 
   data = buf;
   nread = *nbytes;
-  if (err = HURD_FD_PORT_USE (fd, _hurd_ctty_input (port, ctty, readfd)))
+  if (err = HURD_FD_PORT_USE_CANCEL (fd, _hurd_ctty_input (port, ctty, readfd)))
     return err;
 
   if (data != buf)
diff --git a/hurd/fd-write.c b/hurd/fd-write.c
index f63bae5908..2463cea1c5 100644
--- a/hurd/fd-write.c
+++ b/hurd/fd-write.c
@@ -33,7 +33,7 @@  _hurd_fd_write (struct hurd_fd *fd,
       return __io_write (port, buf, *nbytes, offset, &wrote);
     }
 
-  err = HURD_FD_PORT_USE (fd, _hurd_ctty_output (port, ctty, writefd));
+  err = HURD_FD_PORT_USE_CANCEL (fd, _hurd_ctty_output (port, ctty, writefd));
 
   if (! err)
     *nbytes = wrote;
diff --git a/hurd/hurd.h b/hurd/hurd.h
index cd7a149c5e..8f1fdf4a64 100644
--- a/hurd/hurd.h
+++ b/hurd/hurd.h
@@ -92,6 +92,7 @@  extern sigset_t _hurdsig_traced;
 
 /* Shorthand macro for internal library code referencing _hurd_ports (see
    <hurd/port.h>).  */
+/* Also see __USEPORT_CANCEL.  */
 
 #define	__USEPORT(which, expr) \
   HURD_PORT_USE (&_hurd_ports[INIT_PORT_##which], (expr))
diff --git a/hurd/hurd/fd.h b/hurd/hurd/fd.h
index e18c3cef42..d27be21cfd 100644
--- a/hurd/hurd/fd.h
+++ b/hurd/hurd/fd.h
@@ -115,6 +115,7 @@  _hurd_fd_get (int fd)
   HURD_FD_USE ((fd), HURD_FD_PORT_USE (descriptor, (expr)))
 
 /* Likewise, but FD is a pointer to the file descriptor structure.  */
+/* Also see HURD_FD_PORT_USE_CANCEL.  */
 
 #define	HURD_FD_PORT_USE(fd, expr)					      \
   ({ error_t __result;							      \
diff --git a/hurd/hurd/port.h b/hurd/hurd/port.h
index 33e696a4b2..f91b1d5e26 100644
--- a/hurd/hurd/port.h
+++ b/hurd/hurd/port.h
@@ -42,6 +42,7 @@  struct hurd_port
 
 
 /* Evaluate EXPR with the variable `port' bound to the port in PORTCELL.  */
+/* Also see HURD_PORT_USE_CANCEL.  */
 
 #define	HURD_PORT_USE(portcell, expr)					      \
   ({ struct hurd_port *const __p = (portcell);				      \
diff --git a/hurd/port-cleanup.c b/hurd/port-cleanup.c
index 0bb8267743..95b019c4c0 100644
--- a/hurd/port-cleanup.c
+++ b/hurd/port-cleanup.c
@@ -1,4 +1,4 @@ 
-/* Cleanup function for `struct hurd_port' users who longjmp.
+/* Cleanup function for `struct hurd_port' users.
    Copyright (C) 1995-2020 Free Software Foundation, Inc.
    This file is part of the GNU C Library.
 
@@ -28,3 +28,14 @@  _hurd_port_cleanup (void *cleanup_data, jmp_buf env, int val)
 {
   __mach_port_deallocate (__mach_task_self (), (mach_port_t) cleanup_data);
 }
+
+/* We were cancelled while using a port, and called from the cleanup unwinding.
+ */
+
+void
+_hurd_port_use_cleanup (void *arg)
+{
+  struct _hurd_port_use_data *data = arg;
+
+  _hurd_port_free (data->p, &data->link, data->port);
+}
diff --git a/sysdeps/hurd/include/hurd.h b/sysdeps/hurd/include/hurd.h
index d29f580d5b..dc877173b5 100644
--- a/sysdeps/hurd/include/hurd.h
+++ b/sysdeps/hurd/include/hurd.h
@@ -1,5 +1,10 @@ 
 #ifndef	_HURD_H
 #include_next <hurd.h>
+
+/* Like __USEPORT, but cleans fd on cancel.  */
+#define	__USEPORT_CANCEL(which, expr) \
+  HURD_PORT_USE_CANCEL (&_hurd_ports[INIT_PORT_##which], (expr))
+
 #ifndef _ISOMAC
 libc_hidden_proto (_hurd_exec_paths)
 libc_hidden_proto (_hurd_init)
diff --git a/sysdeps/hurd/include/hurd/fd.h b/sysdeps/hurd/include/hurd/fd.h
index 1f3ac380a0..3a6cdc7f36 100644
--- a/sysdeps/hurd/include/hurd/fd.h
+++ b/sysdeps/hurd/include/hurd/fd.h
@@ -1,6 +1,47 @@ 
 #ifndef	_HURD_FD_H
 #include_next <hurd/fd.h>
+
 #ifndef _ISOMAC
+#include <libc-lock.h>
+
+struct _hurd_fd_port_use_data
+  {
+     struct hurd_fd *d;
+     struct hurd_userlink ulink, ctty_ulink;
+     io_t port, ctty;
+  };
+
+extern void _hurd_fd_port_use_cleanup (void *arg);
+
+/* Like HURD_DPORT_USE, but cleans fd on cancel.  */
+#define HURD_DPORT_USE_CANCEL(fd, expr) \
+  HURD_FD_USE ((fd), HURD_FD_PORT_USE_CANCEL (descriptor, (expr)))
+
+/* Like HURD_FD_PORT_USE, but cleans fd on cancel.  */
+#define	HURD_FD_PORT_USE_CANCEL(fd, expr)				      \
+  ({ error_t __result;							      \
+     void *__crit = _hurd_critical_section_lock ();			      \
+     struct _hurd_fd_port_use_data __d;					      \
+     io_t port, ctty;							      \
+     __d.d = (fd);							      \
+     __spin_lock (&__d.d->port.lock);					      \
+     if (__d.d->port.port == MACH_PORT_NULL)				      \
+       {								      \
+	 __spin_unlock (&__d.d->port.lock);				      \
+	 _hurd_critical_section_unlock (__crit);			      \
+	 __result = EBADF;						      \
+       }								      \
+     else								      \
+       {								      \
+	 __d.ctty = ctty = _hurd_port_get (&__d.d->ctty, &__d.ctty_ulink);    \
+	 __d.port = port = _hurd_port_locked_get (&__d.d->port, &__d.ulink);  \
+	 __libc_cleanup_push (_hurd_fd_port_use_cleanup, &__d);		      \
+	 _hurd_critical_section_unlock (__crit);			      \
+	 __result = (expr);						      \
+	 __libc_cleanup_pop (1);					      \
+       }								      \
+     __result; })
+
 libc_hidden_proto (_hurd_intern_fd)
 libc_hidden_proto (_hurd_fd_error)
 libc_hidden_proto (_hurd_fd_error_signal)
diff --git a/sysdeps/hurd/include/hurd/port.h b/sysdeps/hurd/include/hurd/port.h
index ca1d2d111a..7828dd6fc1 100644
--- a/sysdeps/hurd/include/hurd/port.h
+++ b/sysdeps/hurd/include/hurd/port.h
@@ -1,6 +1,30 @@ 
 #ifndef	_HURD_PORT_H
 #include_next <hurd/port.h>
+
 #ifndef _ISOMAC
+struct _hurd_port_use_data
+  {
+     struct hurd_port *p;
+     struct hurd_userlink link;
+     mach_port_t port;
+  };
+
+extern void _hurd_port_use_cleanup (void *arg);
+
+/* Like HURD_PORT_USE, but cleans fd on cancel.  */
+#define	HURD_PORT_USE_CANCEL(portcell, expr)				      \
+  ({ struct _hurd_port_use_data __d;					      \
+     __typeof(expr) __result;						      \
+     void *__crit;							      \
+     __d.p = (portcell);						      \
+     __crit = _hurd_critical_section_lock ();				      \
+     __d.port = port = _hurd_port_get (__d.p, &__d.link);		      \
+     __libc_cleanup_push (_hurd_port_use_cleanup, &__d);		      \
+     _hurd_critical_section_unlock (__crit);				      \
+     __result = (expr);							      \
+     __libc_cleanup_pop (1);						      \
+     __result; })
+
 libc_hidden_proto (_hurd_port_locked_get)
 libc_hidden_proto (_hurd_port_locked_set)
 #ifdef _HURD_PORT_H_HIDDEN_DEF
diff --git a/sysdeps/mach/hurd/recv.c b/sysdeps/mach/hurd/recv.c
index b9ed067265..4fa2f79dcd 100644
--- a/sysdeps/mach/hurd/recv.c
+++ b/sysdeps/mach/hurd/recv.c
@@ -40,12 +40,12 @@  __recv (int fd, void *buf, size_t n, int flags)
   int cancel_oldtype;
 
   cancel_oldtype = LIBC_CANCEL_ASYNC();
-  err = HURD_DPORT_USE (fd, __socket_recv (port, &addrport,
-					       flags, &bufp, &nread,
-					       &ports, &nports,
-					       &cdata, &clen,
-					       &flags,
-					       n));
+  err = HURD_DPORT_USE_CANCEL (fd, __socket_recv (port, &addrport,
+						  flags, &bufp, &nread,
+						  &ports, &nports,
+						  &cdata, &clen,
+						  &flags,
+						  n));
   LIBC_CANCEL_RESET (cancel_oldtype);
 
   if (err == MIG_BAD_ID || err == EOPNOTSUPP)
diff --git a/sysdeps/mach/hurd/recvfrom.c b/sysdeps/mach/hurd/recvfrom.c
index ed3d970722..2cd7092088 100644
--- a/sysdeps/mach/hurd/recvfrom.c
+++ b/sysdeps/mach/hurd/recvfrom.c
@@ -43,12 +43,12 @@  __recvfrom (int fd, void *buf, size_t n, int flags, __SOCKADDR_ARG addrarg,
   int cancel_oldtype;
 
   cancel_oldtype = LIBC_CANCEL_ASYNC();
-  err = HURD_DPORT_USE (fd, __socket_recv (port, &addrport,
-					   flags, &bufp, &nread,
-					   &ports, &nports,
-					   &cdata, &clen,
-					   &flags,
-					   n));
+  err = HURD_DPORT_USE_CANCEL (fd, __socket_recv (port, &addrport,
+						  flags, &bufp, &nread,
+						  &ports, &nports,
+						  &cdata, &clen,
+						  &flags,
+						  n));
   LIBC_CANCEL_RESET (cancel_oldtype);
 
   if (err)
diff --git a/sysdeps/mach/hurd/recvmsg.c b/sysdeps/mach/hurd/recvmsg.c
index 75c5f37f0e..5ac130fed1 100644
--- a/sysdeps/mach/hurd/recvmsg.c
+++ b/sysdeps/mach/hurd/recvmsg.c
@@ -58,7 +58,7 @@  __libc_recvmsg (int fd, struct msghdr *message, int flags)
       while (err == EINTR);
       if (!err)
 	do
-	  err = __USEPORT (AUTH, __auth_user_authenticate (port,
+	  err = __USEPORT_CANCEL (AUTH, __auth_user_authenticate (port,
 					  ref, MACH_MSG_TYPE_MAKE_SEND,
 					  result));
 	while (err == EINTR);
@@ -86,11 +86,11 @@  __libc_recvmsg (int fd, struct msghdr *message, int flags)
 
   buf = data;
   cancel_oldtype = LIBC_CANCEL_ASYNC();
-  err = HURD_DPORT_USE (fd, __socket_recv (port, &aport,
-					   flags, &data, &len,
-					   &ports, &nports,
-					   &cdata, &clen,
-					   &message->msg_flags, amount));
+  err = HURD_DPORT_USE_CANCEL (fd, __socket_recv (port, &aport,
+						  flags, &data, &len,
+						  &ports, &nports,
+						  &cdata, &clen,
+						  &message->msg_flags, amount));
   LIBC_CANCEL_RESET (cancel_oldtype);
   if (err)
     return __hurd_sockfail (fd, flags, err);
diff --git a/sysdeps/mach/hurd/send.c b/sysdeps/mach/hurd/send.c
index 78edbc2ff3..5be6c3a773 100644
--- a/sysdeps/mach/hurd/send.c
+++ b/sysdeps/mach/hurd/send.c
@@ -31,10 +31,10 @@  __send (int fd, const void *buf, size_t n, int flags)
   int cancel_oldtype;
 
   cancel_oldtype = LIBC_CANCEL_ASYNC();
-  err = HURD_DPORT_USE (fd, __socket_send (port, MACH_PORT_NULL,
-					   flags, buf, n,
-					   NULL, MACH_MSG_TYPE_COPY_SEND, 0,
-					   NULL, 0, &wrote));
+  err = HURD_DPORT_USE_CANCEL (fd, __socket_send (port, MACH_PORT_NULL,
+						  flags, buf, n,
+						  NULL, MACH_MSG_TYPE_COPY_SEND, 0,
+						  NULL, 0, &wrote));
   LIBC_CANCEL_RESET (cancel_oldtype);
 
   if (err == MIG_BAD_ID || err == EOPNOTSUPP)
diff --git a/sysdeps/mach/hurd/sendmsg.c b/sysdeps/mach/hurd/sendmsg.c
index 37ab7f3ce0..d8764c1248 100644
--- a/sysdeps/mach/hurd/sendmsg.c
+++ b/sysdeps/mach/hurd/sendmsg.c
@@ -174,7 +174,7 @@  __libc_sendmsg (int fd, const struct msghdr *message, int flags)
 	err = EIEIO;
     }
 
-  err = HURD_DPORT_USE (fd,
+  err = HURD_DPORT_USE_CANCEL (fd,
 			({
 			  if (err)
 			    err = __socket_create_address (port,
diff --git a/sysdeps/mach/hurd/sendto.c b/sysdeps/mach/hurd/sendto.c
index a5faf8ea2e..aab3d3dcae 100644
--- a/sysdeps/mach/hurd/sendto.c
+++ b/sysdeps/mach/hurd/sendto.c
@@ -73,7 +73,7 @@  __sendto (int fd,
       return err_port;
     }
 
-  err = HURD_DPORT_USE (fd,
+  err = HURD_DPORT_USE_CANCEL (fd,
 			({
 			  if (addr != NULL)
 			    err = create_address_port (port, addr, addr_len,