buglwIP - A Lightweight TCP/IP stack - Bugs: bug #27199, Mishandled TCP send window in...

 
 

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

bug #27199: Mishandled TCP send window in tcp_receive()

Submitted by:  David Empson <dempson>
Submitted on:  Fri 07 Aug 2009 04:20:41 AM UTC  
 
Category: TCPSeverity: 3 - Normal
Item Group: Faulty BehaviourStatus: Fixed
Privacy: PublicAssigned to: None
Open/Closed: ClosedPlanned Release: None
lwIP version: CVS Head

Wed 12 Aug 2009 08:35:49 AM UTC, comment #2:

Checked in, resolving fixed, thanks to all involved.

Kieran Mansley <kieranm>
Project Administrator
Mon 10 Aug 2009 10:43:52 AM UTC, comment #1:

I'll look into this. An easy point to answer is the naming of the variables: these are taken straight from the RFC, so I'd prefer to leave them as is.

Kieran Mansley <kieranm>
Project Administrator
Fri 07 Aug 2009 04:20:41 AM UTC, original submission:

Credit to Mousom <mousom_b@yahoo.com> for bringing this up in lwip-users:

There is a bug in tcp_receive(), in its handling of the send window and associated handling of acknowledgements.

There are three places where the wrong sequence number has been used to calculate the right edge of the send window. Each of these refers to the "right_wnd_edge" variable so they are easy to spot.

right_wnd_edge = pcb->snd_wnd + pcb->snd_wl1;

if (pcb->snd_wl1 + pcb->snd_wnd == right_wnd_edge)

LWIP_DEBUGF(TCP_FR_DEBUG, ("tcp_receive: dupack averted %"U32_F" %"U32_F"\n",
pcb->snd_wl1 + pcb->snd_wnd, right_wnd_edge));

In each case, the use of 'pcb->snd_wl1' is incorrect and it shoudl be 'pcb->snd_wl2'.

Explanation:

pcb->snd_wl1 is set from the received seqno, while pcb->snd_wl2 is set from the received ackno.

This means that the duplicate ack and fast retransmit calculations are being done based on a calculated right window edge which is the send window size plus the RECEIVE sequence number, and comparing to the send sequence number. This is meaningless.

I haven't looked through the code in detail to determine the likely side effects, but it appears from a quick glance that this could have implications such as unnecessary retransmission, and never closing connections due to waiting for an acknowledgement which will never arrive.

This may be the cause for several of the bugs which are listed on the bug tracker.

I'm still using LWIP of approximately 1.1.1 vintage and the bug existed at least that far back, but I've confirmed it is still there in CVS head.

As a general comment: I think it would be a good idea to rename these fields. It is not at all obvious what "snd_wl1" and "snd_wl2" are supposed to do. They have similar names so can be easily confused, and there is nothing to associate them with the appropriate direction.

David Empson <dempson>

 

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 kieranm (Posted a comment)
  • -unavailable- added by dempson (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 3 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Wed 12 Aug 2009 08:35:49 AM UTCkieranmStatusNone=>Fixed
      Open/ClosedOpen=>Closed
    Mon 10 Aug 2009 10:43:52 AM UTCkieranmPlanned Release=>1.3.1

    Back to the top


    Powered by Savane 3.1-cleanup1