[PATCH 0/2] Fix a Coverity reported socket leak

[PATCH 0/2] Fix a Coverity reported socket leak

[David Gibson]

I got coverity working for me again, version 2026.3.0.  This pointed
out a potentially fairly nasty socket leak in tcp_listen_handler().
Here's a series that fixes it.

David Gibson (2):
  tcp, tcp_splice: Make helper for setting SO_LINGER socket option
  tcp: Don't leak sockets on error paths

 tcp.c        | 42 ++++++++++++++++++++++++++----------------
 tcp_conn.h   |  3 +++
 tcp_splice.c | 20 +++-----------------
 3 files changed, 32 insertions(+), 33 deletions(-)

-- 
2.54.0

[PATCH 1/2] tcp, tcp_splice: Make helper for setting SO_LINGER socket option

[David Gibson]

Both spliced and non-spliced TCP in some cases set the SO_LINGER socket
option in order to to force a TCP RST on a socket side connection.  In each
case we open code the setsockopt() logic.  We're shortly going to add
another place that needs this, so move the setsockopt() and error handling
logic into a shared helper.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 tcp.c        | 33 ++++++++++++++++++++-------------
 tcp_conn.h   |  3 +++
 tcp_splice.c | 20 +++-----------------
 3 files changed, 26 insertions(+), 30 deletions(-)

diff --git a/tcp.c b/tcp.c
index d6a9ba28..1078bdc3 100644
--- a/tcp.c
+++ b/tcp.c
@@ -1395,29 +1395,36 @@ static int tcp_send_flag(const struct ctx *c, struct tcp_tap_conn *conn,
 }
 
 /**
- * tcp_sock_rst() - Close TCP connection forcing RST on socket side
- * @c:		Execution context
- * @conn:	Connection pointer
+ * tcp_linger0_() - Set SO_LINGER with 0 timeout on socket
+ * @f:		Flow header (only for debug logging)
+ * @s:		Socket to modify
  */
