BUG27209: handle trimming of segments when out of window or out of

order properly

CHANGELOG

@@ -19,6 +19,13 @@ HISTORY
 
   ++ New features:
 
+  ++ Bugfixes:
+
+
+(STABLE-1.3.1)
+
+  ++ New features:
+
   2009-05-10 Simon Goldschmidt
   * opt.h, sockets.c, pbuf.c, netbuf.h, pbuf.h: task #7013: Added option
     LWIP_NETIF_TX_SINGLE_PBUF to try to create transmit packets from only
@@ -108,6 +115,19 @@ HISTORY
 
 
   ++ Bugfixes:
+  2009-08-12 Kieran Mansley
+  * tcp_in.c, tcp.c: Fix bug #27209: handle trimming of segments when
+    out of window or out of order properly
+
+  2009-08-12 Kieran Mansley
+  * tcp_in.c: Fix bug #27199: use snd_wl2 instead of snd_wl1
+
+  2009-07-28 Simon Goldschmidt
+  * mem.h: Fixed bug #27105: "realloc() cannot replace mem_realloc()"s
+
+  2009-07-27 Kieran Mansley
+  * api.h api_msg.h netdb.h sockets.h: add missing #include directives
+
   2009-07-09 Kieran Mansley
   * api_msg.c, sockets.c, api.h: BUG23240 use signed counters for
     recv_avail and don't increment counters until message successfully

src/api/api_msg.c

@@ -52,6 +52,8 @@
 #include "lwip/igmp.h"
 #include "lwip/dns.h"
 
+#include <string.h>
+
 /* forward declarations */
 #if LWIP_TCP
 static err_t do_writemore(struct netconn *conn);

src/api/netdb.c

@@ -42,6 +42,9 @@
 #include "lwip/ip_addr.h"
 #include "lwip/api.h"
 
+#include <string.h>
+#include <stdlib.h>
+
 /** helper struct for gethostbyname_r to access the char* buffer */
 struct gethostbyname_r_helper {
   struct ip_addr *addrs;

src/core/dhcp.c

@@ -192,6 +192,7 @@ dhcp_check(struct netif *netif)
   u16_t msecs;
   LWIP_DEBUGF(DHCP_DEBUG | LWIP_DBG_TRACE | 3, ("dhcp_check(netif=%p) %c%c\n", (void *)netif, (s16_t)netif->name[0],
     (s16_t)netif->name[1]));
+  dhcp_set_state(dhcp, DHCP_CHECKING);
   /* create an ARP query for the offered IP address, expecting that no host
      responds, as the IP address should not be in use. */
   result = etharp_query(netif, &dhcp->offered_ip_addr, NULL);
@@ -202,7 +203,6 @@ dhcp_check(struct netif *netif)
   msecs = 500;
   dhcp->request_timeout = (msecs + DHCP_FINE_TIMER_MSECS - 1) / DHCP_FINE_TIMER_MSECS;
   LWIP_DEBUGF(DHCP_DEBUG | LWIP_DBG_TRACE | LWIP_DBG_STATE, ("dhcp_check(): set request timeout %"U16_F" msecs\n", msecs));
-  dhcp_set_state(dhcp, DHCP_CHECKING);
 }
 
 /**
@@ -248,6 +248,7 @@ dhcp_select(struct netif *netif)
 #endif /* LWIP_NETIF_HOSTNAME */
 
   LWIP_DEBUGF(DHCP_DEBUG | LWIP_DBG_TRACE | 3, ("dhcp_select(netif=%p) %c%c%"U16_F"\n", (void*)netif, netif->name[0], netif->name[1], (u16_t)netif->num));
+  dhcp_set_state(dhcp, DHCP_REQUESTING);
 
   /* create and initialize the DHCP message header */
   result = dhcp_create_request(netif);
