* [PATCH stable-5.4 0/4] tcp: stable backports for CVE-2024-41007
@ 2024-07-16 1:53 Eric Dumazet
2024-07-16 1:53 ` [PATCH stable-5.4 1/4] tcp: refactor tcp_retransmit_timer() Eric Dumazet
` (3 more replies)
0 siblings, 4 replies; 11+ messages in thread
From: Eric Dumazet @ 2024-07-16 1:53 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: David S . Miller, Jakub Kicinski, Paolo Abeni, netdev,
eric.dumazet, Neal Cardwell, Jason Xing, Jon Maxwell,
Kuniyuki Iwashima, Eric Dumazet
For linux-5.4 stable, we have to bring four patches.
(Compared to linux-5.10 where only the last two patches are needed.)
Eric Dumazet (3):
tcp: refactor tcp_retransmit_timer()
tcp: use signed arithmetic in tcp_rtx_probe0_timed_out()
tcp: avoid too many retransmit packets
Menglong Dong (1):
net: tcp: fix unexcepted socket die when snd_wnd is 0
net/ipv4/tcp_timer.c | 45 +++++++++++++++++++++++++++++++++++++++++---
1 file changed, 42 insertions(+), 3 deletions(-)
--
2.45.2.993.g49e7a77208-goog
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH stable-5.4 1/4] tcp: refactor tcp_retransmit_timer()
2024-07-16 1:53 [PATCH stable-5.4 0/4] tcp: stable backports for CVE-2024-41007 Eric Dumazet
@ 2024-07-16 1:53 ` Eric Dumazet
2024-07-16 1:53 ` [PATCH stable-5.4 2/4] net: tcp: fix unexcepted socket die when snd_wnd is 0 Eric Dumazet
` (2 subsequent siblings)
3 siblings, 0 replies; 11+ messages in thread
From: Eric Dumazet @ 2024-07-16 1:53 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: David S . Miller, Jakub Kicinski, Paolo Abeni, netdev,
eric.dumazet, Neal Cardwell, Jason Xing, Jon Maxwell,
Kuniyuki Iwashima, Eric Dumazet, Willem de Bruijn,
Soheil Hassas Yeganeh
commit 0d580fbd2db084a5c96ee9c00492236a279d5e0f upstream.
It appears linux-4.14 stable needs a backport of commit
88f8598d0a30 ("tcp: exit if nothing to retransmit on RTO timeout")
Since tcp_rtx_queue_empty() is not in pre 4.15 kernels,
let's refactor tcp_retransmit_timer() to only use tcp_rtx_queue_head()
I will provide to stable teams the squashed patches.
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Willem de Bruijn <willemb@google.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Acked-by: Soheil Hassas Yeganeh <soheil@google.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
---
net/ipv4/tcp_timer.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/net/ipv4/tcp_timer.c b/net/ipv4/tcp_timer.c
index e20fced3c9cf691c83b494e11e551161e17d6619..e0bd7999d6d51602fcd868a72d00618234bd0c27 100644
--- a/net/ipv4/tcp_timer.c
+++ b/net/ipv4/tcp_timer.c
@@ -452,6 +452,7 @@ void tcp_retransmit_timer(struct sock *sk)
struct net *net = sock_net(sk);
struct inet_connection_sock *icsk = inet_csk(sk);
struct request_sock *req;
+ struct sk_buff *skb;
req = rcu_dereference_protected(tp->fastopen_rsk,
lockdep_sock_is_held(sk));
@@ -464,7 +465,12 @@ void tcp_retransmit_timer(struct sock *sk)
*/
return;
}
- if (!tp->packets_out || WARN_ON_ONCE(tcp_rtx_queue_empty(sk)))
+
+ if (!tp->packets_out)
+ return;
+
+ skb = tcp_rtx_queue_head(sk);
+ if (WARN_ON_ONCE(!skb))
return;
if (!tp->snd_wnd && !sock_flag(sk, SOCK_DEAD) &&
@@ -496,7 +502,7 @@ void tcp_retransmit_timer(struct sock *sk)
goto out;
}
tcp_enter_loss(sk);
- tcp_retransmit_skb(sk, tcp_rtx_queue_head(sk), 1);
+ tcp_retransmit_skb(sk, skb, 1);
__sk_dst_reset(sk);
goto out_reset_timer;
}
--
2.45.2.993.g49e7a77208-goog
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH stable-5.4 2/4] net: tcp: fix unexcepted socket die when snd_wnd is 0
2024-07-16 1:53 [PATCH stable-5.4 0/4] tcp: stable backports for CVE-2024-41007 Eric Dumazet
2024-07-16 1:53 ` [PATCH stable-5.4 1/4] tcp: refactor tcp_retransmit_timer() Eric Dumazet
@ 2024-07-16 1:53 ` Eric Dumazet
2024-07-16 1:53 ` [PATCH stable-5.4 3/4] tcp: use signed arithmetic in tcp_rtx_probe0_timed_out() Eric Dumazet
2024-07-16 1:54 ` [PATCH stable-5.4 4/4] tcp: avoid too many retransmit packets Eric Dumazet
3 siblings, 0 replies; 11+ messages in thread
From: Eric Dumazet @ 2024-07-16 1:53 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: David S . Miller, Jakub Kicinski, Paolo Abeni, netdev,
eric.dumazet, Neal Cardwell, Jason Xing, Jon Maxwell,
Kuniyuki Iwashima, Menglong Dong, Eric Dumazet
From: Menglong Dong <imagedong@tencent.com>
commit e89688e3e97868451a5d05b38a9d2633d6785cd4 upstream.
In tcp_retransmit_timer(), a window shrunk connection will be regarded
as timeout if 'tcp_jiffies32 - tp->rcv_tstamp > TCP_RTO_MAX'. This is not
right all the time.
The retransmits will become zero-window probes in tcp_retransmit_timer()
if the 'snd_wnd==0'. Therefore, the icsk->icsk_rto will come up to
TCP_RTO_MAX sooner or later.
However, the timer can be delayed and be triggered after 122877ms, not
TCP_RTO_MAX, as I tested.
Therefore, 'tcp_jiffies32 - tp->rcv_tstamp > TCP_RTO_MAX' is always true
once the RTO come up to TCP_RTO_MAX, and the socket will die.
Fix this by replacing the 'tcp_jiffies32' with '(u32)icsk->icsk_timeout',
which is exact the timestamp of the timeout.
However, "tp->rcv_tstamp" can restart from idle, then tp->rcv_tstamp
could already be a long time (minutes or hours) in the past even on the
first RTO. So we double check the timeout with the duration of the
retransmission.
Meanwhile, making "2 * TCP_RTO_MAX" as the timeout to avoid the socket
dying too soon.
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Link: https://lore.kernel.org/netdev/CADxym3YyMiO+zMD4zj03YPM3FBi-1LHi6gSD2XT8pyAMM096pg@mail.gmail.com/
Signed-off-by: Menglong Dong <imagedong@tencent.com>
Reviewed-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
---
net/ipv4/tcp_timer.c | 18 +++++++++++++++++-
1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/net/ipv4/tcp_timer.c b/net/ipv4/tcp_timer.c
index e0bd7999d6d51602fcd868a72d00618234bd0c27..76bd619a89848a2a673b49fdb034037b454ced18 100644
--- a/net/ipv4/tcp_timer.c
+++ b/net/ipv4/tcp_timer.c
@@ -434,6 +434,22 @@ static void tcp_fastopen_synack_timer(struct sock *sk, struct request_sock *req)
TCP_TIMEOUT_INIT << req->num_timeout, TCP_RTO_MAX);
}
+static bool tcp_rtx_probe0_timed_out(const struct sock *sk,
+ const struct sk_buff *skb)
+{
+ const struct tcp_sock *tp = tcp_sk(sk);
+ const int timeout = TCP_RTO_MAX * 2;
+ u32 rcv_delta, rtx_delta;
+
+ rcv_delta = inet_csk(sk)->icsk_timeout - tp->rcv_tstamp;
+ 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;
+}
/**
* tcp_retransmit_timer() - The TCP retransmit timeout handler
@@ -497,7 +513,7 @@ void tcp_retransmit_timer(struct sock *sk)
tp->snd_una, tp->snd_nxt);
}
#endif
- if (tcp_jiffies32 - tp->rcv_tstamp > TCP_RTO_MAX) {
+ if (tcp_rtx_probe0_timed_out(sk, skb)) {
tcp_write_err(sk);
goto out;
}
--
2.45.2.993.g49e7a77208-goog
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH stable-5.4 3/4] tcp: use signed arithmetic in tcp_rtx_probe0_timed_out()
2024-07-16 1:53 [PATCH stable-5.4 0/4] tcp: stable backports for CVE-2024-41007 Eric Dumazet
2024-07-16 1:53 ` [PATCH stable-5.4 1/4] tcp: refactor tcp_retransmit_timer() Eric Dumazet
2024-07-16 1:53 ` [PATCH stable-5.4 2/4] net: tcp: fix unexcepted socket die when snd_wnd is 0 Eric Dumazet
@ 2024-07-16 1:53 ` Eric Dumazet
2024-07-16 1:54 ` [PATCH stable-5.4 4/4] tcp: avoid too many retransmit packets Eric Dumazet
3 siblings, 0 replies; 11+ messages in thread
From: Eric Dumazet @ 2024-07-16 1:53 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: David S . Miller, Jakub Kicinski, Paolo Abeni, netdev,
eric.dumazet, Neal Cardwell, Jason Xing, Jon Maxwell,
Kuniyuki Iwashima, Eric Dumazet, Menglong Dong
commit 36534d3c54537bf098224a32dc31397793d4594d upstream.
Due to timer wheel implementation, a timer will usually fire
after its schedule.
For instance, for HZ=1000, a timeout between 512ms and 4s
has a granularity of 64ms.
For this range of values, the extra delay could be up to 63ms.
For TCP, this means that tp->rcv_tstamp may be after
inet_csk(sk)->icsk_timeout whenever the timer interrupt
finally triggers, if one packet came during the extra delay.
We need to make sure tcp_rtx_probe0_timed_out() handles this case.
Fixes: e89688e3e978 ("net: tcp: fix unexcepted socket die when snd_wnd is 0")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Menglong Dong <imagedong@tencent.com>
Acked-by: Neal Cardwell <ncardwell@google.com>
Reviewed-by: Jason Xing <kerneljasonxing@gmail.com>
Link: https://lore.kernel.org/r/20240607125652.1472540-1-edumazet@google.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
net/ipv4/tcp_timer.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/net/ipv4/tcp_timer.c b/net/ipv4/tcp_timer.c
index 76bd619a89848a2a673b49fdb034037b454ced18..cbd4fde47c1f8d29533bf5ce28bddf4c9a00efe7 100644
--- a/net/ipv4/tcp_timer.c
+++ b/net/ipv4/tcp_timer.c
@@ -439,8 +439,13 @@ static bool tcp_rtx_probe0_timed_out(const struct sock *sk,
{
const struct tcp_sock *tp = tcp_sk(sk);
const int timeout = TCP_RTO_MAX * 2;
- u32 rcv_delta, rtx_delta;
+ u32 rtx_delta;
+ s32 rcv_delta;
+ /* Note: timer interrupt might have been delayed by at least one jiffy,
+ * and tp->rcv_tstamp might very well have been written recently.
+ * rcv_delta can thus be negative.
+ */
rcv_delta = inet_csk(sk)->icsk_timeout - tp->rcv_tstamp;
if (rcv_delta <= timeout)
return false;
--
2.45.2.993.g49e7a77208-goog
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH stable-5.4 4/4] tcp: avoid too many retransmit packets
2024-07-16 1:53 [PATCH stable-5.4 0/4] tcp: stable backports for CVE-2024-41007 Eric Dumazet
` (2 preceding siblings ...)
2024-07-16 1:53 ` [PATCH stable-5.4 3/4] tcp: use signed arithmetic in tcp_rtx_probe0_timed_out() Eric Dumazet
@ 2024-07-16 1:54 ` Eric Dumazet
2024-07-16 11:10 ` Miguel Ojeda
3 siblings, 1 reply; 11+ messages in thread
From: Eric Dumazet @ 2024-07-16 1:54 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: David S . Miller, Jakub Kicinski, Paolo Abeni, netdev,
eric.dumazet, Neal Cardwell, Jason Xing, Jon Maxwell,
Kuniyuki Iwashima, Eric Dumazet
commit 97a9063518f198ec0adb2ecb89789de342bb8283 upstream.
If a TCP socket is using TCP_USER_TIMEOUT, and the other peer
retracted its window to zero, tcp_retransmit_timer() can
retransmit a packet every two jiffies (2 ms for HZ=1000),
for about 4 minutes after TCP_USER_TIMEOUT has 'expired'.
The fix is to make sure tcp_rtx_probe0_timed_out() takes
icsk->icsk_user_timeout into account.
Before blamed commit, the socket would not timeout after
icsk->icsk_user_timeout, but would use standard exponential
backoff for the retransmits.
Also worth noting that before commit e89688e3e978 ("net: tcp:
fix unexcepted socket die when snd_wnd is 0"), the issue
would last 2 minutes instead of 4.
Fixes: b701a99e431d ("tcp: Add tcp_clamp_rto_to_user_timeout() helper to improve accuracy")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Neal Cardwell <ncardwell@google.com>
Reviewed-by: Jason Xing <kerneljasonxing@gmail.com>
Reviewed-by: Jon Maxwell <jmaxwell37@gmail.com>
Reviewed-by: Kuniyuki Iwashima <kuniyu@amazon.com>
Link: https://patch.msgid.link/20240710001402.2758273-1-edumazet@google.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
net/ipv4/tcp_timer.c | 16 ++++++++++++++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/net/ipv4/tcp_timer.c b/net/ipv4/tcp_timer.c
index cbd4fde47c1f8d29533bf5ce28bddf4c9a00efe7..a386e9b84984ab0be41b0c38ea015e4ae3377edf 100644
--- a/net/ipv4/tcp_timer.c
+++ b/net/ipv4/tcp_timer.c
@@ -437,16 +437,28 @@ static void tcp_fastopen_synack_timer(struct sock *sk, struct request_sock *req)
static bool tcp_rtx_probe0_timed_out(const struct sock *sk,
const struct sk_buff *skb)
{
+ const struct inet_connection_sock *icsk = inet_csk(sk);
+ u32 user_timeout = READ_ONCE(icsk->icsk_user_timeout);
const struct tcp_sock *tp = tcp_sk(sk);
- const int timeout = TCP_RTO_MAX * 2;
+ int timeout = TCP_RTO_MAX * 2;
u32 rtx_delta;
s32 rcv_delta;
+ if (user_timeout) {
+ /* If user application specified a TCP_USER_TIMEOUT,
+ * it does not want win 0 packets to 'reset the timer'
+ * while retransmits are not making progress.
+ */
+ if (rtx_delta > user_timeout)
+ return true;
+ timeout = min_t(u32, timeout, msecs_to_jiffies(user_timeout));
+ }
+
/* Note: timer interrupt might have been delayed by at least one jiffy,
* and tp->rcv_tstamp might very well have been written recently.
* rcv_delta can thus be negative.
*/
- rcv_delta = inet_csk(sk)->icsk_timeout - tp->rcv_tstamp;
+ rcv_delta = icsk->icsk_timeout - tp->rcv_tstamp;
if (rcv_delta <= timeout)
return false;
--
2.45.2.993.g49e7a77208-goog
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH stable-5.4 4/4] tcp: avoid too many retransmit packets
2024-07-16 1:54 ` [PATCH stable-5.4 4/4] tcp: avoid too many retransmit packets Eric Dumazet
@ 2024-07-16 11:10 ` Miguel Ojeda
2024-07-16 12:40 ` Jason Xing
0 siblings, 1 reply; 11+ messages in thread
From: Miguel Ojeda @ 2024-07-16 11:10 UTC (permalink / raw)
To: gregkh, edumazet
Cc: davem, eric.dumazet, jmaxwell37, kerneljasonxing, kuba, kuniyu,
ncardwell, netdev, pabeni
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!
Cheers,
Miguel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH stable-5.4 4/4] tcp: avoid too many retransmit packets
2024-07-16 11:10 ` Miguel Ojeda
@ 2024-07-16 12:40 ` Jason Xing
2024-07-16 12:53 ` Greg KH
0 siblings, 1 reply; 11+ messages in thread
From: Jason Xing @ 2024-07-16 12:40 UTC (permalink / raw)
To: Miguel Ojeda
Cc: gregkh, edumazet, davem, eric.dumazet, jmaxwell37, kuba, kuniyu,
ncardwell, netdev, pabeni
On Tue, Jul 16, 2024 at 7:10 PM Miguel Ojeda <ojeda@kernel.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.
[1]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=614e8316aa4cafba3e204cb8ee48bd12b92f3d93
>
> Cheers,
> Miguel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH stable-5.4 4/4] tcp: avoid too many retransmit packets
2024-07-16 12:40 ` Jason Xing
@ 2024-07-16 12:53 ` Greg KH
2024-07-16 12:56 ` Greg KH
0 siblings, 1 reply; 11+ messages in thread
From: Greg KH @ 2024-07-16 12:53 UTC (permalink / raw)
To: Jason Xing
Cc: Miguel Ojeda, edumazet, davem, eric.dumazet, jmaxwell37, kuba,
kuniyu, ncardwell, netdev, pabeni
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@kernel.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...
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH stable-5.4 4/4] tcp: avoid too many retransmit packets
2024-07-16 12:53 ` Greg KH
@ 2024-07-16 12:56 ` Greg KH
2024-07-16 13:03 ` Greg KH
0 siblings, 1 reply; 11+ messages in thread
From: Greg KH @ 2024-07-16 12:56 UTC (permalink / raw)
To: Jason Xing
Cc: Miguel Ojeda, edumazet, davem, eric.dumazet, jmaxwell37, kuba,
kuniyu, ncardwell, netdev, pabeni
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@kernel.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...
thanks,
greg k-h
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH stable-5.4 4/4] tcp: avoid too many retransmit packets
2024-07-16 12:56 ` Greg KH
@ 2024-07-16 13:03 ` Greg KH
2024-07-16 14:21 ` Eric Dumazet
0 siblings, 1 reply; 11+ messages in thread
From: Greg KH @ 2024-07-16 13:03 UTC (permalink / raw)
To: Jason Xing
Cc: Miguel Ojeda, edumazet, davem, eric.dumazet, jmaxwell37, kuba,
kuniyu, ncardwell, netdev, pabeni
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@kernel.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...
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;
}
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH stable-5.4 4/4] tcp: avoid too many retransmit packets
2024-07-16 13:03 ` Greg KH
@ 2024-07-16 14:21 ` Eric Dumazet
0 siblings, 0 replies; 11+ messages in thread
From: Eric Dumazet @ 2024-07-16 14:21 UTC (permalink / raw)
To: Greg KH
Cc: Jason Xing, Miguel Ojeda, davem, eric.dumazet, jmaxwell37, kuba,
kuniyu, ncardwell, netdev, pabeni
On Tue, Jul 16, 2024 at 6:03 AM Greg KH <gregkh@linuxfoundation.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@kernel.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.
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2024-07-16 14:21 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-07-16 1:53 [PATCH stable-5.4 0/4] tcp: stable backports for CVE-2024-41007 Eric Dumazet
2024-07-16 1:53 ` [PATCH stable-5.4 1/4] tcp: refactor tcp_retransmit_timer() Eric Dumazet
2024-07-16 1:53 ` [PATCH stable-5.4 2/4] net: tcp: fix unexcepted socket die when snd_wnd is 0 Eric Dumazet
2024-07-16 1:53 ` [PATCH stable-5.4 3/4] tcp: use signed arithmetic in tcp_rtx_probe0_timed_out() Eric Dumazet
2024-07-16 1:54 ` [PATCH stable-5.4 4/4] tcp: avoid too many retransmit packets Eric Dumazet
2024-07-16 11:10 ` Miguel Ojeda
2024-07-16 12:40 ` Jason Xing
2024-07-16 12:53 ` Greg KH
2024-07-16 12:56 ` Greg KH
2024-07-16 13:03 ` Greg KH
2024-07-16 14:21 ` Eric Dumazet
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).