From cf5cba953d0fc0c5bca4f6a658e7a6708fbc42e1 Mon Sep 17 00:00:00 2001 From: Gregory Nutt Date: Thu, 19 Oct 2017 11:55:51 -0600 Subject: [PATCH] There was a possible recursion that could eventually overflow the stack. The error occurred when closing the socket with inet_close() while a socket callback was still queued. When the socket callback was executed by devif_conn_event(), this resulted in a call to psock_send_eventhandler() with TCP_CLOSE flag set which then called tcp_lost_connection(). tcp_shutdown_monitor() then called tcp_callback() again, which again called psock_send_eventhandler(), and so on.... Noted by Pascal Speck. Solution is also similar to a solution proposed by Pascal Speck. --- net/devif/devif_callback.c | 6 +++--- net/tcp/tcp_netpoll.c | 4 ++-- net/tcp/tcp_send_buffered.c | 7 ++++++- net/udp/udp_netpoll.c | 2 +- net/usrsock/usrsock_poll.c | 2 +- 5 files changed, 13 insertions(+), 8 deletions(-) diff --git a/net/devif/devif_callback.c b/net/devif/devif_callback.c index 419d8368692..92718b9798f 100644 --- a/net/devif/devif_callback.c +++ b/net/devif/devif_callback.c @@ -102,18 +102,18 @@ static void devif_callback_free(FAR struct net_driver_s *dev, * it is supposed to be in the device notification list. */ - if (dev) + if (dev != NULL) { /* Find the callback structure in the device event list */ for (prev = NULL, curr = dev->d_devcb; - curr && curr != cb; + curr != NULL && curr != cb; prev = curr, curr = curr->nxtdev); /* Remove the structure from the device event list */ DEBUGASSERT(curr); - if (curr) + if (curr != NULL) { if (prev) { diff --git a/net/tcp/tcp_netpoll.c b/net/tcp/tcp_netpoll.c index c87e620a278..615fb84b565 100644 --- a/net/tcp/tcp_netpoll.c +++ b/net/tcp/tcp_netpoll.c @@ -97,11 +97,11 @@ static uint16_t tcp_poll_eventhandler(FAR struct net_driver_s *dev, ninfo("flags: %04x\n", flags); - DEBUGASSERT(!info || (info->psock && info->fds)); + DEBUGASSERT(info == NULL || (info->psock != NULL && info->fds != NULL)); /* 'priv' might be null in some race conditions (?) */ - if (info) + if (info != NULL) { pollevent_t eventset = 0; diff --git a/net/tcp/tcp_send_buffered.c b/net/tcp/tcp_send_buffered.c index c9c6a2d3ff2..df54f338c37 100644 --- a/net/tcp/tcp_send_buffered.c +++ b/net/tcp/tcp_send_buffered.c @@ -511,7 +511,12 @@ static uint16_t psock_send_eventhandler(FAR struct net_driver_s *dev, { ninfo("Lost connection: %04x\n", flags); - if (psock->s_conn != NULL) + /* We could get here recursively through the callback actions of + * tcp_lost_connection(). So don't repeat that action if we have + * already been disconnected. + */ + + if (psock->s_conn != NULL && _SS_ISCONNECTED(psock->s_flags)) { /* Report not connected */ diff --git a/net/udp/udp_netpoll.c b/net/udp/udp_netpoll.c index 6a592eb80ea..19631b47ba4 100644 --- a/net/udp/udp_netpoll.c +++ b/net/udp/udp_netpoll.c @@ -203,7 +203,7 @@ int udp_pollsetup(FAR struct socket *psock, FAR struct pollfd *fds) /* Allocate a TCP/IP callback structure */ cb = udp_callback_alloc(info->dev, conn); - if (!cb) + if (cb == NULL) { ret = -EBUSY; goto errout_with_lock; diff --git a/net/usrsock/usrsock_poll.c b/net/usrsock/usrsock_poll.c index 1f918f998a0..59f69dbdf73 100644 --- a/net/usrsock/usrsock_poll.c +++ b/net/usrsock/usrsock_poll.c @@ -195,7 +195,7 @@ static int usrsock_pollsetup(FAR struct socket *psock, FAR struct pollfd *fds) /* Allocate a usrsock callback structure */ cb = devif_callback_alloc(NULL, &conn->list); - if (!cb) + if (cb == NULL) { ret = -EBUSY; kmm_free(info); /* fds->priv not set, so we need to free info here. */