Index | Thread | Search

From:
Alexander Bluhm <bluhm@openbsd.org>
Subject:
socket leak in tcp syn cache
To:
tech@openbsd.org
Date:
Tue, 10 Jun 2025 22:29:15 +0200

Download raw body.

Thread
Hi,

In my recent reference counting diff I have introduces a socket
leak.

The TCP SYN cache called soref() to avoid freeing the socket when
it was working with it.  But the unref got lost when I moved the
socket ref count into the inp.

We have to hold the reference over tcp_drop() in the abort case to
unlock the socket afterwards.  But tcp_drop() removes the inpcb
from the table and drops this reference.  Call in_pcbref() instead
of soref() to have references for both inp and so.  After tcp_drop()
call in_pcbsounlock() and in_pcbunref().  Then the memory is freed
in the final step.  While there move m_freem() out of the socket
lock.

When syn_cache_get() returns the socket successfully, keep the
reference count on the inp.  Then tcp_input() can work with this
inpcb and unref it at the end.

ok?

bluhm

Index: netinet/tcp_input.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/tcp_input.c,v
diff -u -p -r1.450 tcp_input.c
--- netinet/tcp_input.c	3 Jun 2025 16:51:26 -0000	1.450
+++ netinet/tcp_input.c	10 Jun 2025 16:40:01 -0000
@@ -793,7 +793,8 @@ findpcb:
 					 * full-blown connection.
 					 */
 					in_pcbunref(inp);
-					inp = in_pcbref(sotoinpcb(so));
+					/* syn_cache_get() has refcounted inp */
+					inp = sotoinpcb(so);
 					tp = intotcpcb(inp);
 					if (tp == NULL)
 						goto badsyn;	/*XXX*/
@@ -3657,8 +3658,8 @@ syn_cache_get(struct sockaddr *src, stru
 	if (so == NULL)
 		goto resetandabort;
 	soassertlocked(so);
-	soref(so);
-	inp = sotoinpcb(so);
+	/* inpcb does refcount socket, both so and inp cannot go away */
+	inp = in_pcbref(sotoinpcb(so));
 	tp = intotcpcb(inp);
 
 #ifdef IPSEC
@@ -3778,9 +3779,10 @@ resetandabort:
 abort:
 	if (tp != NULL)
 		tp = tcp_drop(tp, ECONNABORTED);	/* destroys socket */
-	m_freem(m);
 	in_pcbsounlock(inp, so);
 	in_pcbsounlock(listeninp, listenso);
+	in_pcbunref(inp);
+	m_freem(m);
 	syn_cache_put(sc);
 	tcpstat_inc(tcps_sc_aborted);
 	return ((struct socket *)(-1));