@@ -293,12 +294,11 @@ dhcp_select(struct netif *netif)
     udp_connect(dhcp->pcb, IP_ADDR_ANY, DHCP_SERVER_PORT);
     dhcp_delete_request(netif);
     LWIP_DEBUGF(DHCP_DEBUG | LWIP_DBG_TRACE | LWIP_DBG_STATE, ("dhcp_select: REQUESTING\n"));
-    dhcp_set_state(dhcp, DHCP_REQUESTING);
   } else {
     LWIP_DEBUGF(DHCP_DEBUG | LWIP_DBG_TRACE | 2, ("dhcp_select: could not allocate DHCP request\n"));
   }
   dhcp->tries++;
-  msecs = dhcp->tries < 4 ? dhcp->tries * 1000 : 4 * 1000;
+  msecs = (dhcp->tries < 6 ? 1 << dhcp->tries : 60) * 1000;
   dhcp->request_timeout = (msecs + DHCP_FINE_TIMER_MSECS - 1) / DHCP_FINE_TIMER_MSECS;
   LWIP_DEBUGF(DHCP_DEBUG | LWIP_DBG_STATE, ("dhcp_select(): set request timeout %"U16_F" msecs\n", msecs));
   return result;
@@ -790,6 +790,7 @@ dhcp_discover(struct netif *netif)
   u16_t msecs;
   LWIP_DEBUGF(DHCP_DEBUG | LWIP_DBG_TRACE | 3, ("dhcp_discover()\n"));
   ip_addr_set(&dhcp->offered_ip_addr, IP_ADDR_ANY);
+  dhcp_set_state(dhcp, DHCP_SELECTING);
   /* create and initialize the DHCP message header */
   result = dhcp_create_request(netif);
   if (result == ERR_OK) {
@@ -817,7 +818,6 @@ dhcp_discover(struct netif *netif)
     LWIP_DEBUGF(DHCP_DEBUG | LWIP_DBG_TRACE, ("dhcp_discover: deleting()ing\n"));
     dhcp_delete_request(netif);
     LWIP_DEBUGF(DHCP_DEBUG | LWIP_DBG_TRACE | LWIP_DBG_STATE, ("dhcp_discover: SELECTING\n"));
-    dhcp_set_state(dhcp, DHCP_SELECTING);
   } else {
     LWIP_DEBUGF(DHCP_DEBUG | LWIP_DBG_TRACE | 2, ("dhcp_discover: could not allocate DHCP request\n"));
   }
@@ -828,7 +828,7 @@ dhcp_discover(struct netif *netif)
     autoip_start(netif);
   }
 #endif /* LWIP_DHCP_AUTOIP_COOP */
-  msecs = dhcp->tries < 4 ? (dhcp->tries + 1) * 1000 : 10 * 1000;
+  msecs = (dhcp->tries < 6 ? 1 << dhcp->tries : 60) * 1000;
   dhcp->request_timeout = (msecs + DHCP_FINE_TIMER_MSECS - 1) / DHCP_FINE_TIMER_MSECS;
   LWIP_DEBUGF(DHCP_DEBUG | LWIP_DBG_TRACE | LWIP_DBG_STATE, ("dhcp_discover(): set request timeout %"U16_F" msecs\n", msecs));
   return result;
@@ -1409,9 +1409,12 @@ dhcp_create_request(struct netif *netif)
   LWIP_ASSERT("dhcp_create_request: check that first pbuf can hold struct dhcp_msg",
            (dhcp->p_out->len >= sizeof(struct dhcp_msg)));
 
-  /* give unique transaction identifier to this request */
-  dhcp->xid = xid++;
-  LWIP_DEBUGF(DHCP_DEBUG | LWIP_DBG_TRACE | 2, ("transaction id xid++(%"X32_F") dhcp->xid(%"U32_F")\n",xid,dhcp->xid));
+  /* reuse transaction identifier in retransmissions */
+  if (dhcp->tries==0)
+      xid++;
+  dhcp->xid = xid;
+  LWIP_DEBUGF(DHCP_DEBUG | LWIP_DBG_TRACE | 2,
+              ("transaction id xid(%"X32_F")\n", xid));
 
   dhcp->msg_out = (struct dhcp_msg *)dhcp->p_out->payload;
 

