From a41f625faba8cca16865876f19a48bfccd0ca006 Mon Sep 17 00:00:00 2001 From: Jukka Rissanen Date: Tue, 24 Sep 2024 11:15:46 +0300 Subject: [PATCH] net: tcp: Make sure that connection failure is propagated to app It is possible that TCP connect() will fail if for example network interface does not have IP address set. In this case we close the connection during net_tcp_connect() but do not set the return code properly. This looks in the application like the connection succeeded even if it was not. As the tcp_in() call in net_tcp_connect() might close the connection, we just take extra ref count while calling tcp_in(). Otherwise we might access already freed connection. Before the fix: net_tcp_connect: context: 0x80757c0, local: 0.0.0.0, remote: 192.0.2.2 net_tcp_connect: conn: 0x8087320 src: 0.0.0.0, dst: 192.0.2.2 tcp_in: [LISTEN Seq=1604170158 Ack=0] tcp_conn_close_debug: conn: 0x8087320 closed by TCP stack (tcp_in():3626) tcp_conn_close_debug: LISTEN->CLOSED tcp_conn_unref: conn: 0x8087320, ref_count=1 net_tcp_connect: conn: 0x8087320, ret=0 After the fix: net_tcp_connect: context: 0x80757c0, local: 0.0.0.0, remote: 192.0.2.2 net_tcp_connect: conn: 0x8087320 src: 0.0.0.0, dst: 192.0.2.2 tcp_conn_ref: conn: 0x8087320, ref_count: 2 tcp_in: [LISTEN Seq=1604170158 Ack=0] tcp_conn_close_debug: conn: 0x8087320 closed by TCP stack (tcp_in():3626) tcp_conn_close_debug: LISTEN->CLOSED tcp_conn_unref: conn: 0x8087320, ref_count=2 net_tcp: tcp_conn_unref: conn: 0x8087320, ref_count=1 net_tcp: net_tcp_connect: conn: 0x8087320, ret=-128 Signed-off-by: Jukka Rissanen --- subsys/net/ip/tcp.c | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/subsys/net/ip/tcp.c b/subsys/net/ip/tcp.c index b006f901846..2f7c06bffe1 100644 --- a/subsys/net/ip/tcp.c +++ b/subsys/net/ip/tcp.c @@ -3948,16 +3948,21 @@ int net_tcp_connect(struct net_context *context, * a TCP connection to be established */ conn->in_connect = !IS_ENABLED(CONFIG_NET_TEST_PROTOCOL); + + /* The ref will make sure that if the connection is closed in tcp_in(), + * we do not access already freed connection. + */ + tcp_conn_ref(conn); (void)tcp_in(conn, NULL); if (!IS_ENABLED(CONFIG_NET_TEST_PROTOCOL)) { if (conn->state == TCP_UNUSED || conn->state == TCP_CLOSED) { - ret = -errno; - goto out; + ret = -ENOTCONN; + goto out_unref; } else if ((K_TIMEOUT_EQ(timeout, K_NO_WAIT)) && conn->state != TCP_ESTABLISHED) { ret = -EINPROGRESS; - goto out; + goto out_unref; } else if (k_sem_take(&conn->connect_sem, timeout) != 0 && conn->state != TCP_ESTABLISHED) { if (conn->in_connect) { @@ -3966,11 +3971,15 @@ int net_tcp_connect(struct net_context *context, } ret = -ETIMEDOUT; - goto out; + goto out_unref; } conn->in_connect = false; } - out: + +out_unref: + tcp_conn_unref(conn); + +out: NET_DBG("conn: %p, ret=%d", conn, ret); return ret;