netdev - Re: [PATCH net] tcp: prevent bogus FRTO undos with non-SACK flows

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: <20180701.192435.353563332676493794.davem@davemloft.net>
Date:   Sun, 01 Jul 2018 19:24:35 +0900 (KST)
From:   David Miller <davem@...emloft.net>
To:     ilpo.jarvinen@...sinki.fi
Cc:     netdev@...r.kernel.org, ycheng@...gle.com, edumazet@...gle.com,
        ncardwell@...gle.com, mkubecek@...e.cz
Subject: Re: [PATCH net] tcp: prevent bogus FRTO undos with non-SACK flows

From: Ilpo Järvinen <ilpo.jarvinen@...sinki.fi>
Date: Fri, 29 Jun 2018 13:07:53 +0300 (EEST)

> If SACK is not enabled and the first cumulative ACK after the RTO
> retransmission covers more than the retransmitted skb, a spurious
> FRTO undo will trigger (assuming FRTO is enabled for that RTO).
> The reason is that any non-retransmitted segment acknowledged will
> set FLAG_ORIG_SACK_ACKED in tcp_clean_rtx_queue even if there is
> no indication that it would have been delivered for real (the
> scoreboard is not kept with TCPCB_SACKED_ACKED bits in the non-SACK
> case so the check for that bit won't help like it does with SACK).
> Having FLAG_ORIG_SACK_ACKED set results in the spurious FRTO undo
> in tcp_process_loss.
> 
> We need to use more strict condition for non-SACK case and check
> that none of the cumulatively ACKed segments were retransmitted
> to prove that progress is due to original transmissions. Only then
> keep FLAG_ORIG_SACK_ACKED set, allowing FRTO undo to proceed in
> non-SACK case.
> 
> (FLAG_ORIG_SACK_ACKED is planned to be renamed to FLAG_ORIG_PROGRESS
> to better indicate its purpose but to keep this change minimal, it
> will be done in another patch).
> 
> Besides burstiness and congestion control violations, this problem
> can result in RTO loop: When the loss recovery is prematurely
> undoed, only new data will be transmitted (if available) and
> the next retransmission can occur only after a new RTO which in case
> of multiple losses (that are not for consecutive packets) requires
> one RTO per loss to recover.
> 
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@...sinki.fi>

Applied and queued up for -stable, thanks!