buglwIP - A Lightweight TCP/IP stack - Bugs: bug #34636, FIN_WAIT_2 - Incorrect shutdown of...

 
 

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

bug #34636: FIN_WAIT_2 - Incorrect shutdown of TCP pcb

Submitter:  Amir Shalem <amirshalem>
Submitted:  Mon 24 Oct 2011 08:45:53 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

Jump to the original submission

Sat 21 Jan 2012 05:05:38 PM UTC, comment #8: 

Fixed.

Simon Goldschmidt <goldsimon>
Group administrator
Fri 20 Jan 2012 09:49:38 PM UTC, comment #7: 


> There is no need for a timeout if only shutdown has been called.


Agreed, I'll implement it that way.

Simon Goldschmidt <goldsimon>
Group administrator
Tue 01 Nov 2011 12:45:15 PM UTC, comment #6: 

Yes, I am confident that not starting the timeout until close is called will solve this problem.  There is no need for a timeout if only shutdown has been called.

Kieran Mansley <kieranm>
Group Member
Sat 29 Oct 2011 10:32:44 AM UTC, comment #5: 


> and lwIP TCP uses FIN_WAIT_2 state only for this faulty behavior - it has no other uses


Well, for the shutdown case that might be true, but FIN_WAIT_2 (plus the timeout you mentioned)  is also used for normal close to prevent our socket living forever after close when the remote host's FIN doesn't arrive.

Maybe a solution would be to not time if shutdown was called but close was not, like Kieran said in comment #2.

Simon Goldschmidt <goldsimon>
Group administrator
Sat 29 Oct 2011 09:36:17 AM UTC, comment #4: 

Just to make sure,
The bug is when one side (server) sends lots of data and at the end shutdown(SHUT_WR) - FIN.
The second side (client) receives data, and when it finishes to read all the data, recv() should return 0, and the TCP will send ACK to the server (first side).

The bug in question, which isn't that serious, is that although the server still holds a socket, after the lwIP will receive the ACK, it will brutally close the socket after 20ms.

I expected the behavior to be that the client can still send data - he didn't SHUT_WR his side. And the server can still read data - he didn't SHUT_RD his side (only SHUT_WR)

After I looked at the code I noticed that FIN_WAIT_2 is the reason for this faulty behavior, and lwIP TCP uses FIN_WAIT_2 state only for this faulty behavior - it has no other uses.

Amir Shalem <amirshalem>
Fri 28 Oct 2011 01:30:39 PM UTC, comment #3: 


> http://lxr.linux.no/#linux+v3.1/net/ipv4/tcp.c#L1997


Although linux is sometimes a good example of how network things are done (and we have to be compatible, I guess, given its broad usage), shouldn't we be cautious about referencing to its (GPL) code for copyright reasons?

The close vs. shutdown comment is correct though: I think we currently let go of the pcb before close() is called, so that would be the initial bug.

Simon Goldschmidt <goldsimon>
Group administrator
Fri 28 Oct 2011 12:44:34 PM UTC, comment #2: 

I agree with Simon.  Simply removing the FIN_WAIT_2 state will just create more problems than it solves.

The timeout from FIN_WAIT_2 is important to avoid a denial of service attack from the other end, where it could deliberately fail to send a FIN and keep the connection open forever.

I'd like to understand better what Linux does in this situation...

I can see it has a similar (but three minute) timeout as a result of close being called:
http://lxr.linux.no/#linux+v3.1/net/ipv4/tcp.c#L1997

I think the problem in lwIP is that we are starting the timeout while the app still has a handle on the socket (i.e. after a shutdown) but we should wait until they have called close.

Kieran Mansley <kieranm>
Group Member
Fri 28 Oct 2011 11:21:14 AM UTC, comment #1: 

The state names are a direct representation of the TCP specification, so we won't remove or rename them. In lwIP, shutdown is seen as a rather "optional" feature, so the main focus is to keep normal closing bug-free, for which both the FIN-WAIT states are needed.

Nevertheless, shutdown should work correctly, so we need to find another fix for this. I doubt however, that shutdown has been widely used with lwIP. I'll have to dig in to this.

Simon Goldschmidt <goldsimon>
Group administrator
Mon 24 Oct 2011 08:45:53 PM UTC, original submission:  

I propose to remove FIN_WAIT_2 state in TCP, because its a faulty feature.

Once you send shutdown(SHUT_WR) on a socket you should still be able to recv() from it, select() on it, etc
until the otherside will send a shutdown (FIN), or close (RST).

Currently after our netconn/sockets api does shutdown(SHUT_WR), the tcp pcb moves to FIN_WAIT_1,
which in there it waits for a completeing FIN/ACK,
and if the otherside only ACKs our FIN (because of shutdown), it will enter state FIN_WAIT_2
which there the TCP pcb will be closed brutely after TCP_FIN_WAIT_TIMEOUT (20ms)

This is not how it happens on Linux, nor Windows TCP sockets.
A cleanup patch afterwards will be to rename FIN_WAIT_1 to FIN_WAIT

Patch included.

thanks,
amir.

Amir Shalem <amirshalem>

 

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

Attached Files
file #24192:  incorrect-shutdown-remove-FIN_WAIT_2.patch added by amirshalem (3KiB - application/octet-stream - bugfix patch)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by kieranm (Posted a comment)
  • -email is unavailable- added by goldsimon (Posted a comment)
  • -email is unavailable- added by amirshalem (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-01-21 goldsimon StatusNone Fixed
        Open/ClosedOpen Closed
    2011-10-28 goldsimon Assigned toNone goldsimon
        Planned Release 1.4.1
    2011-10-24 amirshalem Attached File- Added incorrect-shutdown-remove-FIN_WAIT_2.patch, #24192

    Back to the top

    Powered by Savane 3.13-caa5.
    Corresponding source code