Skip to content

Commit 79461a3

Browse files
rlubosfabiobaltieri
rlubos
authored and
fabiobaltieri
committed
net: tcp: Fix TCP connection ref counting
The net_context/TCP connection context ref counting needed to be fixed, to avoid situation where TCP connection context could be released before net_context is released. Generally, this commit modifies the ref counting of the above as follows: * net_context is referenced both by the application and TCP stack, when created. * TCP context is referenced by the application when created. The TCP stack adds ref to the TCP context only after connection has been established. * TCP stack needs to call the final upper layer callback when connection is closed, but before the TCP context is released (i. e. to give upper layer chance to dereference the context on its side). For this, tcp_conn_close() was introduced which is called from within TCP stack instead of directly dereferencing the TCP context. * TCP stack dereferences the net_context only after the TCP context has been released (i. e. upper layer has dereferenced TCP context on it's side and connection has been tear down, if established). Signed-off-by: Robert Lubos <[email protected]>
1 parent c79a25b commit 79461a3

File tree

2 files changed

+42
-42
lines changed

2 files changed

+42
-42
lines changed

‎subsys/net/ip/net_context.c

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -359,8 +359,6 @@ int net_context_unref(struct net_context *context)
359359

360360
k_mutex_lock(&context->lock, K_FOREVER);
361361

362-
net_tcp_unref(context);
363-
364362
if (context->conn_handler) {
365363
if (IS_ENABLED(CONFIG_NET_TCP) || IS_ENABLED(CONFIG_NET_UDP) ||
366364
IS_ENABLED(CONFIG_NET_SOCKETS_CAN) ||

‎subsys/net/ip/tcp.c

Lines changed: 42 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -393,23 +393,13 @@ static void tcp_send_queue_flush(struct tcp *conn)
393393
}
394394
}
395395

396-
#if CONFIG_NET_TCP_LOG_LEVEL >= LOG_LEVEL_DBG
397-
#define tcp_conn_unref(conn, status) \
398-
tcp_conn_unref_debug(conn, status, __func__, __LINE__)
399396

400-
static int tcp_conn_unref_debug(struct tcp *conn, int status,
401-
const char *caller, int line)
402-
#else
403-
static int tcp_conn_unref(struct tcp *conn, int status)
404-
#endif
397+
static int tcp_conn_unref(struct tcp *conn)
405398
{
406399
int ref_count = atomic_get(&conn->ref_count);
407400
struct net_pkt *pkt;
408401

409-
#if CONFIG_NET_TCP_LOG_LEVEL >= LOG_LEVEL_DBG
410-
NET_DBG("conn: %p, ref_count=%d (%s():%d)", conn, ref_count,
411-
caller, line);
412-
#endif
402+
NET_DBG("conn: %p, ref_count=%d", conn, ref_count);
413403

414404
#if !defined(CONFIG_NET_TEST_PROTOCOL)
415405
if (conn->in_connect) {
@@ -444,11 +434,6 @@ static int tcp_conn_unref(struct tcp *conn, int status)
444434
conn->context->conn_handler = NULL;
445435
}
446436

447-
if (conn->context->recv_cb) {
448-
conn->context->recv_cb(conn->context, NULL, NULL, NULL,
449-
status, conn->recv_user_data);
450-
}
451-
452437
conn->context->tcp = NULL;
453438

454439
net_context_unref(conn->context);
@@ -485,12 +470,34 @@ int net_tcp_unref(struct net_context *context)
485470
NET_DBG("context: %p, conn: %p", context, context->tcp);
486471

487472
if (context->tcp) {
488-
ref_count = tcp_conn_unref(context->tcp, 0);
473+
ref_count = tcp_conn_unref(context->tcp);
489474
}
490475

491476
return ref_count;
492477
}
493478

479+
#if CONFIG_NET_TCP_LOG_LEVEL >= LOG_LEVEL_DBG
480+
#define tcp_conn_close(conn, status) \
481+
tcp_conn_close_debug(conn, status, __func__, __LINE__)
482+
483+
static int tcp_conn_close_debug(struct tcp *conn, int status,
484+
const char *caller, int line)
485+
#else
486+
static int tcp_conn_close(struct tcp *conn, int status)
487+
#endif
488+
{
489+
#if CONFIG_NET_TCP_LOG_LEVEL >= LOG_LEVEL_DBG
490+
NET_DBG("conn: %p closed by TCP stack (%s():%d)", conn, caller, line);
491+
#endif
492+
493+
if (conn->context->recv_cb) {
494+
conn->context->recv_cb(conn->context, NULL, NULL, NULL,
495+
status, conn->recv_user_data);
496+
}
497+
498+
return tcp_conn_unref(conn);
499+
}
500+
494501
static bool tcp_send_process_no_lock(struct tcp *conn)
495502
{
496503
bool unref = false;
@@ -569,7 +576,7 @@ static void tcp_send_process(struct k_work *work)
569576
k_mutex_unlock(&conn->lock);
570577

571578
if (unref) {
572-
tcp_conn_unref(conn, -ETIMEDOUT);
579+
tcp_conn_close(conn, -ETIMEDOUT);
573580
}
574581
}
575582

@@ -1074,7 +1081,7 @@ static int tcp_out_ext(struct tcp *conn, uint8_t flags, struct net_pkt *data,
10741081
k_work_schedule_for_queue(&tcp_work_q,
10751082
&conn->send_timer, K_NO_WAIT);
10761083
} else if (tcp_send_process_no_lock(conn)) {
1077-
tcp_conn_unref(conn, -ETIMEDOUT);
1084+
tcp_conn_close(conn, -ETIMEDOUT);
10781085
}
10791086
out:
10801087
return ret;
@@ -1334,7 +1341,7 @@ static void tcp_resend_data(struct k_work *work)
13341341
k_mutex_unlock(&conn->lock);
13351342

13361343
if (conn_unref) {
1337-
tcp_conn_unref(conn, -ETIMEDOUT);
1344+
tcp_conn_close(conn, -ETIMEDOUT);
13381345
}
13391346
}
13401347

