[PATCH] libnetworking: Fix close of active sockets

Sebastian Huber sebastian.huber at embedded-brains.de
Thu Jan 15 13:15:07 UTC 2015


Send a special event to notify tasks waiting for a socket state change
in case this socket gets closed.  This prevents a use after free.

Close #785.
---
 cpukit/libnetworking/kern/uipc_socket.c            |  18 ++-
 cpukit/libnetworking/rtems/rtems_bsdnet_internal.h |   2 +-
 cpukit/libnetworking/rtems/rtems_glue.c            |  20 ++-
 cpukit/libnetworking/rtems/rtems_syscall.c         |  22 +--
 cpukit/rtems/include/rtems/rtems/event.h           |   5 +
 testsuites/libtests/syscall01/init.c               | 158 +++++++++++++++++++--
 6 files changed, 199 insertions(+), 26 deletions(-)

diff --git a/cpukit/libnetworking/kern/uipc_socket.c b/cpukit/libnetworking/kern/uipc_socket.c
index 7a16f7e..634a07d 100644
--- a/cpukit/libnetworking/kern/uipc_socket.c
+++ b/cpukit/libnetworking/kern/uipc_socket.c
@@ -146,6 +146,17 @@ sofree(struct socket *so)
 	FREE(so, M_SOCKET);
 }
 
+static void
+rtems_socket_close_notify(pid_t *pid_p)
+{
+	pid_t pid = *pid_p;
+
+	if (pid != 0) {
+		*pid_p = 0;
+		rtems_event_system_send (pid, RTEMS_EVENT_SYSTEM_NETWORK_CLOSE);
+	}
+}
+
 /*
  * Close a socket on last file table reference removal.
  * Initiate disconnect if connected.
@@ -157,6 +168,10 @@ soclose(struct socket *so)
 	int s = splnet();		/* conservative */
 	int error = 0;
 
