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

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

(Jump to the original submission Jump to the original submission)

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

Fixed.

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
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>
Project AdministratorIn charge of this item.
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>
Project Administrator
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>
Project AdministratorIn charge of this item.
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>
Project AdministratorIn charge of this item.
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>
Project Administrator
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>
Project AdministratorIn charge of this item.
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>

 

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
  • -unavailable- added by kieranm (Posted a comment)
  • -unavailable- added by goldsimon (Posted a comment)
  • -unavailable- added by amirshalem (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 21 Jan 2012 05:05:38 PM UTCgoldsimonStatusNone=>Fixed
      Open/ClosedOpen=>Closed
    Fri 28 Oct 2011 11:21:14 AM UTCgoldsimonAssigned toNone=>goldsimon
      Planned Release=>1.4.1
    Mon 24 Oct 2011 08:45:53 PM UTCamirshalemAttached File-=>Added incorrect-shutdown-remove-FIN_WAIT_2.patch, #24192

    Back to the top


    Powered by Savane 3.1-cleanup1