@@ -1345,16 +1352,15 @@ static void tcp_timewait_timeout(struct k_work *work)
13451352

13461353
NET_DBG("conn: %p %s", conn, tcp_conn_state(conn, NULL));
13471354

1348-
/* Extra unref from net_tcp_put() */
1349-
net_context_unref(conn->context);
1355+
(void)tcp_conn_close(conn, -ETIMEDOUT);
13501356
}
13511357

13521358
static void tcp_establish_timeout(struct tcp *conn)
13531359
{
13541360
NET_DBG("Did not receive %s in %dms", "ACK", ACK_TIMEOUT_MS);
13551361
NET_DBG("conn: %p %s", conn, tcp_conn_state(conn, NULL));
13561362

1357-
(void)tcp_conn_unref(conn, -ETIMEDOUT);
1363+
(void)tcp_conn_close(conn, -ETIMEDOUT);
13581364
}
13591365

13601366
static void tcp_fin_timeout(struct k_work *work)
@@ -1370,8 +1376,7 @@ static void tcp_fin_timeout(struct k_work *work)
13701376
NET_DBG("Did not receive %s in %dms", "FIN", tcp_fin_timeout_ms);
13711377
NET_DBG("conn: %p %s", conn, tcp_conn_state(conn, NULL));
13721378

1373-
/* Extra unref from net_tcp_put() */
1374-
net_context_unref(conn->context);
1379+
(void)tcp_conn_close(conn, -ETIMEDOUT);
13751380
}
13761381

13771382
static void tcp_send_zwp(struct k_work *work)
@@ -1731,13 +1736,13 @@ static struct tcp *tcp_conn_new(struct net_pkt *pkt)
17311736
net_context_set_family(conn->context, net_pkt_family(pkt));
17321737

17331738
if (tcp_endpoint_set(&conn->dst, pkt, TCP_EP_SRC) < 0) {
1734-
net_context_unref(context);
1739+
net_context_put(context);
17351740
conn = NULL;
17361741
goto err;
17371742
}
17381743

