linux-kernel - Re: [REGRESSION] tcp/ipv4: kernel panic because of (possible) division by zero

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]

Date:	Sun, 10 Jan 2016 19:50:21 +0200
From:	Oleksandr Natalenko <oleksandr@...alenko.name>
To:	Neal Cardwell <ncardwell@...gle.com>
Cc:	Yuchung Cheng <ycheng@...gle.com>, netdev <netdev@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Patrick McHardy <kaber@...sh.net>,
	Hideaki YOSHIFUJI <yoshfuji@...ux-ipv6.org>,
	James Morris <jmorris@...ei.org>,
	Alexey Kuznetsov <kuznet@....inr.ac.ru>,
	"David S. Miller" <davem@...emloft.net>
Subject: Re: [REGRESSION] tcp/ipv4: kernel panic because of (possible) division by zero

Haven't you missed "return ssthresh;" statement?

On неділя, 10 січня 2016 р. 12:29:17 EET Neal Cardwell wrote:
> On Sun, Jan 10, 2016 at 9:57 AM, Oleksandr Natalenko
> 
> <oleksandr@...alenko.name> wrote:
> > I use YeAH. But YeAH code wasn't touched between 4.2 and 4.3.
> 
> Oh, interesting. Looks like tcp_yeah_ssthresh() has a bug where its
> intended reduction can be bigger than tp->snd_cwnd, leading to it
> return a zero ssthresh (or even an ssthresh that underflows to ~4
> billion). If tcp_yeah_ssthresh() returns an ssthresh of 0 then PRR
> will try to pull the cwnd down to 0.
> 
> Can you please leave ECN and Yeah enabled and run something like the
> following patch, to verify this conjecture? If the conjecture is
> right, then the tcp_yeah warning should fire but not the new
> tcp_cwnd_reduction() warning:
> 
> -----------
> diff --git a/net/ipv4/tcp_yeah.c b/net/ipv4/tcp_yeah.c
> index 17d3566..ef60cba 100644
> --- a/net/ipv4/tcp_yeah.c
> +++ b/net/ipv4/tcp_yeah.c
> @@ -206,6 +206,7 @@ static u32 tcp_yeah_ssthresh(struct sock *sk)
>         const struct tcp_sock *tp = tcp_sk(sk);
>         struct yeah *yeah = inet_csk_ca(sk);
>         u32 reduction;
> +       s32 ssthresh;
> 
>         if (yeah->doing_reno_now < TCP_YEAH_RHO) {
>                 reduction = yeah->lastQ;
> @@ -219,7 +220,9 @@ static u32 tcp_yeah_ssthresh(struct sock *sk)
>         yeah->fast_count = 0;
>         yeah->reno_count = max(yeah->reno_count>>1, 2U);
> 
> -       return tp->snd_cwnd - reduction;
> +       ssthresh = tp->snd_cwnd - reduction;
> +       if (WARN_ON_ONCE(ssthresh <= 0))
> +               ssthresh = 1;
>  }
> 
>  static struct tcp_congestion_ops tcp_yeah __read_mostly = {
> -----------
> 
> If that works, then we may just want a version of this patch without
> the warning.
> 
> Thanks!
> neal