netdev - Re: [PATCH 1/1] tcp: accommodate sequence number to a peer's shrunk receive window caused by precision loss in window scaling

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: <20170217.110858.1023681409351044871.davem@davemloft.net>
Date:   Fri, 17 Feb 2017 11:08:58 -0500 (EST)
From:   David Miller <davem@...emloft.net>
To:     Cheng.Cui@...app.com
Cc:     netdev@...r.kernel.org, kuznet@....inr.ac.ru, jmorris@...ei.org,
        yoshfuji@...ux-ipv6.org, kaber@...sh.net
Subject: Re: [PATCH 1/1] tcp: accommodate sequence number to a peer's
 shrunk receive window caused by precision loss in window scaling

From: "Cui, Cheng" <Cheng.Cui@...app.com>
Date: Tue, 14 Feb 2017 23:14:59 +0000

> This updates tcp_acceptable_seq(), one of the oldest functions since 2.4.0, by
> preventing sending out a left-shifted sequence number from a Linux sender in
> response to a peer's shrunk receive-window caused by losing least significant
> bits in window-scaling.
> 
> RFC7323 page 10 (Chapter 2.4. Addressing Window Retraction) specifies sender
> side responsibility to handle the sequence number out of window:
>> On the sender side:
>> 
>>    3)  The initial transmission MUST be within the window announced by
>>        the most recent <ACK>.
> 
> Some related discussion can be found at the IETF [tcpm] mailing list:
> https://mailarchive.ietf.org/arch/msg/tcpm/pPO7cYxtky27Qto9b30eaHB_RQI
> 
> The issue has been reproduced and the patch has been verified by scp a 20GB file
> from a Linux box using kernel version 4.4.48 to a FreeBSD 11.0 box.
> 
> [ I mainly want feedback to see if everyone is OK with the approach. ]
> 
> Cc: "David S. Miller" <davem@...emloft.net>
> Cc: Alexey Kuznetsov <kuznet@....inr.ac.ru>
> Cc: James Morris <jmorris@...ei.org>
> Cc: Hideaki YOSHIFUJI <yoshfuji@...ux-ipv6.org>
> Cc: Patrick McHardy <kaber@...sh.net>
> Signed-off-by: Cheng Cui <Cheng.Cui@...app.com>

Patch looks fine to me, but too much parenthesis in the initial conditional and
it's easier to see to high level structure if you format the check like this;

	if (!before(tcp_wnd_end(tp), tp->snd_nxt) ||
	    (tp->rx_opt.wscale_ok &&
	     ((tp->snd_nxt - tcp_wnd_end(tp)) < (1 << tp->rx_opt.rcv_wscale))))

THanks.