From 9cd547f53b7474945e94111e5f8ce9b05fa9579c Mon Sep 17 00:00:00 2001 From: Jukka Rissanen Date: Thu, 21 Mar 2019 12:44:17 +0200 Subject: [PATCH] net: tcp: Fix ref counting for the net_pkt The network packet ref count was not properly increased when the TCP was retried. This meant that the second time the packet was sent, the device driver managed to release the TCP frame even if we had not got ACK to it. Somewhat long debug log follows: The net_pkt 0x08072d5c is created, we write 1K data into it, initial ref count is 1. net_pkt_write: pkt 0x08072d5c data 0x08075d40 length 1024 net_tcp_queue_data: Queue 0x08072d5c len 1024 net_tcp_trace: pkt 0x08072d5c src 5001 dst 5001 net_tcp_trace: seq 0x15d2aa09 (366127625) ack 0x7f67d918 net_tcp_trace: flags uAPrsf net_tcp_trace: win 1280 chk 0x0bea net_tcp_queue_pkt: pkt 0x08072d5c new ref 2 (net_tcp_queue_pkt:850) At this point, the ref is 2. Then the packet is sent as you see below. net_pkt_ref_debug: TX [13] pkt 0x08072d5c ref 2 net_tcp_queue_pkt():850 net_tcp_send_data: Sending pkt 0x08072d5c (1084 bytes) net_pkt_unref_debug: TX [13] pkt 0x08072d5c ref 1 (ethernet_send():597) Ref is still correct, packet is still alive. We have not received ACK, so the packet is resent. tcp_retry_expired: ref pkt 0x08072d5c new ref 2 (tcp_retry_expired:233) net_pkt_ref_debug: TX [10] pkt 0x08072d5c ref 2 tcp_retry_expired():233 net_pkt_unref_debug: TX [10] pkt 0x08072d5c ref 1 ... (net_if_tx():173) net_pkt_unref_debug: TX [10] pkt 0x08072d5c ref 0 ... (net_if_tx():173) Reference count is now wrong, it should have been 1. This is because we did not increase the ref count when packet was placed first time into sent list in tcp.c:tcp_retry_expired(). The fix is quite simple as you can see from this commit. Signed-off-by: Jukka Rissanen --- subsys/net/ip/tcp.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/subsys/net/ip/tcp.c b/subsys/net/ip/tcp.c index ca441d4a7c09..49b44dd4c79c 100644 --- a/subsys/net/ip/tcp.c +++ b/subsys/net/ip/tcp.c @@ -229,10 +229,17 @@ static void tcp_retry_expired(struct k_work *work) pkt = CONTAINER_OF(sys_slist_peek_head(&tcp->sent_list), struct net_pkt, sent_list); - if (net_pkt_sent(pkt)) { - do_ref_if_needed(tcp, pkt); - net_pkt_set_sent(pkt, false); - } + /* In the retry case, the original ref (when the packet + * was created) is set to 1. That original ref was + * decremented when the packet was sent by the driver. + * We need to restore that original ref so that the + * device driver will not remove the retry packet that + * we just sent. Earlier we also checked net_pkt_sent(pkt) + * here but that is not correct as then the packet that was + * sent first time, was removed by the driver and we got + * access to memory already freed. + */ + do_ref_if_needed(tcp, pkt); net_pkt_set_queued(pkt, true);