[incubator-nuttx] branch master updated: net/tcp: fix assertion of fallback connection alloc

[ma...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

masayuki pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-nuttx.git


The following commit(s) were added to refs/heads/master by this push:
     new 9bdeed73e2 net/tcp: fix assertion of fallback connection alloc
9bdeed73e2 is described below

commit 9bdeed73e232cc57926af68c137620f01414b137
Author: chao.an <an...@xiaomi.com>
AuthorDate: Sat Jul 9 03:30:10 2022 +0800

    net/tcp: fix assertion of fallback connection alloc
    
    When the free connection list is unenough to alloc a new instance,
    the TCP stack will reuse the currently closed connection, but if
    the handle is not released by the user via close(2), the reference
    count of the connection remains in a non-zero value, it will cause
    the assertion to fail, so when the handle is not released we should
    not use such a conn instance when being actively closed, and ensure
    that the reference count is assigned within the net lock protection
    
    |(gdb) bt
    |#0  up_assert (filename=0x565c78f7 "tcp/tcp_conn.c", lineno=771) at sim/up_assert.c:75
    |#1  0x56566177 in _assert (filename=0x565c78f7 "tcp/tcp_conn.c", linenum=771) at assert/lib_assert.c:36
    |#2  0x5657d620 in tcp_free (conn=0x565fb3e0 <g_tcp_connections>) at tcp/tcp_conn.c:771
    |#3  0x5657d5a1 in tcp_alloc (domain=2 '\002') at tcp/tcp_conn.c:700
    |#4  0x565b1f50 in inet_tcp_alloc (psock=0xf3dea150) at inet/inet_sockif.c:144
    |#5  0x565b2082 in inet_setup (psock=0xf3dea150, protocol=0) at inet/inet_sockif.c:253
    |#6  0x565b1bf0 in psock_socket (domain=2, type=1, protocol=0, psock=0xf3dea150) at socket/socket.c:121
    |#7  0x56588f5f in socket (domain=2, type=1, protocol=0) at socket/socket.c:278
    |#8  0x565b11c0 in hello_main (argc=1, argv=0xf3dfab10) at hello_main.c:35
    |#9  0x56566631 in nxtask_startup (entrypt=0x565b10ef <hello_main>, argc=1, argv=0xf3dfab10) at sched/task_startup.c:70
    |#10 0x565597fa in nxtask_start () at task/task_start.c:134
    
    Signed-off-by: chao.an <an...@xiaomi.com>
---
 net/tcp/tcp_close.c |  7 ++++++-
 net/tcp/tcp_conn.c  | 13 ++++++-------
 2 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/net/tcp/tcp_close.c b/net/tcp/tcp_close.c
index b3a81a80ce..4dca880ae8 100644
--- a/net/tcp/tcp_close.c
+++ b/net/tcp/tcp_close.c
@@ -294,6 +294,10 @@ static inline int tcp_close_disconnect(FAR struct socket *psock)
     }
 #endif
 
+  /* Discard our reference to the connection */
+
+  conn->crefs = 0;
+
   /* TCP_ESTABLISHED
    *   We need to initiate an active close and wait for its completion.
    *
@@ -327,6 +331,8 @@ static inline int tcp_close_disconnect(FAR struct socket *psock)
       tcp_free(conn);
     }
 
+  psock->s_conn = NULL;
+
   net_unlock();
   return ret;
 }
@@ -356,7 +362,6 @@ int tcp_close(FAR struct socket *psock)
   /* Perform the disconnection now */
 
   tcp_unlisten(conn); /* No longer accepting connections */
-  conn->crefs = 0;    /* Discard our reference to the connection */
 
   /* Break any current connections and close the socket */
 
diff --git a/net/tcp/tcp_conn.c b/net/tcp/tcp_conn.c
index aad46069a5..018622e6fe 100644
--- a/net/tcp/tcp_conn.c
+++ b/net/tcp/tcp_conn.c
@@ -652,15 +652,13 @@ FAR struct tcp_conn_s *tcp_alloc(uint8_t domain)
 
           /* Is this connection in a state we can sacrifice. */
 
-          /* REVISIT: maybe we could check for SO_LINGER but it's buried
-           * in the socket layer.
-           */
-
-          if (tmp->tcpstateflags == TCP_CLOSING    ||
+          if ((tmp->crefs == 0) &&
+              (tmp->tcpstateflags == TCP_CLOSED    ||
+              tmp->tcpstateflags == TCP_CLOSING    ||
               tmp->tcpstateflags == TCP_FIN_WAIT_1 ||
               tmp->tcpstateflags == TCP_FIN_WAIT_2 ||
               tmp->tcpstateflags == TCP_TIME_WAIT  ||
-              tmp->tcpstateflags == TCP_LAST_ACK)
+              tmp->tcpstateflags == TCP_LAST_ACK))
             {
               /* Yes.. Is it the oldest one we have seen so far? */
 
@@ -767,9 +765,10 @@ void tcp_free(FAR struct tcp_conn_s *conn)
    * operation.
    */
 
-  DEBUGASSERT(conn->crefs == 0);
   net_lock();
 
+  DEBUGASSERT(conn->crefs == 0);
+
   tcp_stop_timer(conn);
 
   /* Free remaining callbacks, actually there should be only the send