tcp: Don't skip TIME_WAIT state for loopback sockets

This fixes a SEGFAULT in the tcp add-on reported in issue #15952. See
that issue for some analysis.

The short version is that, when closing a session over the loopback
interface, there is a special branch which skips the TIME_WAIT state
and instead just releases the socket while handling a RST/ACK
segment. If the timing is right this can lead to the reference
counts becoming imbalanced, leading to the code in tcp_receive_data
segfaulting when it tries to release the reference it acquired from
EndpointManager::FindConnection.

I can't find any other systems which skip the TIME_WAIT state with
loopback sessions, and I'm not entirely certain that it's a totally
safe thing to do anyway. This patch instead just treats local sessions
the same way it does a remote session and uses the TIME_WAIT state.

Any workload which creates and discards lots of ephemeral sockets can
just use SO_REUSEADDR to handle this situation like any other system.

To add a final bit of safety, the only place where a net_socket can be
used after calling gSocketModule->release_socket(net_socket*) is in
tcp_receive_data(). release_socket() returns true if the reference
count falls to zero, deleting the socket. There was an unused segment
action flag DELETE_ENDPOINT that I renamed to DELETED_ENDPOINT, which
is used by tcp_receive_data to know whether its safe to release its
reference to the socket after calling TCPEndpoint::SegmentReceived().

Change-Id: I2652fb225c3c8419234cfd627f74ff2de8402003
Reviewed-on: https://review.haiku-os.org/c/haiku/+/2793
Reviewed-by: Axel Dörfler <[email protected]>
Reviewed-by: waddlesplash <[email protected]>

src/add-ons/kernel/network/protocols/tcp/TCPEndpoint.cpp

@@ -1151,12 +1151,6 @@ TCPEndpoint::_EnterTimeWait()
 
 	if (fState == TIME_WAIT) {
 		_CancelConnectionTimers();
-
-		if (IsLocal()) {
-			// we do not use TIME_WAIT state for local connections
-			fFlags |= FLAG_DELETE_ON_CLOSE;
-			return;
-		}
 	}
 
 	_UpdateTimeWait();
@@ -1905,8 +1899,10 @@ TCPEndpoint::SegmentReceived(tcp_segment_header& segment, net_buffer* buffer)
 
 	if ((fFlags & (FLAG_CLOSED | FLAG_DELETE_ON_CLOSE))
 			== (FLAG_CLOSED | FLAG_DELETE_ON_CLOSE)) {
+
 		locker.Unlock();
-		gSocketModule->release_socket(socket);
+		if (gSocketModule->release_socket(socket))
+			segmentAction |= DELETED_ENDPOINT;
 	}
 
 	return segmentAction;

src/add-ons/kernel/network/protocols/tcp/tcp.cpp

@@ -11,6 +11,7 @@
 
 #include "EndpointManager.h"
 #include "TCPEndpoint.h"
+#include "tcp.h"
 
 #include <net_protocol.h>
 #include <net_stat.h>
@@ -710,7 +711,14 @@ tcp_receive_data(net_buffer* buffer)
 		buffer->destination, buffer->source);
 	if (endpoint != NULL) {
 		segmentAction = endpoint->SegmentReceived(segment, buffer);
-		gSocketModule->release_socket(endpoint->socket);
+
+		// There are some states in which the socket could have been deleted
+		// while handling a segment. If this flag is set in segmentAction
+		// then we know the socket has been freed and can skip releasing
+		// the reference acquired in EndpointManager::FindConnection()
+		// above.
+		if ((segmentAction & DELETED_ENDPOINT) == 0)
+			gSocketModule->release_socket(endpoint->socket);
 	} else if ((segment.flags & TCP_FLAG_RESET) == 0)
 		segmentAction = DROP | RESET;
 

src/add-ons/kernel/network/protocols/tcp/tcp.h

@@ -267,12 +267,12 @@ struct tcp_segment_header {
 };
 
 enum tcp_segment_action {
-	KEEP			= 0x00,
+	KEEP					= 0x00,
-	DROP			= 0x01,
+	DROP					= 0x01,
-	RESET			= 0x02,
+	RESET					= 0x02,
-	ACKNOWLEDGE		= 0x04,
+	ACKNOWLEDGE				= 0x04,
-	IMMEDIATE_ACKNOWLEDGE = 0x08,
+	IMMEDIATE_ACKNOWLEDGE	= 0x08,
-	DELETE_ENDPOINT	= 0x10,
+	DELETED_ENDPOINT		= 0x10,
 };