net: tcp: Handle special case where accepted socket is closed

Handle this corner case with TCP connection closing:

1) Client A connects, it is accepted and can send data to us
2) Client B connects, the application needs to call accept()
   before we will receive any data from client A to the application.
   The app has not yet called accept() at this point (for
   whatever reason).
3) Client B then disconnects and we receive FIN. The connection
   cleanup is a bit tricky as the client is in half-connected state
   meaning that the connection is in established state but the
   accept_q in socket queue contains still data which needs to be
   cleared.
4) Client A then disconnects, all data is sent etc

The above was not working correctly as the system did not handle the
step 3) properly. The client B was accepted in the application even
if the connection was closing.

After this commit, the commit called "net: tcp: Accept connections
only in LISTENING state" and related other commits are no longer
needed and are reverted.

Signed-off-by: Jukka Rissanen <jukka.rissanen@linux.intel.com>

include/net/net_context.h

@@ -58,6 +58,12 @@ enum net_context_state {
 /** Remote address set */
 #define NET_CONTEXT_REMOTE_ADDR_SET  BIT(8)
 
+/** Is the socket accepting connections */
+#define NET_CONTEXT_ACCEPTING_SOCK  BIT(9)
+
+/** Is the socket closing / closed */
+#define NET_CONTEXT_CLOSING_SOCK  BIT(10)
+
 struct net_context;
 
 /**
@@ -334,6 +340,70 @@ static inline bool net_context_is_used(struct net_context *context)
 	return context->flags & NET_CONTEXT_IN_USE;
 }
 
+/**
+ * @brief Is this context is accepting data now.
+ *
+ * @param context Network context.
+ *
+ * @return True if the context is accepting connections, False otherwise.
+ */
+static inline bool net_context_is_accepting(struct net_context *context)
+{
+	NET_ASSERT(context);
+
+	return context->flags & NET_CONTEXT_ACCEPTING_SOCK;
+}
+
+/**
+ * @brief Set this context to accept data now.
+ *
+ * @param context Network context.
+ * @param accepting True if accepting, False if not
+ */
+static inline void net_context_set_accepting(struct net_context *context,
+					     bool accepting)
+{
+	NET_ASSERT(context);
+
+	if (accepting) {
+		context->flags |= NET_CONTEXT_ACCEPTING_SOCK;
+	} else {
+		context->flags &= ~NET_CONTEXT_ACCEPTING_SOCK;
+	}
+}
+
+/**
+ * @brief Is this context closing.
+ *
+ * @param context Network context.
+ *
+ * @return True if the context is closing, False otherwise.
+ */
+static inline bool net_context_is_closing(struct net_context *context)
+{
+	NET_ASSERT(context);
+
+	return context->flags & NET_CONTEXT_CLOSING_SOCK;
+}
+
+/**
+ * @brief Set this context to closing.
+ *
+ * @param context Network context.
+ * @param closing True if closing, False if not
+ */
+static inline void net_context_set_closing(struct net_context *context,
+					   bool closing)
+{
+	NET_ASSERT(context);
+
+	if (closing) {
+		context->flags |= NET_CONTEXT_CLOSING_SOCK;
+	} else {
+		context->flags &= ~NET_CONTEXT_CLOSING_SOCK;
+	}
+}
+
 #define NET_CONTEXT_STATE_SHIFT 1
 #define NET_CONTEXT_STATE_MASK 0x03
 

subsys/net/ip/tcp.c

@@ -1943,6 +1943,7 @@ NET_CONN_CB(tcp_established)
 	struct net_context *context = (struct net_context *)user_data;
 	struct net_tcp_hdr *tcp_hdr = proto_hdr->tcp;
 	enum net_verdict ret = NET_OK;
+	bool do_not_send_ack = false;
 	u8_t tcp_flags;
 	u16_t data_len;
 
@@ -2051,6 +2052,8 @@ resend_ack:
 			 */
 			k_delayed_work_submit(&context->tcp->ack_timer,
 					      ACK_TIMEOUT);
+
+			net_context_set_closing(context, true);
 		} else if (net_tcp_get_state(context->tcp)
 			   == NET_TCP_FIN_WAIT_2) {
 			/* Received FIN on FIN_WAIT_2, so cancel the timer */
@@ -2062,7 +2065,14 @@ resend_ack:
 		context->tcp->fin_rcvd = 1U;
 	}
 
-	data_len = net_pkt_remaining_data(pkt);
+	if (!IS_ENABLED(CONFIG_NET_TCP_AUTO_ACCEPT) &&
+	    net_context_is_accepting(context)) {
+		data_len = 0;
+		do_not_send_ack = true;
+	} else {
+		data_len = net_pkt_remaining_data(pkt);
+	}
+
 	if (data_len > net_tcp_get_recv_wnd(context->tcp)) {
 		/* In case we have zero window, we should still accept
 		 * Zero Window Probes from peer, which per convention
@@ -2095,13 +2105,15 @@ resend_ack:
 		net_pkt_unref(pkt);
 	}
 
-	/* Increment the ack */
+	if (do_not_send_ack == false) {
-	context->tcp->send_ack += data_len;
+		/* Increment the ack */
-	if (tcp_flags & NET_TCP_FIN) {
+		context->tcp->send_ack += data_len;
-		context->tcp->send_ack += 1U;
+		if (tcp_flags & NET_TCP_FIN) {
-	}
+			context->tcp->send_ack += 1U;
+		}
 
-	send_ack(context, &conn->remote_addr, false);
+		send_ack(context, &conn->remote_addr, false);
+	}
 
 clean_up:
 	if (net_tcp_get_state(context->tcp) == NET_TCP_TIME_WAIT) {
@@ -2452,6 +2464,12 @@ NET_CONN_CB(tcp_syn_rcvd)
 		 */
 		new_context->tcp->state = NET_TCP_ESTABLISHED;
 
+		/* Mark the new context to be still accepting so that we
+		 * can do proper cleanup if connection is closed before
+		 * we have called accept()
+		 */
+		net_context_set_accepting(new_context, true);
+
 		net_context_set_state(new_context, NET_CONTEXT_CONNECTED);
 
 		if (new_context->remote.sa_family == AF_INET) {

subsys/net/lib/sockets/sockets.c

@@ -406,6 +406,7 @@ int zsock_accept_ctx(struct net_context *parent, struct sockaddr *addr,
 {
 	s32_t timeout = K_FOREVER;
 	struct net_context *ctx;
+	struct net_pkt *last_pkt;
 	int fd;
 
 	fd = z_reserve_fd();
@@ -423,6 +424,23 @@ int zsock_accept_ctx(struct net_context *parent, struct sockaddr *addr,
 		return -1;
 	}
 
+	/* Check if the connection is already disconnected */
+	last_pkt = k_fifo_peek_tail(&ctx->recv_q);
+	if (last_pkt) {
+		if (net_pkt_eof(last_pkt)) {
+			sock_set_eof(ctx);
+			errno = ECONNABORTED;
+			return -1;
+		}
+	}
+
+	if (net_context_is_closing(ctx)) {
+		errno = ECONNABORTED;
+		return -1;
+	}
+
+	net_context_set_accepting(ctx, false);
+
 #ifdef CONFIG_USERSPACE
 	z_object_recycle(ctx);
 #endif