-static void tcp_sock_rst(const struct ctx *c, struct tcp_tap_conn *conn)
+void tcp_linger0_(const struct flow_common *f, int s)
 {
 	const struct linger linger0 = {
 		.l_onoff = 1,
 		.l_linger = 0,
 	};
 
-	/* Force RST on socket to inform the peer
-	 *
-	 * We do this by setting SO_LINGER with 0 timeout, which means that
-	 * close() will send an RST (unless the connection is already closed in
-	 * both directions).
+	/* Setting SO_LINGER with 0 timeout, means that close() will send an RST
+	 * (unless the connection is already closed in both directions).
 	 */
-	if (setsockopt(conn->sock, SOL_SOCKET,
-		       SO_LINGER, &linger0, sizeof(linger0)) < 0) {
-		flow_dbg_perror(conn,
-				"SO_LINGER failed, may not send RST to peer");
+	if (setsockopt(s, SOL_SOCKET, SO_LINGER,
+		       &linger0, sizeof(linger0)) < 0) {
+		flow_log_perror_(f, LOG_DEBUG,
+				 "SO_LINGER failed, may not send RST to peer");
 	}
+}
 
+/**
+ * tcp_sock_rst() - Close TCP connection forcing RST on socket side
+ * @c:		Execution context
+ * @conn:	Connection pointer
+ */
+static void tcp_sock_rst(const struct ctx *c, struct tcp_tap_conn *conn)
+{
+	/* Force RST on socket to inform the peer */
+	tcp_linger0(conn, conn->sock);
 	conn_event(c, conn, CLOSED);
 }
 
diff --git a/tcp_conn.h b/tcp_conn.h
index 9f5bee03..5f7af240 100644
--- a/tcp_conn.h
+++ b/tcp_conn.h
@@ -241,6 +241,9 @@ struct tcp_splice_conn {
 extern int init_sock_pool4	[TCP_SOCK_POOL_SIZE];
 extern int init_sock_pool6	[TCP_SOCK_POOL_SIZE];
 
+void tcp_linger0_(const struct flow_common *f, int s);
+#define tcp_linger0(flow_, s_)	tcp_linger0_(&(flow_)->f, (s_))
+
 bool tcp_flow_defer(const struct tcp_tap_conn *conn);
 
 int tcp_flow_repair_on(struct ctx *c, const struct tcp_tap_conn *conn);
diff --git a/tcp_splice.c b/tcp_splice.c
index 42ee8abc..4c18f0c4 100644
--- a/tcp_splice.c
+++ b/tcp_splice.c
@@ -240,28 +240,14 @@ static void conn_event_do(struct tcp_splice_conn *conn, unsigned long event)
  */
 static void tcp_splice_rst(struct tcp_splice_conn *conn)
 {
-	const struct linger linger0 = {
-		.l_onoff = 1,
-		.l_linger = 0,
-	};
 	unsigned sidei;
 
 	if (conn->flags & CLOSING)
 		return; /* Nothing to do */
 
-	/* Force RST on sockets to inform the peer
-	 *
-	 * We do this by setting SO_LINGER with 0 timeout, which means that
-	 * close() will send an RST (unless the connection is already closed in
-	 * both directions).
-	 */
-	flow_foreach_sidei(sidei) {
-		if (setsockopt(conn->s[sidei], SOL_SOCKET,
-			       SO_LINGER, &linger0, sizeof(linger0)) < 0) {
-			flow_dbg_perror(conn,
-"SO_LINGER failed, may not send RST to peer");
-		}
-	}
+	/* Force RST on sockets to inform the peer */
+	flow_foreach_sidei(sidei)
+		tcp_linger0(conn, conn->s[sidei]);
 
 	conn_flag(conn, CLOSING);
 }
-- 
2.54.0

[PATCH 2/2] tcp: Don't leak sockets on error paths

[David Gibson]

tcp_listen_handler() has several error paths that will cancel the creation
of a new flow, after having accept()ed an incoming socket connection.
Coverity pointed out that in those cases we leak the new socket.  Correct
this by properly closing the socket.  Make sure to also set SO_LINGER so
that the peer will get an RST.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 tcp.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/tcp.c b/tcp.c
index 1078bdc3..652c68a5 100644
--- a/tcp.c
+++ b/tcp.c
@@ -2575,11 +2575,11 @@ void tcp_listen_handler(const struct ctx *c, union epoll_ref ref,
 
 		err("Invalid endpoint from TCP accept(): %s",
 		    sockaddr_ntop(&sa, sastr, sizeof(sastr)));
-		goto cancel;
+		goto rst;
 	}
 
 	if (!flow_target(c, flow, ref.listen.rule, IPPROTO_TCP))
-		goto cancel;
+		goto rst;
 
 	switch (flow->f.pif[TGTSIDE]) {
 	case PIF_SPLICE:
@@ -2595,11 +2595,14 @@ void tcp_listen_handler(const struct ctx *c, union epoll_ref ref,
 		flow_err(flow, "No support for forwarding TCP from %s to %s",
 			 pif_name(flow->f.pif[INISIDE]),
 			 pif_name(flow->f.pif[TGTSIDE]));
-		goto cancel;
+		goto rst;
 	}
 
 	return;
 
+rst:
+	tcp_linger0(flow, s);
+	close(s);
 cancel:
 	flow_alloc_cancel(flow);
 }
-- 
2.54.0

Re: [PATCH 0/2] Fix a Coverity reported socket leak

[Stefano Brivio]

On Wed, 13 May 2026 17:18:19 +1000
David Gibson <david@gibson.dropbear.id.au> wrote:

> I got coverity working for me again, version 2026.3.0.  This pointed
> out a potentially fairly nasty socket leak in tcp_listen_handler().
> Here's a series that fixes it.

As we discussed in the call on Monday, Jon was going to look into
those.

Cc'ing him for awareness, as maybe he doesn't need to look into those
anymore (but I'm not sure if you already tried passing the '--security'
switch).

-- 
Stefano