buglwIP - A Lightweight TCP/IP stack - Bugs: bug #48839, Possible bug with snd_wnd semantics

 
 

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

bug #48839: Possible bug with snd_wnd semantics

Submitted by:  Ambroz Bizjak <abizjak>
Submitted on:  Sun 21 Aug 2016 12:02:51 PM UTC  
 
Category: TCPSeverity: 3 - Normal
Item Group: Faulty BehaviourStatus: Confirmed
Privacy: PublicAssigned to: Simon Goldschmidt <goldsimon>
Open/Closed: OpenPlanned Release: None
lwIP version: git head

Mon 22 Aug 2016 07:49:50 AM UTC, comment #3:

Hi Simon,
I think retransmissions can trigger a lastack update without a window update.

- Remote sends two data segments with consecutive data; the first has a lower sequence number than the second.
- Lwip receives the second segment only as the first one is lost. It updates the window based on the second segment.
- The remote later retransmits the first segment. It will have the lower sequence number of the first segment but it will have the newest ack value (rcv_nxt of remote), greater than the one of both the first and second segment originally sent, due to the remote having received some data.
- Lwip receives the retransmitted first segment. A window update is inhibited (details [1]) since the ack number acknowledges new data, lastack is updated. The window is effectively now too large.

The other kind of inconsistency seems less likely to me.

[1] The retransmitted segment has SEQ lesser than snd_wl1 and ACK different from the snd_wl2, which is sufficient to pass /* Update window. */

Ambroz Bizjak <abizjak>
Mon 22 Aug 2016 07:16:44 AM UTC, comment #2:

I see this is a theoretical inconsistency in lwIP's usage of variables. And having code where two 'if' clauses depend on different input but trigger at the same time is bad coding style.

But I'm not convinced this is a real bug (or a bug in the spec):
To convince me, you'd have to come up with a sequence of segments that produces this error.
In other words, if we receive an ACK that updates 'lastack', how would that segment fail to update 'snd_wl2'? I guess if you trust on the remote side being a good TCP, this never happens.

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Sun 21 Aug 2016 12:04:37 PM UTC, comment #1:

s/snd_wl1/snd_wl2/g

Ambroz Bizjak <abizjak>
Sun 21 Aug 2016 12:02:51 PM UTC, original submission:

Hi,
(I report this as a theoretical bug only based reading the code/specs)

The window value in a segment is relative to its ACK number. Consequently pcb->snd_wnd is relative to pcb->snd_wl1. But tcp_output seems to assume it is relative to pcb->lastack.

I suppose a discrepancy between lastack and snd_wl1 can occur when a segment is received that:
- Is recognized as a window update but not a valid acknowledgement.
- Is recognized as a valid acknowledgement but not a window update.

(I have not done any more detailed analysis when this can occur)

Depending on the case snd_wl1 may become smaller or larger than lastack. Effectively tcp_output may end up transmitting more data than the advertised window allows, or limit the amount of transmitted data unnecessarily, respectively.

Note, I also think there is a similar spec bug in RFC 793 p72, in "If SND.UNA < SEG.ACK =< SND.NXT then, set SND.UNA <- SEG.ACK." (SND.UNA is lastack, updated due to newly acked data). Here it is not considered that SND.WND is relative to SND.UNA, so by increasing SND.UNA without a corresponding decrease of SND.WND, the effective send window could now be larger than the remote permits.

I see two approaches to fix this in lwIP:
1. Change tcp_output to consider snd_wnd relative to snd_wl1 instead of lastack.
2. Change any relevant code to always keep snd_wnd relative to lastack (e.g. window update and lastack update).
... but I'm not comfortable implementing anything specific due to possible ripple effects, and also I'm currently writing my own new TCP/IP stack :)

Ambroz Bizjak <abizjak>

 

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 abizjak (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 2 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Mon 22 Aug 2016 07:16:44 AM UTCgoldsimonStatusNone=>Confirmed
      Assigned toNone=>goldsimon

    Back to the top


    Powered by Savane 3.1-cleanup1