netdev - Re: [PATCH stable-5.4 4/4] tcp: avoid too many retransmit packets

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

Message-ID: <CANn89iLE9Out6wYhLGeRmq7fFycHD1D1NtfqY-hkZmL9v+Ab0A@mail.gmail.com>
Date: Tue, 16 Jul 2024 07:21:17 -0700
From: Eric Dumazet <edumazet@...gle.com>
To: Greg KH <gregkh@...uxfoundation.org>
Cc: Jason Xing <kerneljasonxing@...il.com>, Miguel Ojeda <ojeda@...nel.org>, davem@...emloft.net, 
	eric.dumazet@...il.com, jmaxwell37@...il.com, kuba@...nel.org, 
	kuniyu@...zon.com, ncardwell@...gle.com, netdev@...r.kernel.org, 
	pabeni@...hat.com
Subject: Re: [PATCH stable-5.4 4/4] tcp: avoid too many retransmit packets

On Tue, Jul 16, 2024 at 6:03 AM Greg KH <gregkh@...uxfoundation.org> wrote:
>
> On Tue, Jul 16, 2024 at 02:56:28PM +0200, Greg KH wrote:
> > On Tue, Jul 16, 2024 at 02:53:12PM +0200, Greg KH wrote:
> > > On Tue, Jul 16, 2024 at 08:40:40PM +0800, Jason Xing wrote:
> > > > On Tue, Jul 16, 2024 at 7:10 PM Miguel Ojeda <ojeda@...nel.org> wrote:
> > > > >
> > > > > Hi Greg, Eric, all,
> > > > >
> > > > > I noticed this in stable-rc/queue and stable-rc/linux- for 6.1 and 6.6:
> > > > >
> > > > >     net/ipv4/tcp_timer.c:472:7: error: variable 'rtx_delta' is uninitialized when used here [-Werror,-Wuninitialized]
> > > > >                     if (rtx_delta > user_timeout)
> > > > >                         ^~~~~~~~~
> > > > >     net/ipv4/tcp_timer.c:464:15: note: initialize the variable 'rtx_delta' to silence this warning
> > > > >             u32 rtx_delta;
> > > > >                         ^
> > > > >                         = 0
> > > > >
> > > > > I hope that helps!
> > > >
> > > > Thanks for the report!
> > > >
> > > > I think it missed one small snippet of code from [1] compared to the
> > > > latest kernel. We can init this part before using it, something like
> > > > this:
> > > >
> > > > +       rtx_delta = (u32)msecs_to_jiffies(tcp_time_stamp(tp) -
> > > > +                       (tp->retrans_stamp ?: tcp_skb_timestamp(skb)));
> > > >
> > > > Note: fully untested.
> > > >
> > > > Since Eric is very busy, I decided to check and provide some useful
> > > > information here.
> > >
> > > Thanks all, this was probably due to my manual backporting here, let me
> > > go check what went wrong...
> >
> > Yeah, this is my fault, due to 614e8316aa4c ("tcp: add support for usec
> > resolution in TCP TS values") not being in the tree, let me go rework
> > things...
>
> Ok, backporting that commit is not going to happen, that's crazy...

Absolutely right, this is not stable material.

>
> Anyway, the diff below is what I made on top of the existing one, which
> should be doing the right thing.  But ideally someone can test this,
> somehow...  I'll push out -rc releases later today so that people can
> pound on it easier.
>
> thanks for the review!
>
> greg k-h
>
>
> --- a/net/ipv4/tcp_timer.c
> +++ b/net/ipv4/tcp_timer.c
> @@ -464,6 +464,9 @@ static bool tcp_rtx_probe0_timed_out(con
>         u32 rtx_delta;
>         s32 rcv_delta;
>
> +       rtx_delta = (u32)msecs_to_jiffies(tcp_time_stamp(tp) -
> +                       (tp->retrans_stamp ?: tcp_skb_timestamp(skb)));
> +
>         if (user_timeout) {
>                 /* If user application specified a TCP_USER_TIMEOUT,
>                  * it does not want win 0 packets to 'reset the timer'
> @@ -482,9 +485,6 @@ static bool tcp_rtx_probe0_timed_out(con
>         if (rcv_delta <= timeout)
>                 return false;
>
> -       rtx_delta = (u32)msecs_to_jiffies(tcp_time_stamp(tp) -
> -                       (tp->retrans_stamp ?: tcp_skb_timestamp(skb)));
> -
>         return rtx_delta > timeout;
>  }
>

Hi Greg, this looks great to me, thanks for taking care of this.