net: tcp: Fix possible double TCP context dereferencing

In case TCP connection is being closed from the TCP stack, due to for
instance retransmission timeout, the stack should also switch the TCP
state to CLOSED. Otherwise, there was a risk of dereferencing the TCP
context twice, for example if the application was in active socket
send(), and tried to reschedule data transmission.

Additionally, make sure that the TCP_CLOSED state handling is a no-op
state - otherwise, there is a risk that if packets keep incoming before
the application dereferences the TCP context on its side, TCP stack
will incorrectly dereference the context for the second time from
within due to current TCP_CLOSED state logic.

Signed-off-by: Robert Lubos <[email protected]>

subsys/net/ip/tcp.c

@@ -75,6 +75,7 @@ static K_KERNEL_STACK_DEFINE(work_q_stack, CONFIG_NET_TCP_WORKQ_STACK_SIZE);
 static enum net_verdict tcp_in(struct tcp *conn, struct net_pkt *pkt);
 static bool is_destination_local(struct net_pkt *pkt);
 static void tcp_out(struct tcp *conn, uint8_t flags);
+static const char *tcp_state_to_str(enum tcp_state state, bool prefix);
 
 int (*tcp_send_cb)(struct net_pkt *pkt) = NULL;
 size_t (*tcp_recv_cb)(struct tcp *conn, struct net_pkt *pkt) = NULL;
@@ -489,6 +490,9 @@ static int tcp_conn_close(struct tcp *conn, int status)
 #if CONFIG_NET_TCP_LOG_LEVEL >= LOG_LEVEL_DBG
 	NET_DBG("conn: %p closed by TCP stack (%s():%d)", conn, caller, line);
 #endif
+	k_mutex_lock(&conn->lock, K_FOREVER);
+	conn_state(conn, TCP_CLOSED);
+	k_mutex_unlock(&conn->lock);
 
 	if (conn->in_connect) {
 		if (conn->connect_cb) {
@@ -2097,7 +2101,7 @@ static enum net_verdict tcp_in(struct tcp *conn, struct net_pkt *pkt)
 
 	if (th && th_off(th) < 5) {
 		tcp_out(conn, RST);
-		conn_state(conn, TCP_CLOSED);
+		do_close = true;
 		close_status = -ECONNRESET;
 		goto next_state;
 	}
@@ -2113,7 +2117,7 @@ static enum net_verdict tcp_in(struct tcp *conn, struct net_pkt *pkt)
 		/* Valid RST received. */
 		verdict = NET_OK;
 		net_stats_update_tcp_seg_rst(net_pkt_iface(pkt));
-		conn_state(conn, TCP_CLOSED);
+		do_close = true;
 		close_status = -ECONNRESET;
 		goto next_state;
 	}
@@ -2122,7 +2126,7 @@ static enum net_verdict tcp_in(struct tcp *conn, struct net_pkt *pkt)
 						  tcp_options_len)) {
 		NET_DBG("DROP: Invalid TCP option list");
 		tcp_out(conn, RST);
-		conn_state(conn, TCP_CLOSED);
+		do_close = true;
 		close_status = -ECONNRESET;
 		goto next_state;
 	}
@@ -2137,7 +2141,7 @@ static enum net_verdict tcp_in(struct tcp *conn, struct net_pkt *pkt)
 			conn, tcp_state_to_str(conn->state, false));
 		net_stats_update_tcp_seg_drop(conn->iface);
 		tcp_out(conn, RST);
-		conn_state(conn, TCP_CLOSED);
+		do_close = true;
 		close_status = -ECONNRESET;
 		goto next_state;
 	}
@@ -2353,7 +2357,7 @@ static enum net_verdict tcp_in(struct tcp *conn, struct net_pkt *pkt)
 					conn->send_data_total);
 				net_stats_update_tcp_seg_drop(conn->iface);
 				tcp_out(conn, RST);
-				conn_state(conn, TCP_CLOSED);
+				do_close = true;
 				close_status = -ECONNRESET;
 				break;
 			}
@@ -2407,7 +2411,7 @@ static enum net_verdict tcp_in(struct tcp *conn, struct net_pkt *pkt)
 			ret = tcp_send_queued_data(conn);
 			if (ret < 0 && ret != -ENOBUFS) {
 				tcp_out(conn, RST);
-				conn_state(conn, TCP_CLOSED);
+				do_close = true;
 				close_status = ret;
 				verdict = NET_OK;
 				break;
@@ -2461,13 +2465,12 @@ static enum net_verdict tcp_in(struct tcp *conn, struct net_pkt *pkt)
 	case TCP_LAST_ACK:
 		if (th && FL(&fl, ==, ACK, th_seq(th) == conn->ack)) {
 			tcp_send_timer_cancel(conn);
-			next = TCP_CLOSED;
+			do_close = true;
 			verdict = NET_OK;
 			close_status = 0;
 		}
 		break;
 	case TCP_CLOSED:
-		do_close = true;
 		break;
 	case TCP_FIN_WAIT_1:
 		if (th) {