buglwIP - A Lightweight TCP/IP stack - Bugs: bug #35435, No pcb state check before adding...

 
 

You are not allowed to post comments on this tracker with your current authentication level.

bug #35435: No pcb state check before adding it to time-wait queue while closing

Submitter:  Yosic <yosic>
Submitted:  Thu 02 Feb 2012 12:24:58 PM UTC
   
 
Category:  TCP Severity:  3 - Normal
Item Group:  Faulty Behaviour Status:  Fixed
Privacy:  Public Assigned to:  goldsimon
Open/Closed:  Closed Planned Release:  None
lwIP version:  CVS Head

Sat 11 Feb 2012 05:16:10 PM UTC, comment #4: 

Fixed by only sending RST from ESTABLISHED or CLOSE_WAIT, moving to TIME_WAIT only from ESTABLISHED and deallocating the pcb from CLOSE_WAIT (since we sent a RST).

Simon Goldschmidt <goldsimon>
Group administrator
Mon 06 Feb 2012 08:37:59 AM UTC, comment #3: 

Kieran, your comment is correct, but there's a bug in the code that sends RST if not all received data is processed by the application when close is called. In that state, TIME_WAIT is entered without a check. I'll fix that.

Simon Goldschmidt <goldsimon>
Group administrator
Mon 06 Feb 2012 07:18:15 AM UTC, comment #2: 

May be it is so. But any way it is. I don't know the origin of the problem but in the function I mentioned an attempt to add pcb to time-wait queue is made. May be it is required some other sort of check not to get to this function at all. But as for now it tries to put pcb into time-wait queue twice. I tested it on the head.
I've followed the call stack and found that there is no check against connection has been closed while one tries to close it through sockets api. So It may be already closed and second attempt will try to close it second time. May be it is the origin of the problem? A check before tcp_close_shutdown against connection not has been closed is required to avoid the issue?
tcp_close seems like a good place to such kind of check. In particular in tcp_shutdown it is already a check against this condition. So may be it is required it tcp_close also?

Yosic <yosic>
Sat 04 Feb 2012 03:41:50 PM UTC, comment #1: 

But a pcb that hasn't been closed shouldn't already be in time_wait.  It would be in close_wait.

Kieran Mansley <kieranm>
Group Member
Thu 02 Feb 2012 12:24:58 PM UTC, original submission:  

tcp.c function tcp_close_shutdown.
There are lines after "/* move to TIME_WAIT since we close actively */"
A check against pcb being already in time-wait queue is required here since it can lead to the behavior when an already added pcb is added second time and later, in a timer, we have infinite looping. This behavior is reproducible when one tries to close connection right after another part has requested close as well. In particular I got this issue wile closing a connection of http server after serving a request without keep-alives.
The browser requests connection termination just before I did and my system hangs in infinite loop in a timer(don't remember exactly which one).
I have fixed the issue with the following code. But I'm not sure is my fixed correct from the whole stack point of view since I'm not familiar with all it's internals. Any way the issue should be fixed since it major problem. I had it reproducible every request.
Instead of :
      TCP_RMV(&tcp_active_pcbs, pcb);
      pcb->state = TIME_WAIT;
      TCP_REG(&tcp_tw_pcbs, pcb);

I use :
      if(pcb->state != TIME_WAIT) // check state
      {
          TCP_RMV(&tcp_active_pcbs, pcb);
          pcb->state = TIME_WAIT;
          TCP_REG(&tcp_tw_pcbs, pcb);
      }

Yosic <yosic>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by goldsimon (Posted a comment)
  • -email is unavailable- added by kieranm (Posted a comment)
  • -email is unavailable- added by yosic (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

     

    Follow 5 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2012-02-11 goldsimon StatusIn Progress Fixed
        Open/ClosedOpen Closed
    2012-02-06 goldsimon StatusNone In Progress
        Assigned toNone goldsimon
        Planned Release 1.4.1

    Back to the top

    Powered by Savane 3.13-caa5.
    Corresponding source code