netdev - tcp_shift_skb_data uses wrong mss in non-gso case?

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <D3B72A8DFCA0A8418CB61D1E9E310A99E5F35219@US01XMB01.org.nasdaqomx.com>
Date:	Tue, 17 Aug 2010 18:35:45 -0400
From:	Robert Evans <robert.evans@...daqomx.com>
To:	"'netdev@...r.kernel.org'" <netdev@...r.kernel.org>
Subject: tcp_shift_skb_data uses wrong mss in non-gso case?

Hello all,

In reading through the latest SACK code introduced by 832d11c, I have noticed
that for the in_sack case tcp_shift_skb will take mss = tcp_skb_seglen(skb).
This seems to be wrong since the queue might contain small packets (f.e.
TCP_NODELAY). If the collapse succeeds, the resulting skb will have an
arbitrarily small gso_size equal to the original skb length.

8ed88d4:net/ipv4/tcp_input.c
   1506         in_sack = !after(start_seq, TCP_SKB_CB(skb)->seq) &&
   1507                   !before(end_seq, TCP_SKB_CB(skb)->end_seq);
   1508 
   1509         if (in_sack) {
   1510                 len = skb->len;
   1511                 pcount = tcp_skb_pcount(skb);
   1512                 mss = tcp_skb_seglen(skb);     /* possibly wrong? */
   1513 
   1514                 /* TODO: Fix DSACKs to not fragment already SACKed and w
   1514 e can
   1515                  * drop this restriction as unnecessary
   1516                  */
   1517                 if (mss != tcp_skb_seglen(prev))
   1518                         goto fallback;
   1519         } else {

This ends up being troublesome if the segment is later retransmitted and the
device driver has trouble with very small gso_size (e1000e seems to be an
example).

Is the small gso_size the correct and/or desired behavior?  Or am I missing
something else that prevents this from being a problem?

-Bob Evans
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html