TCP: Fix some potential error conditions that could result from deferring the connection until the full 3-way handshake has completed.

net/tcp/Kconfig

@@ -156,13 +156,6 @@ config NET_TCPBACKLOG
 		Incoming connections pend in a backlog until accept() is called.
 		The size of the backlog is selected when listen() is called.
 
-config NET_ACCEPT_ON_ACK
-	bool "accept() returns on TCP 3-Way Handshake completion"
-	default n
-	---help---
-		accept() returns after 3-Way Handshake is complete (SYN, SYN/ACK, ACK).
-		Without this flag it returns after SYN packet is received.
-
 config NET_TCP_SPLIT
 	bool "Enable packet splitting"
 	default n

net/tcp/tcp.h

@@ -697,6 +697,19 @@ void tcp_timer(FAR struct net_driver_s *dev, FAR struct tcp_conn_s *conn,
 
 void tcp_listen_initialize(void);
 
+/****************************************************************************
+ * Name: tcp_findlistener
+ *
+ * Description:
+ *   Return the connection listener for connections on this port (if any)
+ *
+ * Assumptions:
+ *   Called at interrupt level
+ *
+ ****************************************************************************/
+
+FAR struct tcp_conn_s *tcp_findlistener(uint16_t portno);
+
 /****************************************************************************
  * Name: tcp_unlisten
  *

net/tcp/tcp_input.c

@@ -179,23 +179,25 @@ static void tcp_input(FAR struct net_driver_s *dev, unsigned int iplen)
           conn = tcp_alloc_accept(dev, tcp);
           if (conn)
             {
-              /* The connection structure was successfully allocated.  Now see if
+              /* The connection structure was successfully allocated and has
-               * there is an application waiting to accept the connection (or at
+               * been initialized in the TCP_SYN_RECVD state.  The expected
-               * least queue it it for acceptance).
+               * sequence of events is then the rest of the 3-way handshake:
+               *
+               *  1. We just received a TCP SYN packet from a remote host.
+               *  2. We will send the SYN-ACK response below (perhaps
+               *     repeatedly in the event of a timeout)
+               *  3. Then we expect to receive an ACK from the remote host
+               *     indicated the TCP socket connection is ESTABLISHED.
+               *
+               * Possible failure:
+               *
+               *  1. The ACK is never received.  This will be handled by
+               *     a timeout managed by tcp_timer().
+               *  2. The listener "unlistens()".  This will be handled by
+               *     the failure of tcp_accept_connection() when the ACK is received.
                */
 
               conn->crefs = 1;
-
-#ifndef CONFIG_NET_ACCEPT_ON_ACK
-              if (tcp_accept_connection(dev, conn, tmp16) != OK)
-                {
-                  /* No, then we have to give the connection back and drop the packet */
-
-                  conn->crefs = 0;
-                  tcp_free(conn);
-                  conn = NULL;
-                }
-#endif
             }
 
           if (!conn)
@@ -465,9 +467,14 @@ found:
 
         if ((flags & TCP_ACKDATA) != 0)
           {
+            /* The three way handshake is complete and the TCP connection
+             * is now in the ESTABLISHED state.
+             */
+
             conn->tcpstateflags = TCP_ESTABLISHED;
 
-#ifdef CONFIG_NET_ACCEPT_ON_ACK
+            /* Wake up any listener waiting for a connection on this port */
+
             if (tcp_accept_connection(dev, conn, tcp->destport) != OK)
               {
                 /* No more listener for current port.  We can free conn here
@@ -482,7 +489,6 @@ found:
                 conn = NULL;
                 goto drop;
               }
-#endif
 
 #ifdef CONFIG_NET_TCP_WRITE_BUFFERS
             conn->isn           = tcp_getsequence(tcp->ackno);

net/tcp/tcp_listen.c

@@ -257,7 +257,7 @@ int tcp_accept_connection(FAR struct net_driver_s *dev,
    */
 
   listener = tcp_findlistener(portno);
-  if (listener)
+  if (listener != NULL)
     {
       /* Yes, there is a listener.  Is it accepting connections now? */
 

net/tcp/tcp_timer.c

@@ -218,17 +218,57 @@ void tcp_timer(FAR struct net_driver_s *dev, FAR struct tcp_conn_s *conn,
                   goto done;
                 }
 #endif
-              /* Should we close the connection? */
+              /* Check for a timeout on connection in the TCP_SYN_RCVD state.
+               * On such timeouts, we would normally resend the SYNACK until
+               * the ACK is received, completing the 3-way handshek.  But if
+               * the retry count elapsed, then we must assume that no ACK is
+               * forthcoming and terminate the attempted connection.
+               */
 
-              if (
+              if (conn->tcpstateflags == TCP_SYN_RCVD &&
+                  conn->nrtx >= TCP_MAXSYNRTX)
+                {
+                  FAR struct tcp_conn_s *listener;
+
+                  conn->tcpstateflags = TCP_CLOSED;
+                  ninfo("TCP state: TCP_CLOSED\n");
+
+                  /* Find the listener for this connectins */
+
+                  listener = tcp_findlistener(conn->lport);
+                  if (listener != NULL)
+                    {
+                      /* We call tcp_callback() for the connection with
+                       * TCP_TIMEDOUT to inform the listener that the
+                       * connection has timed out.
+                       */
+
+                      result = tcp_callback(dev, listener, TCP_TIMEDOUT);
+                    }
+
+                  /* We also send a reset packet to the remote host. */
+
+                  tcp_send(dev, conn, TCP_RST | TCP_ACK, hdrlen);
+
+                  /* Finally, we must free this TCP connection structure */
+
+                  tcp_free(conn);
+                  goto done;
+                }
+
+              /* Otherwise, check for a timeout on an established connection.
+               * If the retry count is exceeded in this case, we should
+               * close the connection.
+               */
+
+              else if (
 #ifdef CONFIG_NET_TCP_WRITE_BUFFERS
                   conn->expired > 0 ||
 #else
-                  conn->nrtx == TCP_MAXRTX ||
+                  conn->nrtx >= TCP_MAXRTX ||
 #endif
-                  ((conn->tcpstateflags == TCP_SYN_SENT ||
+                  (conn->tcpstateflags == TCP_SYN_SENT &&
-                    conn->tcpstateflags == TCP_SYN_RCVD) &&
+                   conn->nrtx >= TCP_MAXSYNRTX)
-                    conn->nrtx == TCP_MAXSYNRTX)
                  )
                 {
                   conn->tcpstateflags = TCP_CLOSED;