linux-kernel - Re: [PATCH net v2 1/2] tcp: process the 3rd ACK with sk

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-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20240723214112.61715-1-kuniyu@amazon.com>
Date: Tue, 23 Jul 2024 14:41:12 -0700
From: Kuniyuki Iwashima <kuniyu@...zon.com>
To: <matttbe@...nel.org>
CC: <davem@...emloft.net>, <dsahern@...nel.org>, <edumazet@...gle.com>,
	<kuba@...nel.org>, <kuniyu@...zon.com>, <linux-kernel@...r.kernel.org>,
	<mptcp@...ts.linux.dev>, <ncardwell@...gle.com>, <netdev@...r.kernel.org>,
	<pabeni@...hat.com>
Subject: Re: [PATCH net v2 1/2] tcp: process the 3rd ACK with sk_socket for TFO/MPTCP

From: Matthieu Baerts <matttbe@...nel.org>
Date: Tue, 23 Jul 2024 21:09:40 +0200
[...]
> >>> Problem of this 'goto consume' is that we are not properly sending a
> >>> DUPACK in this case.
> >>>
> >>>  +.01 socket(..., SOCK_STREAM, IPPROTO_TCP) = 4
> >>>    +0 fcntl(4, F_SETFL, O_RDWR|O_NONBLOCK) = 0
> >>>    +0 sendto(4, ..., 1000, MSG_FASTOPEN, ..., ...) = 1000
> >>>    +0 > S 0:1000(1000) <mss 1460,nop,nop,sackOK,nop,wscale 8,FO
> >>> abcd1234,nop,nop>
> >>> // Simul. SYN-data crossing: we don't support that yet so ack only remote ISN
> >>> +.005 < S 1234:1734(500) win 14600 <mss 1040,nop,nop,sackOK,nop,wscale
> >>> 6,FO 87654321,nop,nop>
> >>>    +0 > S. 0:0(0) ack 1235 <mss 1460,nop,nop,sackOK,nop,wscale 8>
> >>>
> >>> +.045 < S. 1234:1234(0) ack 1001 win 14600 <mss
> >>> 940,nop,nop,sackOK,nop,wscale 6,FO 12345678,nop,nop>
> >>>    +0 > . 1001:1001(0) ack 1 <nop,nop,sack 0:1>  // See here
> >>
> >> I'm sorry, but is it normal to have 'ack 1' with 'sack 0:1' here?
> > 
> > It is normal, because the SYN was already received/processed.
> > 
> > sack 0:1 represents this SYN sequence.
> 
> Thank you for your reply!
> 
> Maybe it is just me, but does it not look strange to have the SACK
> covering a segment (0:1) that is before the ACK (1)?
> 
> 'ack 1' and 'sack 0:1' seem to cover the same block, no?
> Before Kuniyuki's patch, this 'sack 0:1' was not present.

This looks a bit strange to me too, but I guess this is also not
forbidden ?