From 33d309715dfb0780e033b31f6480ce9f6111e94a Mon Sep 17 00:00:00 2001
From: David Cermak <cermak@espressif.com>
Date: Tue, 29 Dec 2020 21:35:25 +0100
Subject: [PATCH] tcp: Use TF_REFUSED_DATA flag to indicate pcb refused recv
 event

If posting of any received event failed (TCP_EVENT_CLOSED or TCP_EVENT_RECV),
we mark the pcb with TF_REFUSED_DATA flag, so that we could retry next
time. Previously, only the member `refused_data` stored the received pbuf if TCP_EVENT_RECV
failed. Now we differentiate these tree potential event refusals:
* TCP_EVENT_RECV: (pcb->flags & TF_REFUSED_DATA) && pcb->refused_data ==
  recv_pbuf: Post the refused-data next time
* both TCP_EVENT_CLOSED and TCP_EVENT_RECV: (pcb->flags & TF_REFUSED_DATA) && pcb->refused_data ==
  recv_pbuf && set(refused_data->flags)): Post the refused data first,
and then the closed event.
* TCP_EVENT_CLOSED: (pcb->flags & TF_REFUSED_DATA) && pcb->refused_data
  == NULL): Post the null/EOF event (TCP_EVENT_RECV(NULL) is equivalent
to TCP_EVENT_CLOSED())

The last use-case was not supported and could potentially cause the TCP state
machine lockup in half-open half-closed connection.
---
 src/core/tcp.c         | 29 +++++++++++++----------------
 src/core/tcp_in.c      | 14 +++++++++-----
 src/include/lwip/tcp.h | 15 +++++++++++++++
 3 files changed, 37 insertions(+), 21 deletions(-)

