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

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>
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>

1 files changed, 5 insertions, 4 deletions

diff --git a/tcp.c b/tcp.c
index e91c0cf..3202d33 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);