Thu 05 Nov 2015 11:59:35 PM UTC, original submission:
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.
|