[lwip-devel] [bug #31177] lwIP 1.3.2 tcp_slowtmr() can corrupt tcp_active_pcbs in some cases.

[Dave Wilson]

URL:
  <http://savannah.nongnu.org/bugs/?31177>

                 Summary: lwIP 1.3.2 tcp_slowtmr() can corrupt
tcp_active_pcbs in some cases.
                 Project: lwIP - A Lightweight TCP/IP stack
            Submitted by: dawilson
            Submitted on: Thu 30 Sep 2010 08:49:10 PM GMT
                Category: TCP
                Severity: 3 - Normal
              Item Group: Faulty Behaviour
                  Status: None
                 Privacy: Public
             Assigned to: None
             Open/Closed: Open
         Discussion Lock: Any
         Planned Release: 
            lwIP version: CVS Head

    _______________________________________________________

Details:

If a client error callback function (registered using tcp_err) allocates a
new PCB then function tcp_slowtmr() can corrupt tcp_active_pcbs in some cases.
This was seen in an application which makes 2 connections to a telnet server.
In cases where the server is down when the initial connection attempt is made
and tcp_slowtmr() detects the "Max SYN retries" case and removes the PCBs from
tcp_active_pcbs list, we got into a state where one of the 2 connections was
never reestablished when the server came back online.

Investigation showed that the root cause of the problem was an assumption
that local variable "prev" would not need updated after a PCB is removed
(around line 755 or thereabouts of tcp.c). This is correct if the application
error callback doesn't allocate any PCBs but, if it does, this leads to
problems. In our case, the wrong PCB was removed on the next PCB loop
iteration and our connection "died".

To fix the problem, the following code can be added immediately after the
line "pcb = pcb2;" in tcp_slowtmr().

      /* Determine the previous PCB in the list.  We
         need to do this in case the client error handler
         did something that allocates a new PCB. If this
         happens and the PCB we just freed was at the head
         of the list, we end up with a bad value in prev and
         can corrupt the list. */
      prev = tcp_active_pcbs;
      while(prev->next != pcb){
        prev = prev->next;
      }




    _______________________________________________________

Reply to this item at:

  <http://savannah.nongnu.org/bugs/?31177>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.nongnu.org/