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 <[email protected]>

Author: jukkar

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;