[lwip-devel] [bug #46384] Segment size calculation bug with MSS != TCP_MSS

[Deomid Ryabkov]

URL:
  <http://savannah.nongnu.org/bugs/?46384>

                 Summary: Segment size calculation bug with MSS != TCP_MSS
                 Project: lwIP - A Lightweight TCP/IP stack
            Submitted by: rojer
            Submitted on: Thu 05 Nov 2015 11:59:35 PM GMT
                Category: TCP
                Severity: 3 - Normal
              Item Group: Faulty Behaviour
                  Status: None
                 Privacy: Public
             Assigned to: None
             Open/Closed: Open
         Discussion Lock: Any
         Planned Release: 
            lwIP version: git head

    _______________________________________________________

Details:

tcp_write() performs segmentation in 3 phases: (1) filling remaining portion
of the last segment's pbuf, then (2) filling the segment with a new pbuf up to
MSS (- headers), then (3) creating a new segment.
However, step (1) does not handle a case where pbuf may have more space
available than MSS allows. This happens when MSS is less than TCP_MSS, such as
when peer explicitly asked for lower MSS during handshake (e.g. if its
interface has lower MTU).


specifically, this is the problematic check:

http://git.savannah.gnu.org/cgit/lwip.git/tree/src/core/tcp_out.c#n472

oversize_used = oversize < len ? oversize : len;

it decides how much "oversize" i.e. remaining space in pbuf to use, and caps
it by the number of bytes passed to tcp_write, but not by the amount left in
this segment, which is "space". it then subtracts the amount from space,
which, if both len and oversize were greater than space, will cause it to wrap
around to 65000 and, if len was big enough, aggravate the problem by also
creating additional pbuf in this segment in step (2). this results in
oversized segment being transmitted on the wire, possibly larger than even the
sending interface's MTU, which can lead to all sorts of problems.

the problematic condition should be changed to take space into account and
prevent wraparound, to something like this:

oversize_used = LWIP_MIN(space, LWIP_MIN(oversize, len));

one could also argue that smaller pbuf should be allocated if connection's MSS
is smaller than TCP_MSS. this would be memory efficient, since we're not
allowed to use full TCP_MSS anyway.
i have not looked at that, this fix was sufficient for my purposes.




    _______________________________________________________

Reply to this item at:

  <http://savannah.nongnu.org/bugs/?46384>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.nongnu.org/