Skip to content

Commit

Permalink
net/tcp: fix assertion of fallback connection alloc
Browse files Browse the repository at this point in the history
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
|apache#7  0x56588f5f in socket (domain=2, type=1, protocol=0) at socket/socket.c:278
|apache#8  0x565b11c0 in hello_main (argc=1, argv=0xf3dfab10) at hello_main.c:35
|apache#9  0x56566631 in nxtask_startup (entrypt=0x565b10ef <hello_main>, argc=1, argv=0xf3dfab10) at sched/task_startup.c:70
|apache#10 0x565597fa in nxtask_start () at task/task_start.c:134

Signed-off-by: chao.an <anchao@xiaomi.com>
  • Loading branch information
anchao authored and masayuki2009 committed Jul 9, 2022
1 parent cd3e93e commit 9bdeed7
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 8 deletions.
7 changes: 6 additions & 1 deletion net/tcp/tcp_close.c
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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 */

Expand Down
13 changes: 6 additions & 7 deletions net/tcp/tcp_conn.c
Original file line number Diff line number Diff line change
Expand Up @@ -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? */

Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 9bdeed7

Please sign in to comment.