[PATCH] tcp: Properly remove sockets from epoll loop when connection is closed

[PATCH] tcp: Properly remove sockets from epoll loop when connection is closed

[David Gibson]

Most of the handling for closing a TCP connectin is in conn_event_do() when
it receives a 'CLOSED' event.  We specifically check for this case and,
correctly, remove the connection from the flow hash table.  However, we
also bypass the call tp tcp_epoll_ctl() which is not correct.  By skipping
tcp_epoll_ctl() we skip it's specific handling of the CLOSED event, which
includes removing the TCP socket from epoll.

If we somehow get an event on such a stale socket, we'll get a stale flow
reference.  That flow slot might have been re-used, leading to to a crash
in conn_at_sidx().

Fixes: b86afe3559c0 ("tcp: Don't defer hash table removal")
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 tcp.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/tcp.c b/tcp.c
index e91c0cf5..3202d338 100644
--- a/tcp.c
+++ b/tcp.c
@@ -694,12 +694,13 @@ void conn_event_do(const struct ctx *c, struct tcp_tap_conn *conn,
 		flow_dbg(conn, "%s",
 			 num == -1 	       ? "CLOSED" : tcp_event_str[num]);
 
-	if (event == CLOSED)
-		flow_hash_remove(c, TAP_SIDX(conn));
-	else if ((event == TAP_FIN_RCVD) && !(conn->events & SOCK_FIN_RCVD))
+	if ((event == TAP_FIN_RCVD) && !(conn->events & SOCK_FIN_RCVD)) {
 		conn_flag(c, conn, ACTIVE_CLOSE);
-	else
+	} else {
+		if (event == CLOSED)
+			flow_hash_remove(c, TAP_SIDX(conn));
 		tcp_epoll_ctl(c, conn);
+	}
 
 	if (CONN_HAS(conn, SOCK_FIN_SENT | TAP_FIN_ACKED))
 		tcp_timer_ctl(conn);
-- 
2.51.1

Re: [PATCH] tcp: Properly remove sockets from epoll loop when connection is closed

[Stefano Brivio]

On Tue, 11 Nov 2025 14:25:20 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> Most of the handling for closing a TCP connectin is in conn_event_do() when
> it receives a 'CLOSED' event.  We specifically check for this case and,
> correctly, remove the connection from the flow hash table.  However, we
> also bypass the call tp tcp_epoll_ctl() which is not correct.  By skipping
> tcp_epoll_ctl() we skip it's specific handling of the CLOSED event, which
> includes removing the TCP socket from epoll.
> 
> If we somehow get an event on such a stale socket, we'll get a stale flow
> reference.  That flow slot might have been re-used, leading to to a crash
> in conn_at_sidx().
> 
> Fixes: b86afe3559c0 ("tcp: Don't defer hash table removal")
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>

Applied.

-- 
Stefano