src/core/ipv4/inet_chksum.c

@@ -41,6 +41,8 @@
 #include "lwip/inet_chksum.h"
 #include "lwip/inet.h"
 
+#include <stddef.h>
+
 /* These are some reference implementations of the checksum algorithm, with the
  * aim of being simple, correct and fully portable. Checksumming is the
  * first thing you would want to optimize for your platform. If you create

src/core/ipv4/ip.c

@@ -56,6 +56,8 @@
 #include "lwip/stats.h"
 #include "arch/perf.h"
 
+#include <string.h>
+
 /**
  * The interface that provided the packet for the current callback
  * invocation.

src/core/pbuf.c

@@ -200,7 +200,8 @@ pbuf_alloc(pbuf_layer layer, u16_t length, pbuf_type type)
     LWIP_ASSERT("check p->payload + p->len does not overflow pbuf",
                 ((u8_t*)p->payload + p->len <=
                  (u8_t*)p + SIZEOF_STRUCT_PBUF + PBUF_POOL_BUFSIZE_ALIGNED));
-    LWIP_ASSERT("PBUF_POOL_BUFSIZE must be bigger than MEM_ALIGNMENT", p->len != 0);
+    LWIP_ASSERT("PBUF_POOL_BUFSIZE must be bigger than MEM_ALIGNMENT",
+      (PBUF_POOL_BUFSIZE_ALIGNED - LWIP_MEM_ALIGN_SIZE(offset)) > 0 );
     /* set reference count (needed here in case we fail) */
     p->ref = 1;
 

src/core/tcp.c

@@ -49,6 +49,7 @@
 #include "lwip/memp.h"
 #include "lwip/snmp.h"
 #include "lwip/tcp.h"
+#include "lwip/debug.h"
 
 #include <string.h>
 
@@ -423,6 +424,9 @@ tcp_recved(struct tcp_pcb *pcb, u16_t len)
 {
   int wnd_inflation;
 
+  LWIP_ASSERT("tcp_recved: len would wrap rcv_wnd\n",
+              len <= 0xffff - pcb->rcv_wnd );
+
   pcb->rcv_wnd += len;
   if (pcb->rcv_wnd > TCP_WND)
     pcb->rcv_wnd = TCP_WND;
@@ -1399,6 +1403,7 @@ tcp_debug_print_flags(u8_t flags)
   if (flags & TCP_CWR) {
     LWIP_DEBUGF(TCP_DEBUG, ("CWR "));
   }
+  LWIP_DEBUGF(TCP_DEBUG, ("\n"));
 }
 
 /**

src/core/tcp_in.c

@@ -408,6 +408,7 @@ tcp_listen_input(struct tcp_pcb_listen *pcb)
     LWIP_DEBUGF(TCP_DEBUG, ("TCP connection request %"U16_F" -> %"U16_F".\n", tcphdr->src, tcphdr->dest));
 #if TCP_LISTEN_BACKLOG
     if (pcb->accepts_pending >= pcb->backlog) {
+      LWIP_DEBUGF(TCP_DEBUG, ("tcp_listen_input: listen backlog exceeded for port %"U16_F"\n", tcphdr->dest));
       return ERR_ABRT;
     }
 #endif /* TCP_LISTEN_BACKLOG */
