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()

Submitter:  David Empson <dempson>
Submitted:  Fri 07 Aug 2009 04:20:41 AM UTC
   
 
Category:  TCP Severity:  3 - Normal
Item Group:  Faulty Behaviour Status:  Fixed
Privacy:  Public Assigned to:  None
Open/Closed:  Closed Planned 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>
Group Member
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>
Group Member
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>

 

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

    Date Changed by Updated Field Previous Value => Replaced by
    2009-08-12 kieranm StatusNone Fixed
        Open/ClosedOpen Closed
    2009-08-10 kieranm Planned Release 1.3.1

    Back to the top

    Powered by Savane 3.13-d3ae.
    Corresponding source code