From 0f7a52bc2831f3d2f8f09a4c538e963bdf2f7b93 Mon Sep 17 00:00:00 2001 From: Gregory Nutt Date: Tue, 29 Aug 2017 12:27:58 -0600 Subject: [PATCH] Networking: Fix a runaway recursion problem introduced the previous fixe for shutting down dup'ed sockets. --- net/inet/inet_close.c | 2 +- net/inet/inet_recvfrom.c | 8 +------ net/sixlowpan/sixlowpan_tcpsend.c | 6 +++--- net/socket/net_clone.c | 9 ++++++++ net/tcp/tcp.h | 22 ++++++++++++------- net/tcp/tcp_connect.c | 4 ++-- net/tcp/tcp_monitor.c | 35 ++++++++++++++++++++++++------- net/tcp/tcp_netpoll.c | 14 +++++++++---- net/tcp/tcp_send_buffered.c | 6 +++--- net/tcp/tcp_send_unbuffered.c | 6 +++--- net/tcp/tcp_sendfile.c | 18 ++++++++++------ 11 files changed, 86 insertions(+), 44 deletions(-) diff --git a/net/inet/inet_close.c b/net/inet/inet_close.c index f372f87950..8a1c34d1e6 100644 --- a/net/inet/inet_close.c +++ b/net/inet/inet_close.c @@ -508,7 +508,7 @@ int inet_close(FAR struct socket *psock) /* Stop the network monitor */ - tcp_stop_monitor(conn); + tcp_stop_monitor(conn, TCP_CLOSE); } else { diff --git a/net/inet/inet_recvfrom.c b/net/inet/inet_recvfrom.c index bc8768e136..c795c3c3d5 100644 --- a/net/inet/inet_recvfrom.c +++ b/net/inet/inet_recvfrom.c @@ -737,15 +737,9 @@ static uint16_t inet_tcp_interrupt(FAR struct net_driver_s *dev, { ninfo("Lost connection\n"); - /* Stop further callbacks */ - - pstate->ir_cb->flags = 0; - pstate->ir_cb->priv = NULL; - pstate->ir_cb->event = NULL; - /* Handle loss-of-connection event */ - tcp_lost_connection(pstate->ir_sock, flags); + tcp_lost_connection(pstate->ir_sock, pstate->ir_cb, flags); /* Check if the peer gracefully closed the connection. */ diff --git a/net/sixlowpan/sixlowpan_tcpsend.c b/net/sixlowpan/sixlowpan_tcpsend.c index f5c9f9ab2c..cbcf4c32f0 100644 --- a/net/sixlowpan/sixlowpan_tcpsend.c +++ b/net/sixlowpan/sixlowpan_tcpsend.c @@ -439,11 +439,11 @@ static uint16_t tcp_send_interrupt(FAR struct net_driver_s *dev, else if ((flags & TCP_DISCONN_EVENTS) != 0) { - /* Report not connected */ - ninfo("Lost connection\n"); - tcp_lost_connection(sinfo->s_sock, flags); + /* Report the disconnection event to all socket clones */ + + tcp_lost_connection(sinfo->s_sock, sinfo->s_cb, flags); sinfo->s_result = -ENOTCONN; goto end_wait; } diff --git a/net/socket/net_clone.c b/net/socket/net_clone.c index dcaf92ecce..a22c1c7ea1 100644 --- a/net/socket/net_clone.c +++ b/net/socket/net_clone.c @@ -62,6 +62,15 @@ * Description: * Performs the low level, common portion of net_dupsd() and net_dupsd2() * + * Input Parameters: + * psock1 - The existing socket that is being cloned. + * psock2 - A reference to an uninitialized socket structure alloated by + * the caller. + * + * Returned Value: + * Zero (OK) is returned on success; a negated errno value is returned on + * any failure. + * ****************************************************************************/ int net_clone(FAR struct socket *psock1, FAR struct socket *psock2) diff --git a/net/tcp/tcp.h b/net/tcp/tcp.h index 2e30877885..114d76456b 100644 --- a/net/tcp/tcp.h +++ b/net/tcp/tcp.h @@ -557,10 +557,12 @@ int tcp_start_monitor(FAR struct socket *psock); * Name: tcp_stop_monitor * * Description: - * Stop monitoring TCP connection changes for a given socket + * Stop monitoring TCP connection changes for a sockets associated with + * a given TCP connection stucture. * * Input Parameters: * conn - The TCP connection of interest + * flags Set of disconnection events * * Returned Value: * None @@ -571,27 +573,33 @@ int tcp_start_monitor(FAR struct socket *psock); * ****************************************************************************/ -void tcp_stop_monitor(FAR struct tcp_conn_s *conn); +void tcp_stop_monitor(FAR struct tcp_conn_s *conn, uint16_t flags); /**************************************************************************** * Name: tcp_lost_connection * * Description: - * Called when a loss-of-connection event has occurred. + * Called when a loss-of-connection event has been detected by network + * "interrupt" handling logic. Perform operations like tcp_stop_monitor + * but (1) explicitly amek this socket and (2) disable further callbacks + * the "interrupt handler". * * Parameters: - * psock The TCP socket structure associated. - * flags Set of connection events events + * psock - The TCP socket structure associated. + * cb - devif callback structure + * flags - Set of connection events events * * Returned Value: * None * * Assumptions: - * The caller holds the network lock. + * The caller holds the network lock (if not, it will be locked momentarily + * by this function). * ****************************************************************************/ -void tcp_lost_connection(FAR struct socket *psock, uint16_t flags); +void tcp_lost_connection(FAR struct socket *psock, + FAR struct devif_callback_s *cb, uint16_t flags); /**************************************************************************** * Name: tcp_ipv4_select diff --git a/net/tcp/tcp_connect.c b/net/tcp/tcp_connect.c index 44a6e98e4b..62e6f4a5d7 100644 --- a/net/tcp/tcp_connect.c +++ b/net/tcp/tcp_connect.c @@ -152,7 +152,7 @@ static void psock_teardown_callbacks(FAR struct tcp_connect_s *pstate, { /* Failed to connect. Stop the connection event monitor */ - tcp_stop_monitor(conn); + tcp_stop_monitor(conn, TCP_CLOSE); } } @@ -411,7 +411,7 @@ int psock_tcp_connect(FAR struct socket *psock, * happen in this context, but just in case... */ - tcp_lost_connection(psock, TCP_ABORT); + tcp_stop_monitor(psock->s_conn, TCP_ABORT); } } } diff --git a/net/tcp/tcp_monitor.c b/net/tcp/tcp_monitor.c index 5c9c4863c8..e092bf4d5e 100644 --- a/net/tcp/tcp_monitor.c +++ b/net/tcp/tcp_monitor.c @@ -302,11 +302,12 @@ int tcp_start_monitor(FAR struct socket *psock) * Name: tcp_stop_monitor * * Description: - * Stop monitoring TCP connection changes for a given socket. This is part - * of a graceful shutdown. + * Stop monitoring TCP connection changes for a sockets associated with + * a given TCP connection stucture. * * Input Parameters: * conn - The TCP connection of interest + * flags Set of disconnection events * * Returned Value: * None @@ -317,24 +318,27 @@ int tcp_start_monitor(FAR struct socket *psock) * ****************************************************************************/ -void tcp_stop_monitor(FAR struct tcp_conn_s *conn) +void tcp_stop_monitor(FAR struct tcp_conn_s *conn, uint16_t flags) { DEBUGASSERT(conn != NULL); /* Stop the network monitor */ - tcp_shutdown_monitor(conn, TCP_CLOSE); + tcp_shutdown_monitor(conn, flags); } /**************************************************************************** * Name: tcp_lost_connection * * Description: - * Called when a loss-of-connection event has occurred. This is for an - * unexpected disconnection of some sort from the host. + * Called when a loss-of-connection event has been detected by network + * "interrupt" handling logic. Perform operations like tcp_stop_monitor + * but (1) explicitly amek this socket and (2) disable further callbacks + * the "interrupt handler". * * Parameters: * psock - The TCP socket structure associated. + * cb - devif callback structure * flags - Set of connection events events * * Returned Value: @@ -346,11 +350,26 @@ void tcp_stop_monitor(FAR struct tcp_conn_s *conn) * ****************************************************************************/ -void tcp_lost_connection(FAR struct socket *psock, uint16_t flags) +void tcp_lost_connection(FAR struct socket *psock, + FAR struct devif_callback_s *cb, uint16_t flags) { DEBUGASSERT(psock != NULL && psock->s_conn != NULL); - /* Stop the network monitor */ + /* Nullify the callback structure so that recursive callbacks are not + * received by the "interrupt handler" due to disconnection processing. + */ + + cb->flags = 0; + cb->priv = NULL; + cb->event = NULL; + + /* Make sure that this socket is explicitly marked. It may not bet a + * callback due to the above nullification. + */ + + tcp_close_connection(psock, flags); + + /* Then stop the network monitor */ tcp_shutdown_monitor((FAR struct tcp_conn_s *)psock->s_conn, flags); } diff --git a/net/tcp/tcp_netpoll.c b/net/tcp/tcp_netpoll.c index 27e5c71e8e..7fc3937e2c 100644 --- a/net/tcp/tcp_netpoll.c +++ b/net/tcp/tcp_netpoll.c @@ -73,8 +73,8 @@ struct tcp_poll_s * Name: tcp_poll_interrupt * * Description: - * This function is called from the interrupt level to perform the actual - * TCP receive operation via by the device interface layer. + * This function is called to perform the actual TCP receive operation via + * the device interface layer. * * Parameters: * dev The structure of the network driver that caused the interrupt @@ -124,14 +124,20 @@ static uint16_t tcp_poll_interrupt(FAR struct net_driver_s *dev, FAR void *conn, { /* Mark that the connection has been lost */ - tcp_lost_connection(info->psock, flags); + tcp_lost_connection(info->psock, pstate->cb, flags); eventset |= (POLLERR | POLLHUP); } /* Awaken the caller of poll() if requested event occurred. */ - if (eventset) + if (eventset != 0) { + /* Stop further callbacks */ + + pstate->cb->flags = 0; + pstate->cb->priv = NULL; + pstate->cb->event = NULL; + info->fds->revents |= eventset; sem_post(info->fds->sem); } diff --git a/net/tcp/tcp_send_buffered.c b/net/tcp/tcp_send_buffered.c index ede040e22b..3b8f6061dd 100644 --- a/net/tcp/tcp_send_buffered.c +++ b/net/tcp/tcp_send_buffered.c @@ -515,7 +515,7 @@ static uint16_t psock_send_interrupt(FAR struct net_driver_s *dev, { /* Report not connected */ - tcp_lost_connection(psock, flags); + tcp_lost_connection(psock, psock->s_sndcb, flags); } /* Free write buffers and terminate polling */ @@ -1030,14 +1030,14 @@ ssize_t psock_tcp_send(FAR struct socket *psock, FAR const void *buf, /* Allocate resources to receive a callback */ - if (!psock->s_sndcb) + if (psock->s_sndcb == NULL) { psock->s_sndcb = tcp_callback_alloc(conn); } /* Test if the callback has been allocated */ - if (!psock->s_sndcb) + if (psock->s_sndcb == NULL) { /* A buffer allocation error occurred */ diff --git a/net/tcp/tcp_send_unbuffered.c b/net/tcp/tcp_send_unbuffered.c index a5c61224c3..3313dc5c69 100644 --- a/net/tcp/tcp_send_unbuffered.c +++ b/net/tcp/tcp_send_unbuffered.c @@ -396,11 +396,11 @@ static uint16_t tcpsend_interrupt(FAR struct net_driver_s *dev, else if ((flags & TCP_DISCONN_EVENTS) != 0) { - /* Report not connected */ - ninfo("Lost connection\n"); - tcp_lost_connection(pstate->snd_sock, flags); + /* Report not connected */ + + tcp_lost_connection(pstate->snd_sock, pstate->snd_cb, flags); pstate->snd_sent = -ENOTCONN; goto end_wait; } diff --git a/net/tcp/tcp_sendfile.c b/net/tcp/tcp_sendfile.c index eb17e6b8ac..7da57d02e0 100644 --- a/net/tcp/tcp_sendfile.c +++ b/net/tcp/tcp_sendfile.c @@ -222,14 +222,20 @@ static uint16_t ack_interrupt(FAR struct net_driver_s *dev, FAR void *pvconn, else if ((flags & TCP_DISCONN_EVENTS) != 0) { - /* Report not connected */ - nwarn("WARNING: Lost connection\n"); - tcp_lost_connection(pstate->snd_sock, flags); + /* Report not connected */ + + tcp_lost_connection(pstate->snd_sock, pstate->snd_ackcb, flags); pstate->snd_sent = -ENOTCONN; } + /* Prohibit further callbacks */ + + pstate->snd_ackcb->flags = 0; + pstate->snd_ackcb->priv = NULL; + pstate->snd_ackcb->event = NULL; + /* Wake up the waiting thread */ sem_post(&pstate->snd_sem); @@ -344,11 +350,11 @@ static uint16_t sendfile_interrupt(FAR struct net_driver_s *dev, FAR void *pvcon if ((flags & TCP_DISCONN_EVENTS) != 0) { - /* Report not connected */ - nwarn("WARNING: Lost connection\n"); - tcp_lost_connection(pstate->snd_sock, flags); + /* Report not connected */ + + tcp_lost_connection(pstate->snd_sock, pstate->snd_datacb, flags); pstate->snd_sent = -ENOTCONN; goto end_wait; }