Commit 2c750703 authored by Robert Lubos's avatar Robert Lubos Committed by Anas Nashif
Browse files

net: sockets: tcp: Fix possible race between connect/recv 

Installing recv callback with net_context_recv() after
net_context_connect() left an opening for a possible race - in case the
server send some data immediately after establishing TCP connection, and
Zephyr did not manage to install the callback on time, the data would be
lost, corrupting the stream.

This can be avoided, by installing the recv callback before the
connection is triggered. As net_context_recv() called w/o timeout only
registers the callback function, it should have no negative impact. The
only change on the TCP side is when the connection is closed - in case
TCP is in connect stage, do not call the recv callback (before this
change it'd be NULL at that point).

Signed-off-by: default avatarRobert Lubos <robert.lubos@nordicsemi.no>
parent 5f68ea68

subsys/net/ip/tcp.c

+1 −3
Changes for subsys/net/ip/tcp.c: 1 added line, 3 removed lines.
Original line number Diff line number Diff line
@@ -497,9 +497,7 @@ static int tcp_conn_close(struct tcp *conn, int status) 
			/* Make sure the connect_cb is only called once. */

			conn->connect_cb = NULL;

		}

	}



	if (conn->context->recv_cb) {

	} else if (conn->context->recv_cb) {

		conn->context->recv_cb(conn->context, NULL, NULL, NULL,

				       status, conn->recv_user_data);

	}

subsys/net/lib/sockets/sockets.c

+10 −11
Changes for subsys/net/lib/sockets/sockets.c: 10 added lines, 11 removed lines.
Original line number Diff line number Diff line
@@ -490,15 +490,12 @@ static void zsock_connected_cb(struct net_context *ctx, int status, void *user_d 
	if (status < 0) {

		ctx->user_data = INT_TO_POINTER(-status);

		sock_set_error(ctx);

	} else if (status == 0) {

		(void)net_context_recv(ctx, zsock_received_cb, K_NO_WAIT, ctx->user_data);

	}

}



int zsock_connect_ctx(struct net_context *ctx, const struct sockaddr *addr,

		      socklen_t addrlen)

{

	k_timeout_t timeout;



#if defined(CONFIG_SOCKS)

	if (net_context_is_proxy_enabled(ctx)) {
@@ -516,17 +513,19 @@ int zsock_connect_ctx(struct net_context *ctx, const struct sockaddr *addr, 
		} else {

			SET_ERRNO(-EALREADY);

		}

	} else if (sock_is_nonblock(ctx)) {

		timeout = K_NO_WAIT;

		SET_ERRNO(net_context_connect(ctx, addr, addrlen,

					      zsock_connected_cb, timeout,

					      ctx->user_data));

	} else {

		timeout = K_MSEC(CONFIG_NET_SOCKETS_CONNECT_TIMEOUT);

		SET_ERRNO(net_context_connect(ctx, addr, addrlen, NULL,

					      timeout, NULL));

		k_timeout_t timeout = K_MSEC(CONFIG_NET_SOCKETS_CONNECT_TIMEOUT);

		net_context_connect_cb_t cb = NULL;



		if (sock_is_nonblock(ctx)) {

			timeout = K_NO_WAIT;

			cb = zsock_connected_cb;

		}



		SET_ERRNO(net_context_recv(ctx, zsock_received_cb, K_NO_WAIT,

					   ctx->user_data));

		SET_ERRNO(net_context_connect(ctx, addr, addrlen, cb, timeout,

					      ctx->user_data));

	}



	return 0;

CRA Git | Maintained and supported by SUSTech CRA and CCSE