+	rtems_socket_close_notify(&so->so_pgid);
+	rtems_socket_close_notify(&so->so_snd.sb_sel.si_pid);
+	rtems_socket_close_notify(&so->so_rcv.sb_sel.si_pid);
+
 	if (so->so_options & SO_ACCEPTCONN) {
 		struct socket *sp, *sonext;
 
@@ -743,7 +758,8 @@ dontblock:
 				break;
 			error = sbwait(&so->so_rcv);
 			if (error) {
-				sbunlock(&so->so_rcv);
+				if (error != ENXIO)
+					sbunlock(&so->so_rcv);
 				splx(s);
 				return (0);
 			}
diff --git a/cpukit/libnetworking/rtems/rtems_bsdnet_internal.h b/cpukit/libnetworking/rtems/rtems_bsdnet_internal.h
index 5be781b..b790f05 100644
--- a/cpukit/libnetworking/rtems/rtems_bsdnet_internal.h
+++ b/cpukit/libnetworking/rtems/rtems_bsdnet_internal.h
@@ -219,7 +219,7 @@ int ioctl (int, ioctl_command_t, ...);
 #define NETISR_IP_EVENT        (1L << NETISR_IP)
 #define NETISR_ARP_EVENT       (1L << NETISR_ARP)
 #define NETISR_EVENTS  (NETISR_IP_EVENT|NETISR_ARP_EVENT)
-#if (SBWAIT_EVENT & SOSLEEP_EVENT & NETISR_EVENTS)
+#if (SBWAIT_EVENT & SOSLEEP_EVENT & NETISR_EVENTS & RTEMS_EVENT_SYSTEM_NETWORK_CLOSE)
 # error "Network event conflict"
 #endif
 
diff --git a/cpukit/libnetworking/rtems/rtems_glue.c b/cpukit/libnetworking/rtems/rtems_glue.c
index 4c90a98..18c92dd 100644
--- a/cpukit/libnetworking/rtems/rtems_glue.c
+++ b/cpukit/libnetworking/rtems/rtems_glue.c
@@ -438,7 +438,8 @@ rtems_bsdnet_semaphore_release (void)
 int
 sbwait(struct sockbuf *sb)
 {
-	rtems_event_set events;
+	rtems_event_set in = SBWAIT_EVENT | RTEMS_EVENT_SYSTEM_NETWORK_CLOSE;
+	rtems_event_set out;
 	rtems_id tid;
 	rtems_status_code sc;
 
@@ -447,7 +448,7 @@ sbwait(struct sockbuf *sb)
 	 * The sleep/wakeup synchronization in the FreeBSD
 	 * kernel has no memory.
 	 */
-	rtems_event_system_receive (SBWAIT_EVENT, RTEMS_EVENT_ANY | RTEMS_NO_WAIT, RTEMS_NO_TIMEOUT, &events);
+	rtems_event_system_receive (in, RTEMS_EVENT_ANY | RTEMS_NO_WAIT, RTEMS_NO_TIMEOUT, &out);
 
 	/*
 	 * Set this task as the target of the wakeup operation.
@@ -463,7 +464,10 @@ sbwait(struct sockbuf *sb)
 	/*
 	 * Wait for the wakeup event.
 	 */
-	sc = rtems_bsdnet_event_receive (SBWAIT_EVENT, RTEMS_EVENT_ANY | RTEMS_WAIT, sb->sb_timeo, &events);
+	sc = rtems_bsdnet_event_receive (in, RTEMS_EVENT_ANY | RTEMS_WAIT, sb->sb_timeo, &out);
+
+	if (out & RTEMS_EVENT_SYSTEM_NETWORK_CLOSE)
+		return ENXIO;
 
 	/*
 	 * Return the status of the wait.
@@ -514,7 +518,8 @@ wakeup (void *p)
 int
 soconnsleep (struct socket *so)
 {
-	rtems_event_set events;
+	rtems_event_set in = SOSLEEP_EVENT | RTEMS_EVENT_SYSTEM_NETWORK_CLOSE;
+	rtems_event_set out;
 	rtems_id tid;
 	rtems_status_code sc;
 
@@ -523,7 +528,7 @@ soconnsleep (struct socket *so)
 	 * The sleep/wakeup synchronization in the FreeBSD
 	 * kernel has no memory.
 	 */
-	rtems_event_system_receive (SOSLEEP_EVENT, RTEMS_EVENT_ANY | RTEMS_NO_WAIT, RTEMS_NO_TIMEOUT, &events);
+	rtems_event_system_receive (in, RTEMS_EVENT_ANY | RTEMS_NO_WAIT, RTEMS_NO_TIMEOUT, &out);
 
 	/*
 	 * Set this task as the target of the wakeup operation.
@@ -536,7 +541,10 @@ soconnsleep (struct socket *so)
 	/*
 	 * Wait for the wakeup event.
 	 */
-	sc = rtems_bsdnet_event_receive (SOSLEEP_EVENT, RTEMS_EVENT_ANY | RTEMS_WAIT, so->so_rcv.sb_timeo, &events);
+	sc = rtems_bsdnet_event_receive (in, RTEMS_EVENT_ANY | RTEMS_WAIT, so->so_rcv.sb_timeo, &out);
+
+	if (out & RTEMS_EVENT_SYSTEM_NETWORK_CLOSE)
+		return ENXIO;
 
 	/*
 	 * Relinquish ownership of the socket.
diff --git a/cpukit/libnetworking/rtems/rtems_syscall.c b/cpukit/libnetworking/rtems/rtems_syscall.c
index 324a634..1063798 100644
--- a/cpukit/libnetworking/rtems/rtems_syscall.c
+++ b/cpukit/libnetworking/rtems/rtems_syscall.c
@@ -204,17 +204,17 @@ connect (int s, struct sockaddr *name, int namelen)
 		rtems_bsdnet_semaphore_release ();
 		return -1;
 	}
-	while ((so->so_state & SS_ISCONNECTING) && so->so_error == 0) {
+	error = so->so_error;
+	while (error == 0 && (so->so_state & SS_ISCONNECTING)) {
 		error = soconnsleep (so);
 		if (error)
 			break;
-	}
-	if (error == 0) {
 		error = so->so_error;
 		so->so_error = 0;
 	}
     bad:
-	so->so_state &= ~SS_ISCONNECTING;
+	if (error != ENXIO)
+		so->so_state &= ~SS_ISCONNECTING;
 	m_freem (nam);
 	if (error)
 		errno = error;
@@ -249,6 +249,7 @@ accept (int s, struct sockaddr *name, int *namelen)
 	int fd;
 	struct socket *head, *so;
 	struct mbuf *nam;
+	int error;
 
 	rtems_bsdnet_semaphore_obtain ();
 	if ((head = rtems_bsdnet_fdToSocket (s)) == NULL) {
@@ -265,16 +266,16 @@ accept (int s, struct sockaddr *name, int *namelen)
 		rtems_bsdnet_semaphore_release ();
 		return -1;
 	}
-        while (head->so_comp.tqh_first == NULL && head->so_error == 0) {
+	error = head->so_error;
+        while (error ==  0 && head->so_comp.tqh_first == NULL) {
                 if (head->so_state & SS_CANTRCVMORE) {
-                        head->so_error = ECONNABORTED;
+                        error = ECONNABORTED;
                         break;
                 }
-		head->so_error = soconnsleep (head);
+		error = soconnsleep (head);
         }
-	if (head->so_error) {
-		errno = head->so_error;
-		head->so_error = 0;
+	if (error) {
+		errno = error;
 		rtems_bsdnet_semaphore_release ();
 		return -1;
 	}
@@ -715,6 +716,7 @@ rtems_bsdnet_close (rtems_libio_t *iop)
 		rtems_bsdnet_semaphore_release ();
 		return -1;
 	}
+	iop->data1 = NULL;
 	error = soclose (so);
 	rtems_bsdnet_semaphore_release ();
 	if (error) {
diff --git a/cpukit/rtems/include/rtems/rtems/event.h b/cpukit/rtems/include/rtems/rtems/event.h
index ca48ef2..451a313 100644
--- a/cpukit/rtems/include/rtems/rtems/event.h
+++ b/cpukit/rtems/include/rtems/rtems/event.h
@@ -314,6 +314,11 @@ rtems_status_code rtems_event_receive (
 #define RTEMS_EVENT_SYSTEM_NETWORK_SOSLEEP RTEMS_EVENT_25
 
 /**
+ * @brief Reserved system event for network socket close.
+ */
+#define RTEMS_EVENT_SYSTEM_NETWORK_CLOSE RTEMS_EVENT_26
+
+/**
  * @brief Reserved system event for transient usage.
  */
 #define RTEMS_EVENT_SYSTEM_TRANSIENT RTEMS_EVENT_31
diff --git a/testsuites/libtests/syscall01/init.c b/testsuites/libtests/syscall01/init.c
index bfffa0e..58eaed6 100644
--- a/testsuites/libtests/syscall01/init.c
+++ b/testsuites/libtests/syscall01/init.c
@@ -1,8 +1,8 @@
 /*
- * Copyright (c) 2012 embedded brains GmbH.  All rights reserved.
+ * Copyright (c) 2012-2015 embedded brains GmbH.  All rights reserved.
  *
  *  embedded brains GmbH
- *  Obere Lagerstr. 30
+ *  Dornierstr. 4
  *  82178 Puchheim
  *  Germany
  *  <rtems at embedded-brains.de>
@@ -20,6 +20,7 @@
 
 #include <sys/socket.h>
 #include <sys/stat.h>
+#include <netinet/in.h>
 #include <errno.h>
 #include <fcntl.h>
 #include <unistd.h>
@@ -29,14 +30,19 @@
 
 const char rtems_test_name[] = "SYSCALL 1";
 
-/* forward declarations to avoid warnings */
-static rtems_task Init(rtems_task_argument argument);
-
 static const char open_driver_path [] = "/dev/open_driver";
 
 struct rtems_bsdnet_config rtems_bsdnet_config;
 
-static void test(void)
+typedef struct {
+  rtems_id main_task;
+  rtems_id close_task;
+  int fd;
+} test_context;
+
+static test_context test_instance;
+
+static void test_sync(void)
 {
   int rv;
   char buf [1];
@@ -76,16 +82,152 @@ static void test(void)
   rtems_test_assert(rv == 0);
 }
 
+static void close_task(rtems_task_argument arg)
+{
+  test_context *ctx = (test_context *) arg;
+
+  while (true) {
+    rtems_status_code sc;
+    int rv;
+
+    rv = close(ctx->fd);
+    rtems_test_assert(rv == 0);
+
+    sc = rtems_event_transient_send(ctx->main_task);
+    rtems_test_assert(sc == RTEMS_SUCCESSFUL);
+  }
+}
+
+static void wait_for_close_task(void)
+{
+  rtems_status_code sc;
+
+  sc = rtems_event_transient_receive(RTEMS_WAIT, RTEMS_NO_TIMEOUT);
+  rtems_test_assert(sc == RTEMS_SUCCESSFUL);
+}
+
+static void test_accept_and_close(test_context *ctx)
+{
+  int rv;
+  int fd;
+  struct sockaddr_in addr;
+  socklen_t addrlen = sizeof(addr);
+
+  ctx->fd = socket(PF_INET, SOCK_STREAM, 0);
+  rtems_test_assert(ctx->fd >= 0);
+
+  rv = listen(ctx->fd, 1);
+  rtems_test_assert(rv == 0);
+
+  errno = 0;
+  fd = accept(ctx->fd, (struct sockaddr *) &addr, &addrlen);
+  rtems_test_assert(fd == -1);
+  rtems_test_assert(errno == ENXIO);
+
+  errno = 0;
+  fd = accept(ctx->fd, (struct sockaddr *) &addr, &addrlen);
+  rtems_test_assert(fd == -1);
+  rtems_test_assert(errno == EBADF);
+
+  wait_for_close_task();
+}
+
+static void test_connect_and_close(test_context *ctx)
+{
+  int rv;
+  struct sockaddr_in addr;
+  socklen_t addrlen = sizeof(addr);
+
+  ctx->fd = socket(PF_INET, SOCK_STREAM, 0);
+  rtems_test_assert(ctx->fd >= 0);
+
+  memset(&addr, 0, sizeof(addr));
+  addr.sin_family = AF_INET;
+  addr.sin_port = htons(1234);
+  addr.sin_addr.s_addr = htonl(INADDR_ANY);
+
+  errno = 0;
+  rv = connect(ctx->fd, (struct sockaddr *) &addr, addrlen);
+  rtems_test_assert(rv == -1);
+  rtems_test_assert(errno == ENXIO);
+
+  errno = 0;
+  rv = connect(ctx->fd, (struct sockaddr *) &addr, addrlen);
+  rtems_test_assert(rv == -1);
+  rtems_test_assert(errno == EBADF);
+
+  wait_for_close_task();
+}
+
+static void test_recv_and_close(test_context *ctx)
+{
+  int rv;
+  struct sockaddr_in addr;
+  socklen_t addrlen = sizeof(addr);
+  char buf[1];
+  ssize_t n;
+
+  ctx->fd = socket(PF_INET, SOCK_DGRAM, 0);
+  rtems_test_assert(ctx->fd >= 0);
+
+  memset(&addr, 0, sizeof(addr));
+  addr.sin_family = AF_INET;
+  addr.sin_port = htons(1234);
+  addr.sin_addr.s_addr = htonl(INADDR_ANY);
+
+  rv = bind(ctx->fd, (struct sockaddr *) &addr, addrlen);
+  rtems_test_assert(rv == 0);
+
+  errno = 0;
+  n = recv(ctx->fd, &buf[0], sizeof(buf), 0);
+  rtems_test_assert(n == -1);
+  rtems_test_assert(errno == ENXIO);
+
+  errno = 0;
+  n = recv(ctx->fd, &buf[0], sizeof(buf), 0);
+  rtems_test_assert(n == -1);
+  rtems_test_assert(errno == EBADF);
+
+  wait_for_close_task();
+}
+
 static void Init(rtems_task_argument arg)
 {
+  test_context *ctx = &test_instance;
+  rtems_status_code sc;
   int rv;
 
   TEST_BEGIN();
 
+  ctx->main_task = rtems_task_self();
+
+  sc = rtems_task_create(
+    rtems_build_name('C', 'L', 'O', 'S'),
+    2,
+    RTEMS_MINIMUM_STACK_SIZE,
+    RTEMS_DEFAULT_MODES,
+    RTEMS_DEFAULT_ATTRIBUTES,
+    &ctx->close_task
+  );
+  rtems_test_assert(sc == RTEMS_SUCCESSFUL);
+
+  sc = rtems_task_start(
+    ctx->close_task,
+    close_task,
+    (rtems_task_argument) ctx
+  );
+  rtems_test_assert(sc == RTEMS_SUCCESSFUL);
+
   rv = rtems_bsdnet_initialize_network();
   rtems_test_assert(rv == 0);
 
-  test();
+  test_sync();
+  test_accept_and_close(ctx);
+  test_connect_and_close(ctx);
+  test_recv_and_close(ctx);
+
+  sc = rtems_task_delete(ctx->close_task);
+  rtems_test_assert(sc == RTEMS_SUCCESSFUL);
 
   TEST_END();
 
@@ -129,7 +271,7 @@ static rtems_device_driver open_driver_open(
 
 #define CONFIGURE_LIBIO_MAXIMUM_FILE_DESCRIPTORS 4
 
-#define CONFIGURE_MAXIMUM_TASKS 2
+#define CONFIGURE_MAXIMUM_TASKS 3
 
 #define CONFIGURE_INITIAL_EXTENSIONS RTEMS_TEST_INITIAL_EXTENSION
 
-- 
1.8.4.5



More information about the devel mailing list