netdev - Re: [PATCH net] tcp_bbr: fix u32 wrap bug in round logic if bbr

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: <9dfee38c-1a4a-8c3c-a9ac-20763a9becc7@gmail.com>
Date:   Wed, 11 Aug 2021 09:49:52 +0200
From:   Eric Dumazet <eric.dumazet@...il.com>
To:     Neal Cardwell <ncardwell@...gle.com>,
        David Miller <davem@...emloft.net>
Cc:     netdev@...r.kernel.org, Yuchung Cheng <ycheng@...gle.com>,
        Kevin Yang <yyd@...gle.com>
Subject: Re: [PATCH net] tcp_bbr: fix u32 wrap bug in round logic if
 bbr_init() called after 2B packets



On 8/11/21 4:40 AM, Neal Cardwell wrote:
> Currently if BBR congestion control is initialized after more than 2B
> packets have been delivered, depending on the phase of the
> tp->delivered counter the tracking of BBR round trips can get stuck.
> 
> The bug arises because if tp->delivered is between 2^31 and 2^32 at
> the time the BBR congestion control module is initialized, then the
> initialization of bbr->next_rtt_delivered to 0 will cause the logic to
> believe that the end of the round trip is still billions of packets in
> the future. More specifically, the following check will fail
> repeatedly:
> 
>   !before(rs->prior_delivered, bbr->next_rtt_delivered)
> 
> and thus the connection will take up to 2B packets delivered before
> that check will pass and the connection will set:
> 
>   bbr->round_start = 1;
> 
> This could cause many mechanisms in BBR to fail to trigger, for
> example bbr_check_full_bw_reached() would likely never exit STARTUP.
> 
> This bug is 5 years old and has not been observed, and as a practical
> matter this would likely rarely trigger, since it would require
> transferring at least 2B packets, or likely more than 3 terabytes of
> data, before switching congestion control algorithms to BBR.
> 
> This patch is a stable candidate for kernels as far back as v4.9,
> when tcp_bbr.c was added.
> 
> Fixes: 0f8782ea1497 ("tcp_bbr: add BBR congestion control")
> Signed-off-by: Neal Cardwell <ncardwell@...gle.com>
> Reviewed-by: Yuchung Cheng <ycheng@...gle.com>
> Reviewed-by: Kevin Yang <yyd@...gle.com>

Nice catch :(

Reviewed-by: Eric Dumazet <edumazet@...gle.com>