diff --git a/src/core/tcp.c b/src/core/tcp.c
index 5b4f4e5e..51316eed 100644
--- a/src/core/tcp.c
+++ b/src/core/tcp.c
@@ -350,7 +350,7 @@ tcp_close_shutdown(struct tcp_pcb *pcb, u8_t rst_on_unacked_data)
   LWIP_ASSERT("tcp_close_shutdown: invalid pcb", pcb != NULL);
 
   if (rst_on_unacked_data && ((pcb->state == ESTABLISHED) || (pcb->state == CLOSE_WAIT))) {
-    if ((pcb->refused_data != NULL) || (pcb->rcv_wnd != TCP_WND_MAX(pcb))) {
+    if (tcp_has_refused_data(pcb) || (pcb->rcv_wnd != TCP_WND_MAX(pcb))) {
       /* Not all data received by application, send RST to tell the remote
          side about this. */
       LWIP_ASSERT("pcb->flags & TF_RXCLOSED", pcb->flags & TF_RXCLOSED);
@@ -529,10 +529,7 @@ tcp_shutdown(struct tcp_pcb *pcb, int shut_rx, int shut_tx)
       return tcp_close_shutdown(pcb, 1);
     }
     /* ... and free buffered data */
-    if (pcb->refused_data != NULL) {
-      pbuf_free(pcb->refused_data);
-      pcb->refused_data = NULL;
-    }
+    tcp_clear_refused_data_free_if_any(pcb);
   }
   if (shut_tx) {
     /* This can't happen twice since if it succeeds, the pcb's state is changed.
@@ -1510,7 +1507,7 @@ tcp_fasttmr_start:
       next = pcb->next;
 
       /* If there is data which was previously "refused" by upper layer */
-      if (pcb->refused_data != NULL) {
+      if (tcp_has_refused_data(pcb)) {
         tcp_active_pcbs_changed = 0;
         tcp_process_refused_data(pcb);
         if (tcp_active_pcbs_changed) {
@@ -1538,7 +1535,7 @@ tcp_txnow(void)
   }
 }
 
-/** Pass pcb->refused_data to the recv callback */
+/** Pass the refused_data to the recv callback */
 err_t
 tcp_process_refused_data(struct tcp_pcb *pcb)
 {
@@ -1549,19 +1546,22 @@ tcp_process_refused_data(struct tcp_pcb *pcb)
   LWIP_ERROR("tcp_process_refused_data: invalid pcb", pcb != NULL, return ERR_ARG);
 
 #if TCP_QUEUE_OOSEQ && LWIP_WND_SCALE
-  while (pcb->refused_data != NULL)
+  while (tcp_has_refused_data(pcb))
 #endif /* TCP_QUEUE_OOSEQ && LWIP_WND_SCALE */
   {
     err_t err;
-    u8_t refused_flags = pcb->refused_data->flags;
+    u8_t refused_flags = 0;
     /* set pcb->refused_data to NULL in case the callback frees it and then
        closes the pcb */
     struct pbuf *refused_data = pcb->refused_data;
+    if (refused_data != NULL) {
+        refused_flags = pcb->refused_data->flags;
+    } /* else: just sends EOF (data==NULL), both refused_flags and refused_data is 0 */
 #if TCP_QUEUE_OOSEQ && LWIP_WND_SCALE
     pbuf_split_64k(refused_data, &rest);
     pcb->refused_data = rest;
 #else /* TCP_QUEUE_OOSEQ && LWIP_WND_SCALE */
-    pcb->refused_data = NULL;
+    tcp_clear_refused_data(pcb);
 #endif /* TCP_QUEUE_OOSEQ && LWIP_WND_SCALE */
     /* Notify again application with data previously received. */
     LWIP_DEBUGF(TCP_INPUT_DEBUG, ("tcp_input: notify kept packet\n"));
@@ -1594,9 +1594,10 @@ tcp_process_refused_data(struct tcp_pcb *pcb)
 #if TCP_QUEUE_OOSEQ && LWIP_WND_SCALE
       if (rest != NULL) {
         pbuf_cat(refused_data, rest);
+        tcp_set_refused_data(pcb, refused_data);
       }
 #endif /* TCP_QUEUE_OOSEQ && LWIP_WND_SCALE */
-      pcb->refused_data = refused_data;
+      tcp_set_refused_data(pcb, refused_data);
       return ERR_INPROGRESS;
     }
   }
@@ -2143,11 +2144,7 @@ tcp_pcb_purge(struct tcp_pcb *pcb)
 
     tcp_backlog_accepted(pcb);
 
-    if (pcb->refused_data != NULL) {
-      LWIP_DEBUGF(TCP_DEBUG, ("tcp_pcb_purge: data left on ->refused_data\n"));
-      pbuf_free(pcb->refused_data);
-      pcb->refused_data = NULL;
-    }
+    tcp_clear_refused_data_log_if_any(pcb, ("tcp_pcb_purge: data left on ->refused_data\n"));
     if (pcb->unsent != NULL) {
       LWIP_DEBUGF(TCP_DEBUG, ("tcp_pcb_purge: not all data sent\n"));
     }
diff --git a/src/core/tcp_in.c b/src/core/tcp_in.c
index c7a1f7b7..9764b7de 100644
--- a/src/core/tcp_in.c
+++ b/src/core/tcp_in.c
@@ -419,9 +419,9 @@ tcp_input(struct pbuf *p, struct netif *inp)
     }
 
     /* If there is data which was previously "refused" by upper layer */
-    if (pcb->refused_data != NULL) {
+    if (tcp_has_refused_data(pcb)) {
       if ((tcp_process_refused_data(pcb) == ERR_ABRT) ||
-          ((pcb->refused_data != NULL) && (tcplen > 0))) {
+          (tcp_has_refused_data(pcb) && (tcplen > 0))) {
         /* pcb has been aborted or refused data is still refused and the new
            segment contains data */
         if (pcb->rcv_ann_wnd == 0) {
@@ -483,7 +483,7 @@ tcp_input(struct pbuf *p, struct netif *inp)
         if (recv_data != NULL) {
 #endif /* TCP_QUEUE_OOSEQ && LWIP_WND_SCALE */
 
-          LWIP_ASSERT("pcb->refused_data == NULL", pcb->refused_data == NULL);
+          LWIP_ASSERT("tcp_has_refused_data(pcb) == 0", tcp_has_refused_data(pcb) == 0);
           if (pcb->flags & TF_RXCLOSED) {
             /* received data although already closed -> abort (send RST) to
                notify the remote host that not all data has been processed */
@@ -515,7 +515,7 @@ tcp_input(struct pbuf *p, struct netif *inp)
               pbuf_cat(recv_data, rest);
             }
 #endif /* TCP_QUEUE_OOSEQ && LWIP_WND_SCALE */
-            pcb->refused_data = recv_data;
+            tcp_set_refused_data(pcb, recv_data);
             LWIP_DEBUGF(TCP_INPUT_DEBUG, ("tcp_input: keep incoming packet, because pcb is \"full\"\n"));
 #if TCP_QUEUE_OOSEQ && LWIP_WND_SCALE
             break;
@@ -529,7 +529,7 @@ tcp_input(struct pbuf *p, struct netif *inp)
         /* If a FIN segment was received, we call the callback
            function with a NULL buffer to indicate EOF. */
         if (recv_flags & TF_GOT_FIN) {
-          if (pcb->refused_data != NULL) {
+          if (tcp_has_refused_data(pcb) && pcb->refused_data != NULL) {
             /* Delay this if we have refused data. */
             pcb->refused_data->flags |= PBUF_FLAG_TCP_FIN;
           } else {
@@ -541,6 +541,10 @@ tcp_input(struct pbuf *p, struct netif *inp)
             TCP_EVENT_CLOSED(pcb, err);
             if (err == ERR_ABRT) {
               goto aborted;
+            } else if (err == ERR_MEM) {
+              /* If the upper layer can't receive the CLOSED event, flag the pcb with refused_data,
+                 but no actual data, just the event (FIN received, indicating the EVENT_CLOSED, EOF state) */
+              tcp_set_refused_data(pcb, NULL);
             }
           }
         }
diff --git a/src/include/lwip/tcp.h b/src/include/lwip/tcp.h
index 3991fd6e..43f95c4e 100644
--- a/src/include/lwip/tcp.h
+++ b/src/include/lwip/tcp.h
@@ -270,6 +270,7 @@ struct tcp_pcb {
 #if LWIP_TCP_SACK_OUT
 #define TF_SACK        0x1000U /* Selective ACKs enabled */
 #endif
+#define TF_REFUSED_DATA 0x2000U /* If set, the pcb failed to post a recv event (TCP_EVENT_CLOSED or TCP_EVENT_RECV) */
 
   /* the rest of the fields are in host byte order
      as we have to do some math with them */
@@ -424,6 +425,20 @@ void             tcp_poll    (struct tcp_pcb *pcb, tcp_poll_fn poll, u8_t interv
 #define          tcp_clear_flags(pcb, clr_flags)   do { (pcb)->flags = (tcpflags_t)((pcb)->flags & (tcpflags_t)(~(clr_flags) & TCP_ALLFLAGS)); } while(0)
 #define          tcp_is_flag_set(pcb, flag)        (((pcb)->flags & (flag)) != 0)
 
+#define          tcp_has_refused_data(pcb)        tcp_is_flag_set(pcb, TF_REFUSED_DATA)
+#define          tcp_set_refused_data(pcb, p)     do { tcp_set_flags(pcb, TF_REFUSED_DATA); pcb->refused_data = p; } while(0)
+#define          tcp_clear_refused_data_run_code_if_any(pcb, code)      do { \
+   tcp_clear_flags(pcb, TF_REFUSED_DATA); \
+   if (pcb->refused_data != NULL) { \
+        code                        \
+        pcb->refused_data = NULL;   \
+    } } while(0)
+#define          tcp_clear_refused_data_log_if_any(pcb, message)        \
+   tcp_clear_refused_data_run_code_if_any(pcb, LWIP_DEBUGF(TCP_DEBUG, message); free(pcb->refused_data); )
+#define          tcp_clear_refused_data_free_if_any(pcb)                \
+   tcp_clear_refused_data_run_code_if_any(pcb, free(pcb->refused_data); )
+#define          tcp_clear_refused_data(pcb)    tcp_clear_refused_data_run_code_if_any(pcb, )
+
 #if LWIP_TCP_TIMESTAMPS
 #define          tcp_mss(pcb)             (((pcb)->flags & TF_TIMESTAMP) ? ((pcb)->mss - 12)  : (pcb)->mss)
 #else /* LWIP_TCP_TIMESTAMPS */
-- 
2.29.2