17391744
if (tcp_endpoint_set(&conn->src, pkt, TCP_EP_DST) < 0) {
1740-
net_context_unref(context);
1745+
net_context_put(context);
17411746
conn = NULL;
17421747
goto err;
17431748
}
@@ -1772,7 +1777,7 @@ static struct tcp *tcp_conn_new(struct net_pkt *pkt)
17721777
ret = net_context_bind(context, &local_addr, sizeof(local_addr));
17731778
if (ret < 0) {
17741779
NET_DBG("Cannot bind accepted context, connection reset");
1775-
net_context_unref(context);
1780+
net_context_put(context);
17761781
conn = NULL;
17771782
goto err;
17781783
}
@@ -1796,7 +1801,7 @@ static struct tcp *tcp_conn_new(struct net_pkt *pkt)
17961801
&context->conn_handler);
17971802
if (ret < 0) {
17981803
NET_ERR("net_conn_register(): %d", ret);
1799-
net_context_unref(context);
1804+
net_context_put(context);
18001805
conn = NULL;
18011806
goto err;
18021807
}
@@ -2188,6 +2193,7 @@ static enum net_verdict tcp_in(struct tcp *conn, struct net_pkt *pkt)
21882193
k_work_cancel_delayable(&conn->establish_timer);
21892194
tcp_send_timer_cancel(conn);
21902195
next = TCP_ESTABLISHED;
2196+
tcp_conn_ref(conn);
21912197
net_context_set_state(conn->context,
21922198
NET_CONTEXT_CONNECTED);
21932199

@@ -2228,6 +2234,7 @@ static enum net_verdict tcp_in(struct tcp *conn, struct net_pkt *pkt)
22282234
}
22292235

22302236
next = TCP_ESTABLISHED;
2237+
tcp_conn_ref(conn);
22312238
net_context_set_state(conn->context,
22322239
NET_CONTEXT_CONNECTED);
22332240
tcp_out(conn, ACK);
@@ -2518,7 +2525,7 @@ static enum net_verdict tcp_in(struct tcp *conn, struct net_pkt *pkt)
25182525
* to a deadlock.
25192526
*/
25202527
if (do_close) {
2521-
tcp_conn_unref(conn, close_status);
2528+
tcp_conn_close(conn, close_status);
25222529
}
25232530

25242531
return verdict;
@@ -2569,16 +2576,11 @@ int net_tcp_put(struct net_context *context)
25692576

25702577
conn_state(conn, TCP_FIN_WAIT_1);
25712578
}
2572-
2573-
/* Make sure we do not delete the connection yet until we have
2574-
* sent the final ACK.
2575-
*/
2576-
net_context_ref(context);
25772579
}
25782580

25792581
k_mutex_unlock(&conn->lock);
25802582

2581-
net_context_unref(context);
2583+
net_tcp_unref(context);
25822584

25832585
return 0;
25842586
}
@@ -2677,7 +2679,7 @@ int net_tcp_queue_data(struct net_context *context, struct net_pkt *pkt)
26772679

26782680
ret = tcp_send_queued_data(conn);
26792681
if (ret < 0 && ret != -ENOBUFS) {
2680-
tcp_conn_unref(conn, ret);
2682+
tcp_conn_close(conn, ret);
26812683
goto out;
26822684
}
26832685

@@ -2861,7 +2863,7 @@ int net_tcp_connect(struct net_context *context,
28612863
if (k_sem_take(&conn->connect_sem, timeout) != 0 &&
28622864
conn->state != TCP_ESTABLISHED) {
28632865
conn->in_connect = false;
2864-
tcp_conn_unref(conn, -ETIMEDOUT);
2866+
tcp_conn_close(conn, -ETIMEDOUT);
28652867
ret = -ETIMEDOUT;
28662868
goto out;
28672869
}
@@ -3169,7 +3171,7 @@ enum net_verdict tp_input(struct net_conn *net_conn,
31693171

31703172
conn = (void *)sys_slist_peek_head(&tcp_conns);
31713173
context = conn->context;
3172-
while (tcp_conn_unref(conn, 0))
3174+
while (tcp_conn_close(conn, 0))
31733175
;
31743176
tcp_free(context);
31753177
}

0 commit comments

Comments
 (0)