netdev - Re: [MPTCP] Re: [PATCH net-next 09/11] tcp: Check for filled TCP option space before SACK

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 for Android: free password hash cracker in your pocket

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

Message-ID: <efc70920e3dcdb694ad5791b845f4cf05478b07f.camel@redhat.com>
Date:   Mon, 16 Dec 2019 18:28:13 +0100
From:   Paolo Abeni <pabeni@...hat.com>
To:     Eric Dumazet <eric.dumazet@...il.com>,
        Mat Martineau <mathew.j.martineau@...ux.intel.com>,
        netdev@...r.kernel.org, mptcp@...ts.01.org
Subject: Re: [MPTCP] Re: [PATCH net-next 09/11] tcp: Check for filled TCP
 option space before SACK

On Mon, 2019-12-16 at 08:55 -0800, Eric Dumazet wrote:
> On 12/16/19 4:52 AM, Paolo Abeni wrote:
> > Thank you for the feedback!
> > 
> > Unfortunatelly, the above commit is not enough when MPTCP is enabled,
> > as, without this patch, we can reach the following code:
> > 
> > 		const unsigned int remaining = MAX_TCP_OPTION_SPACE - size;
> > 		opts->num_sack_blocks =
> > 			min_t(unsigned int, eff_sacks,
> > 			      (remaining - TCPOLEN_SACK_BASE_ALIGNED) /
> > 			      TCPOLEN_SACK_PERBLOCK);
> > 
> > with 'size == MAX_TCP_OPTION_SPACE' and num_sack_blocks will be
> > miscalculated. So we need 'fix' but only for MPTCP/when MPTCP is
> > enabled. Still ok for a -net commit?
> > 
> 
> Does it means MPTCP flows can not use SACK at all ? That would be very bad.
> 
> What is the size of MPTCP options ?

MPTCP DSS+ACK+csum is up to 28 bytes, but an MPTCP stream does not need
to attach such option to each packet.

Bare MPTCP ACK are at most 12 bytes, so there is enough room for sack.

The MPTCP specs allow for different opt sizes, because e.g. sequence
numbers can be either 32 or 64 bits. Currently we only implement 64
bits seq.

Paolo