Skip to content

Commit ba6387f

Browse files
rluboscfriedt
authored andcommitted
net: tcp: Close the connection unconditionally on forced close
The current approach was buggy, for example the TCP context could be unrefed twice in case of forced close. Or in case of a race, when the application closed the socket first, the TCP context wouldn't be dereferenced at all. Calling the tcp_conn_close() unconditionally in case of forced close solves all those issues. Signed-off-by: Robert Lubos <[email protected]>
1 parent 7fe33a5 commit ba6387f

File tree

1 file changed

+26
-32
lines changed

1 file changed

+26
-32
lines changed

subsys/net/ip/tcp.c

Lines changed: 26 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -3705,6 +3705,17 @@ int net_tcp_put(struct net_context *context, bool force_close)
37053705
({ const char *state = net_context_state(context);
37063706
state ? state : "<unknown>"; }));
37073707

3708+
if (force_close) {
3709+
k_work_cancel_delayable(&conn->send_data_timer);
3710+
keep_alive_timer_stop(conn);
3711+
3712+
k_mutex_unlock(&conn->lock);
3713+
3714+
tcp_conn_close(conn, -ENETRESET);
3715+
3716+
return 0;
3717+
}
3718+
37083719
if (conn->state == TCP_ESTABLISHED ||
37093720
conn->state == TCP_SYN_RECEIVED) {
37103721
/* Send all remaining data if possible. */
@@ -3713,43 +3724,26 @@ int net_tcp_put(struct net_context *context, bool force_close)
37133724
conn->send_data_total);
37143725
conn->in_close = true;
37153726

3716-
if (force_close) {
3717-
k_work_cancel_delayable(&conn->send_data_timer);
3718-
3719-
keep_alive_timer_stop(conn);
3720-
tcp_conn_close(conn, -ENETRESET);
3721-
} else {
3722-
/* How long to wait until all the data has been sent?
3723-
*/
3724-
k_work_reschedule_for_queue(&tcp_work_q,
3725-
&conn->send_data_timer,
3726-
K_MSEC(TCP_RTO_MS));
3727-
}
3727+
/* How long to wait until all the data has been sent? */
3728+
k_work_reschedule_for_queue(&tcp_work_q,
3729+
&conn->send_data_timer,
3730+
K_MSEC(TCP_RTO_MS));
37283731

37293732
} else {
3730-
if (force_close) {
3731-
NET_DBG("[%p] TCP connection in %s close, "
3732-
"disposing immediately",
3733-
conn, "forced");
3734-
3735-
keep_alive_timer_stop(conn);
3736-
tcp_conn_close(conn, -ENETRESET);
3737-
} else {
3738-
NET_DBG("[%p] TCP connection in %s close, "
3739-
"not disposing yet (waiting %dms)",
3740-
conn, "active", tcp_max_timeout_ms);
3741-
k_work_reschedule_for_queue(&tcp_work_q,
3742-
&conn->fin_timer,
3743-
FIN_TIMEOUT);
3733+
NET_ERR("[%p] TCP connection in %s close, "
3734+
"not disposing yet (waiting %dms)",
3735+
conn, "active", tcp_max_timeout_ms);
3736+
k_work_reschedule_for_queue(&tcp_work_q,
3737+
&conn->fin_timer,
3738+
FIN_TIMEOUT);
37443739

3745-
tcp_out(conn, FIN | ACK);
3746-
conn_seq(conn, + 1);
3747-
tcp_setup_retransmission(conn);
3740+
tcp_out(conn, FIN | ACK);
3741+
conn_seq(conn, + 1);
3742+
tcp_setup_retransmission(conn);
37483743

3749-
conn_state(conn, TCP_FIN_WAIT_1);
3744+
conn_state(conn, TCP_FIN_WAIT_1);
37503745

3751-
keep_alive_timer_stop(conn);
3752-
}
3746+
keep_alive_timer_stop(conn);
37533747
}
37543748
} else if (conn->in_connect) {
37553749
conn->in_connect = false;

0 commit comments

Comments
 (0)