[rtems commit] libnetworking: Fix close of active sockets
Sebastian Huber
sebh at rtems.org
Tue Jan 20 06:15:06 UTC 2015
Module: rtems
Branch: master
Commit: f87ede57a2e97f0743a85b94072c7163fa485ae9
Changeset: http://git.rtems.org/rtems/commit/?id=f87ede57a2e97f0743a85b94072c7163fa485ae9
Author: Sebastian Huber <sebastian.huber at embedded-brains.de>
Date: Thu Jan 15 14:13:19 2015 +0100
libnetworking: Fix close of active sockets
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 | 27 ++-
cpukit/libnetworking/rtems/rtems_bsdnet_internal.h | 2 +-
cpukit/libnetworking/rtems/rtems_glue.c | 102 +++++------
cpukit/libnetworking/rtems/rtems_select.c | 7 +-
cpukit/libnetworking/rtems/rtems_syscall.c | 22 +--
cpukit/rtems/include/rtems/rtems/event.h | 5 +
testsuites/libtests/syscall01/init.c | 191 ++++++++++++++++++++-
testsuites/samples/loopback/init.c | 5 +-
8 files changed, 286 insertions(+), 75 deletions(-)
diff --git a/cpukit/libnetworking/kern/uipc_socket.c b/cpukit/libnetworking/kern/uipc_socket.c
index 7ed3ad0..b221a37 100644
--- a/cpukit/libnetworking/kern/uipc_socket.c
+++ b/cpukit/libnetworking/kern/uipc_socket.c
@@ -145,6 +145,26 @@ sofree(struct socket *so)
FREE(so, M_SOCKET);
}
+static void
+rtems_socket_close_notify(struct socket *so)
+{
+ if (so->so_pgid) {
+ rtems_event_system_send(so->so_pgid, RTEMS_EVENT_SYSTEM_NETWORK_CLOSE);
+ }
+}
+
+static void
+rtems_sockbuf_close_notify(struct socket *so, struct sockbuf *sb)
+{
+ if (sb->sb_flags & SB_WAIT) {
+ rtems_event_system_send(sb->sb_sel.si_pid,
+ RTEMS_EVENT_SYSTEM_NETWORK_CLOSE);
+ }
+
+ if (sb->sb_wakeup)
+ (*sb->sb_wakeup)(so, sb->sb_wakeuparg);
+}
+
/*
* Close a socket on last file table reference removal.
* Initiate disconnect if connected.
@@ -156,6 +176,10 @@ soclose(struct socket *so)
int s = splnet(); /* conservative */
int error = 0;
+ rtems_socket_close_notify(so);
+ rtems_sockbuf_close_notify(so, &so->so_snd);
+ rtems_sockbuf_close_notify(so, &so->so_rcv);
+
if (so->so_options & SO_ACCEPTCONN) {
struct socket *sp, *sonext;
@@ -742,7 +766,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..9d122e4 100644
--- a/cpukit/libnetworking/rtems/rtems_glue.c
+++ b/cpukit/libnetworking/rtems/rtems_glue.c
@@ -432,47 +432,68 @@ rtems_bsdnet_semaphore_release (void)
#endif
}
-/*
- * Wait for something to happen to a socket buffer
- */
-int
-sbwait(struct sockbuf *sb)
+static int
+rtems_bsdnet_sleep(rtems_event_set in, rtems_interval ticks)
{
- rtems_event_set events;
- rtems_id tid;
rtems_status_code sc;
+ rtems_event_set out;
+ rtems_event_set out2;
+
+ in |= RTEMS_EVENT_SYSTEM_NETWORK_CLOSE;
/*
- * Soak up any pending events.
- * The sleep/wakeup synchronization in the FreeBSD
- * kernel has no memory.
+ * Soak up any pending events. 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.
+ * Wait for the wakeup event.
*/
- rtems_task_ident (RTEMS_SELF, 0, &tid);
- sb->sb_sel.si_pid = tid;
+ sc = rtems_bsdnet_event_receive(in, RTEMS_EVENT_ANY | RTEMS_WAIT,
+ ticks, &out);
/*
- * Show that socket is waiting
+ * Get additional events that may have been received between the
+ * rtems_event_system_receive() and the rtems_bsdnet_semaphore_obtain().
*/
- sb->sb_flags |= SB_WAIT;
+ rtems_event_system_receive(in, RTEMS_EVENT_ANY | RTEMS_NO_WAIT,
+ RTEMS_NO_TIMEOUT, &out2);
+ out |= out2;
+
+ if (out & RTEMS_EVENT_SYSTEM_NETWORK_CLOSE)
+ return (ENXIO);
+
+ if (sc == RTEMS_SUCCESSFUL)
+ return (0);
+
+ return (EWOULDBLOCK);
+}
+
+/*
+ * Wait for something to happen to a socket buffer
+ */
+int
+sbwait(struct sockbuf *sb)
+{
+ int error;
/*
- * Wait for the wakeup event.
+ * Set this task as the target of the wakeup operation.
*/
- sc = rtems_bsdnet_event_receive (SBWAIT_EVENT, RTEMS_EVENT_ANY | RTEMS_WAIT, sb->sb_timeo, &events);
+ sb->sb_sel.si_pid = rtems_task_self();
/*
- * Return the status of the wait.
+ * Show that socket is waiting
*/
- switch (sc) {
- case RTEMS_SUCCESSFUL: return 0;
- case RTEMS_TIMEOUT: return EWOULDBLOCK;
- default: return ENXIO;
- }
+ sb->sb_flags |= SB_WAIT;
+
+ error = rtems_bsdnet_sleep(SBWAIT_EVENT, sb->sb_timeo);
+ if (error != ENXIO)
+ sb->sb_flags &= ~SB_WAIT;
+
+ return (error);
}
@@ -485,7 +506,6 @@ sowakeup(
struct sockbuf *sb)
{
if (sb->sb_flags & SB_WAIT) {
- sb->sb_flags &= ~SB_WAIT;
rtems_event_system_send (sb->sb_sel.si_pid, SBWAIT_EVENT);
}
if (sb->sb_wakeup) {
@@ -514,40 +534,20 @@ wakeup (void *p)
int
soconnsleep (struct socket *so)
{
- rtems_event_set events;
- rtems_id tid;
- rtems_status_code sc;
-
- /*
- * Soak up any pending events.
- * 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);
+ int error;
/*
* Set this task as the target of the wakeup operation.
*/
if (so->so_pgid)
rtems_panic ("Another task is already sleeping on that socket");
- rtems_task_ident (RTEMS_SELF, 0, &tid);
- so->so_pgid = tid;
-
- /*
- * Wait for the wakeup event.
- */
- sc = rtems_bsdnet_event_receive (SOSLEEP_EVENT, RTEMS_EVENT_ANY | RTEMS_WAIT, so->so_rcv.sb_timeo, &events);
+ so->so_pgid = rtems_task_self();
- /*
- * Relinquish ownership of the socket.
- */
- so->so_pgid = 0;
+ error = rtems_bsdnet_sleep(SOSLEEP_EVENT, so->so_rcv.sb_timeo);
+ if (error != ENXIO)
+ so->so_pgid = 0;
- switch (sc) {
- case RTEMS_SUCCESSFUL: return 0;
- case RTEMS_TIMEOUT: return EWOULDBLOCK;
- default: return ENXIO;
- }
+ return (error);
}
/*
diff --git a/cpukit/libnetworking/rtems/rtems_select.c b/cpukit/libnetworking/rtems/rtems_select.c
index 47a4cb2..05c8951 100644
--- a/cpukit/libnetworking/rtems/rtems_select.c
+++ b/cpukit/libnetworking/rtems/rtems_select.c
@@ -119,7 +119,8 @@ select (int nfds, fd_set *__restrict readfds, fd_set *__restrict writefds,
int retval = 0;
rtems_id tid;
rtems_interval then = 0, now;
- rtems_event_set events;
+ rtems_event_set in = SBWAIT_EVENT | RTEMS_EVENT_SYSTEM_NETWORK_CLOSE;
+ rtems_event_set out;
if (nfds < 0)
return (EINVAL);
@@ -145,7 +146,7 @@ select (int nfds, fd_set *__restrict readfds, fd_set *__restrict writefds,
#undef getbits
rtems_task_ident (RTEMS_SELF, 0, &tid);
- 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);
for (;;) {
rtems_bsdnet_semaphore_obtain ();
error = selscan(tid, ibits, obits, nfds, &retval);
@@ -159,7 +160,7 @@ select (int nfds, fd_set *__restrict readfds, fd_set *__restrict writefds,
break;
then = now;
}
- rtems_event_system_receive (SBWAIT_EVENT, RTEMS_EVENT_ANY | RTEMS_WAIT, timo, &events);
+ rtems_event_system_receive (in, RTEMS_EVENT_ANY | RTEMS_WAIT, timo, &out);
}
#define putbits(name,i) if (name) *name = ob[i]
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..ec5e5b2 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>
@@ -18,8 +18,10 @@
#include "tmacros.h"
+#include <sys/select.h>
#include <sys/socket.h>
#include <sys/stat.h>
+#include <netinet/in.h>
#include <errno.h>
#include <fcntl.h>
#include <unistd.h>
@@ -29,14 +31,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 +83,184 @@ 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 test_select_and_close(test_context *ctx)
+{
+ int rv;
+ struct sockaddr_in addr;
+ socklen_t addrlen = sizeof(addr);
+ int nfds;
+ struct fd_set set;
+
+ 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);
+
+ nfds = ctx->fd + 1;
+ FD_ZERO(&set);
+ FD_SET(ctx->fd, &set);
+
+ errno = 0;
+ rv = select(nfds, &set, NULL, NULL, NULL);
+ rtems_test_assert(rv == -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);
+ test_select_and_close(ctx);
+
+ sc = rtems_task_delete(ctx->close_task);
+ rtems_test_assert(sc == RTEMS_SUCCESSFUL);
TEST_END();
@@ -129,7 +304,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
diff --git a/testsuites/samples/loopback/init.c b/testsuites/samples/loopback/init.c
index 7ac376f..ccaf3c6 100644
--- a/testsuites/samples/loopback/init.c
+++ b/testsuites/samples/loopback/init.c
@@ -167,7 +167,10 @@ static rtems_task serverTask(rtems_task_argument arg)
addrlen = sizeof farAddr;
s1 = accept(s, (struct sockaddr *)&farAddr, &addrlen);
if (s1 < 0)
- rtems_panic("Can't accept connection: %s", strerror(errno));
+ if (errno == ENXIO)
+ rtems_task_delete(RTEMS_SELF);
+ else
+ rtems_panic("Can't accept connection: %s", strerror(errno));
else
printf("ACCEPTED:%lX\n", ntohl(farAddr.sin_addr.s_addr));
spawnTask(workerTask, myPriority, s1);
More information about the vc
mailing list