@@ -737,7 +738,7 @@ tcp_receive(struct tcp_pcb *pcb)
   u8_t accepted_inseq = 0;
 
   if (flags & TCP_ACK) {
-    right_wnd_edge = pcb->snd_wnd + pcb->snd_wl1;
+    right_wnd_edge = pcb->snd_wnd + pcb->snd_wl2;
 
     /* Update window. */
     if (TCP_SEQ_LT(pcb->snd_wl1, seqno) ||
@@ -764,7 +765,7 @@ tcp_receive(struct tcp_pcb *pcb)
     if (pcb->lastack == ackno) {
       pcb->acked = 0;
 
-      if (pcb->snd_wl1 + pcb->snd_wnd == right_wnd_edge){
+      if (pcb->snd_wl2 + pcb->snd_wnd == right_wnd_edge){
         ++pcb->dupacks;
         if (pcb->dupacks >= 3 && pcb->unacked != NULL) {
           if (!(pcb->flags & TF_INFR)) {
@@ -801,7 +802,7 @@ tcp_receive(struct tcp_pcb *pcb)
         }
       } else {
         LWIP_DEBUGF(TCP_FR_DEBUG, ("tcp_receive: dupack averted %"U32_F" %"U32_F"\n",
-                                   pcb->snd_wl1 + pcb->snd_wnd, right_wnd_edge));
+                                   pcb->snd_wl2 + pcb->snd_wnd, right_wnd_edge));
       }
     } else if (TCP_SEQ_BETWEEN(ackno, pcb->lastack+1, pcb->snd_nxt)){
       /* We come here when the ACK acknowledges new data. */
@@ -1054,35 +1055,73 @@ tcp_receive(struct tcp_pcb *pcb)
         /* The incoming segment is the next in sequence. We check if
            we have to trim the end of the segment and update rcv_nxt
            and pass the data to the application. */
+        tcplen = TCP_TCPLEN(&inseg);
+
+        if (tcplen > pcb->rcv_wnd) {
+          LWIP_DEBUGF(TCP_INPUT_DEBUG, 
+                      ("tcp_receive: other end overran receive window"
+                       "seqno %"U32_F" len %"U32_F" right edge %"U32_F"\n",
+                       seqno, tcplen, pcb->rcv_nxt + pcb->rcv_wnd));
+          if (TCPH_FLAGS(inseg.tcphdr) & TCP_FIN) {
+            /* Must remove the FIN from the header as we're trimming 
+             * that byte of sequence-space from the packet */
+            TCPH_FLAGS_SET(inseg.tcphdr, TCPH_FLAGS(inseg.tcphdr) &~ TCP_FIN);
+          }
+          /* Adjust length of segment to fit in the window. */
+          inseg.len = pcb->rcv_wnd;
+          if (TCPH_FLAGS(inseg.tcphdr) & TCP_SYN) {
+            inseg.len -= 1;
+          }
+          pbuf_realloc(inseg.p, inseg.len);
+          tcplen = TCP_TCPLEN(&inseg);
+          LWIP_ASSERT("tcp_receive: segment not trimmed correctly to rcv_wnd\n",
+                      (seqno + tcplen) == (pcb->rcv_nxt + pcb->rcv_wnd));
+        }
 #if TCP_QUEUE_OOSEQ
-        if (pcb->ooseq != NULL &&
-                TCP_SEQ_LEQ(pcb->ooseq->tcphdr->seqno, seqno + inseg.len)) {
-          if (pcb->ooseq->len > 0) {
-            /* We have to trim the second edge of the incoming
-               segment. */
-            inseg.len = (u16_t)(pcb->ooseq->tcphdr->seqno - seqno);
-            pbuf_realloc(inseg.p, inseg.len);
-          } else {
-            /* does the ooseq segment contain only flags that are in inseg also? */
-            if ((TCPH_FLAGS(inseg.tcphdr) & (TCP_FIN|TCP_SYN)) ==
-                (TCPH_FLAGS(pcb->ooseq->tcphdr) & (TCP_FIN|TCP_SYN))) {
+        if (pcb->ooseq != NULL) {
+          if (TCPH_FLAGS(inseg.tcphdr) & TCP_FIN) {
+            LWIP_DEBUGF(TCP_INPUT_DEBUG, 
+                        ("tcp_receive: received in-order FIN, binning ooseq queue\n"));
+            /* Received in-order FIN means anything that was received
+             * out of order must now have been received in-order, so
+             * bin the ooseq queue */
+            while (pcb->ooseq != NULL) {
               struct tcp_seg *old_ooseq = pcb->ooseq;
               pcb->ooseq = pcb->ooseq->next;
               memp_free(MEMP_TCP_SEG, old_ooseq);
+            }               
+          } else if (TCP_SEQ_LEQ(pcb->ooseq->tcphdr->seqno, seqno + tcplen)) {
+            if (pcb->ooseq->len > 0) {
+              /* We have to trim the second edge of the incoming segment. */
+              LWIP_ASSERT("tcp_receive: trimmed segment would have zero length\n",
+                          TCP_SEQ_GT(pcb->ooseq->tcphdr->seqno, seqno));
+              /* FIN in inseg already handled by dropping whole ooseq queue */
+              inseg.len = (u16_t)(pcb->ooseq->tcphdr->seqno - seqno);
+              if (TCPH_FLAGS(inseg.tcphdr) & TCP_SYN) {
+                inseg.len -= 1;
+              }
+              pbuf_realloc(inseg.p, inseg.len);
+              tcplen = TCP_TCPLEN(&inseg);
+              LWIP_ASSERT("tcp_receive: segment not trimmed correctly to ooseq queue\n",
+                          (seqno + tcplen) == pcb->ooseq->tcphdr->seqno);
+            } else {
+              /* does the ooseq segment contain only flags that are in inseg also? */
+              if ((TCPH_FLAGS(inseg.tcphdr) & (TCP_FIN|TCP_SYN)) ==
+                  (TCPH_FLAGS(pcb->ooseq->tcphdr) & (TCP_FIN|TCP_SYN))) {
+                struct tcp_seg *old_ooseq = pcb->ooseq;
+                pcb->ooseq = pcb->ooseq->next;
+                memp_free(MEMP_TCP_SEG, old_ooseq);
+              }
             }
           }
         }
 #endif /* TCP_QUEUE_OOSEQ */
 
-        tcplen = TCP_TCPLEN(&inseg);
         pcb->rcv_nxt = seqno + tcplen;
 
         /* Update the receiver's (our) window. */
-        if (pcb->rcv_wnd < tcplen) {
-          pcb->rcv_wnd = 0;
-        } else {
-          pcb->rcv_wnd -= tcplen;
-        }
+        LWIP_ASSERT("tcp_receive: tcplen > rcv_wnd\n", pcb->rcv_wnd >= tcplen);
+        pcb->rcv_wnd -= tcplen;
 
         tcp_update_rcv_ann_wnd(pcb);
 
@@ -1117,11 +1156,9 @@ tcp_receive(struct tcp_pcb *pcb)
           seqno = pcb->ooseq->tcphdr->seqno;
 
           pcb->rcv_nxt += TCP_TCPLEN(cseg);
-          if (pcb->rcv_wnd < TCP_TCPLEN(cseg)) {
-            pcb->rcv_wnd = 0;
-          } else {
-            pcb->rcv_wnd -= TCP_TCPLEN(cseg);
-          }
+          LWIP_ASSERT("tcp_receive: ooseq tcplen > rcv_wnd\n",
+                      pcb->rcv_wnd >= TCP_TCPLEN(cseg));
+          pcb->rcv_wnd -= TCP_TCPLEN(cseg);
 
           tcp_update_rcv_ann_wnd(pcb);
 

src/include/ipv4/lwip/ip_addr.h

@@ -144,11 +144,15 @@ u8_t ip_addr_isbroadcast(struct ip_addr *, struct netif *);
 #define ip_addr_islinklocal(addr1) (((addr1)->addr & ntohl(0xffff0000UL)) == ntohl(0xa9fe0000UL))
 
 #define ip_addr_debug_print(debug, ipaddr) \
-        LWIP_DEBUGF(debug, ("%"U16_F".%"U16_F".%"U16_F".%"U16_F, \
-                ipaddr ? (u16_t)(ntohl((ipaddr)->addr) >> 24) & 0xff : 0, \
-                ipaddr ? (u16_t)(ntohl((ipaddr)->addr) >> 16) & 0xff : 0, \
-                ipaddr ? (u16_t)(ntohl((ipaddr)->addr) >> 8) & 0xff : 0, \
-                ipaddr ? (u16_t)ntohl((ipaddr)->addr) & 0xff : 0))
+  LWIP_DEBUGF(debug, ("%"U16_F".%"U16_F".%"U16_F".%"U16_F,              \
+                      ipaddr != NULL ?                                  \
+                      (u16_t)(ntohl((ipaddr)->addr) >> 24) & 0xff : 0,  \
+                      ipaddr != NULL ?                                  \
+                      (u16_t)(ntohl((ipaddr)->addr) >> 16) & 0xff : 0,  \
+                      ipaddr != NULL ?                                  \
+                      (u16_t)(ntohl((ipaddr)->addr) >> 8) & 0xff : 0,   \
+                      ipaddr != NULL ?                                  \
+                      (u16_t)ntohl((ipaddr)->addr) & 0xff : 0))
 
 /* These are cast to u16_t, with the intent that they are often arguments
  * to printf using the U16_F format from cc.h. */

src/include/lwip/api.h

@@ -36,6 +36,8 @@
 
 #if LWIP_NETCONN /* don't build if not configured for use in lwipopts.h */
 
+#include <stddef.h> /* for size_t */
+
 #include "lwip/netbuf.h"
 #include "lwip/sys.h"
 #include "lwip/ip_addr.h"

src/include/lwip/api_msg.h

@@ -36,6 +36,8 @@
 
 #if LWIP_NETCONN /* don't build if not configured for use in lwipopts.h */
 
+#include <stddef.h> /* for size_t */
+
 #include "lwip/ip_addr.h"
 #include "lwip/err.h"
 #include "lwip/sys.h"

src/include/lwip/init.h

@@ -44,10 +44,10 @@ extern "C" {
 #define LWIP_VERSION_MINOR      3U
 /** x.x.X: Revision of the stack */
 #define LWIP_VERSION_REVISION   1U
-/** For release candidates, this is set to 1..255
+/** For release candidates, this is set to 1..254
   * For official releases, this is set to 255 (LWIP_RC_RELEASE)
   * For development versions (CVS), this is set to 0 (LWIP_RC_DEVELOPMENT) */
-#define LWIP_VERSION_RC         1U
+#define LWIP_VERSION_RC         2U
 
 /** LWIP_VERSION_RC is set to LWIP_RC_RELEASE for official releases */
 #define LWIP_RC_RELEASE         255U

src/include/lwip/mem.h

@@ -59,7 +59,11 @@ typedef size_t mem_size_t;
 #define mem_calloc calloc
 #endif
 #ifndef mem_realloc
-#define mem_realloc realloc
+static void *mem_realloc(void *mem, mem_size_t size)
+{
+  LWIP_UNUSED_ARG(size);
+  return mem;
+}
 #endif
 #else /* MEM_LIBC_MALLOC */
 

src/include/lwip/netdb.h

@@ -31,6 +31,8 @@
 
 #if LWIP_DNS && LWIP_SOCKET
 
+#include <stddef.h> /* for size_t */
+
 #include "lwip/sockets.h"
 
 /* some rarely used options */

src/include/lwip/sockets.h

@@ -38,6 +38,8 @@
 
 #if LWIP_SOCKET /* don't build if not configured for use in lwipopts.h */
 
+#include <stddef.h> /* for size_t */
+
 #include "lwip/ip_addr.h"
 #include "lwip/inet.h"
 

src/include/lwip/tcpip.h

@@ -41,6 +41,7 @@
 #include "lwip/pbuf.h"
 #include "lwip/api.h"
 #include "lwip/sys.h"
+#include "lwip/netif.h"
 
 #ifdef __cplusplus
 extern "C" {