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

Submitted by:  Yosic <yosic>
Submitted on:  Thu 02 Feb 2012 12:24:58 PM UTC  
 
Category: TCPSeverity: 3 - Normal
Item Group: Faulty BehaviourStatus: Fixed
Privacy: PublicAssigned to: Simon Goldschmidt <goldsimon>
Open/Closed: ClosedPlanned 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>
Project AdministratorIn charge of this item.
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>
Project AdministratorIn charge of this item.
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>
Project Administrator
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>

 

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

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

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 5 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Sat 11 Feb 2012 05:16:10 PM UTCgoldsimonStatusIn Progress=>Fixed
      Open/ClosedOpen=>Closed
    Mon 06 Feb 2012 08:37:59 AM UTCgoldsimonStatusNone=>In Progress
      Assigned toNone=>goldsimon
      Planned Release=>1.4.1

    Back to the top


    Powered by Savane 3.1-cleanup1