Archive-only list for patches
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: stable@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	patches@lists.linux.dev, Neal Cardwell <ncardwell@google.com>,
	Eric Dumazet <edumazet@google.com>,
	Yuchung Cheng <ycheng@google.com>, Kevin Yang <yyd@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Sasha Levin <sashal@kernel.org>
Subject: [PATCH 4.19 43/66] tcp: fix incorrect undo caused by DSACK of TLP retransmit
Date: Tue, 16 Jul 2024 17:31:18 +0200	[thread overview]
Message-ID: <20240716152739.812069497@linuxfoundation.org> (raw)
In-Reply-To: <20240716152738.161055634@linuxfoundation.org>

4.19-stable review patch.  If anyone has any objections, please let me know.

------------------

From: Neal Cardwell <ncardwell@google.com>

[ Upstream commit 0ec986ed7bab6801faed1440e8839dcc710331ff ]

Loss recovery undo_retrans bookkeeping had a long-standing bug where a
DSACK from a spurious TLP retransmit packet could cause an erroneous
undo of a fast recovery or RTO recovery that repaired a single
really-lost packet (in a sequence range outside that of the TLP
retransmit). Basically, because the loss recovery state machine didn't
account for the fact that it sent a TLP retransmit, the DSACK for the
TLP retransmit could erroneously be implicitly be interpreted as
corresponding to the normal fast recovery or RTO recovery retransmit
that plugged a real hole, thus resulting in an improper undo.

For example, consider the following buggy scenario where there is a
real packet loss but the congestion control response is improperly
undone because of this bug:

+ send packets P1, P2, P3, P4
+ P1 is really lost
+ send TLP retransmit of P4
+ receive SACK for original P2, P3, P4
+ enter fast recovery, fast-retransmit P1, increment undo_retrans to 1
+ receive DSACK for TLP P4, decrement undo_retrans to 0, undo (bug!)
+ receive cumulative ACK for P1-P4 (fast retransmit plugged real hole)

The fix: when we initialize undo machinery in tcp_init_undo(), if
there is a TLP retransmit in flight, then increment tp->undo_retrans
so that we make sure that we receive a DSACK corresponding to the TLP
retransmit, as well as DSACKs for all later normal retransmits, before
triggering a loss recovery undo. Note that we also have to move the
line that clears tp->tlp_high_seq for RTO recovery, so that upon RTO
we remember the tp->tlp_high_seq value until tcp_init_undo() and clear
it only afterward.

Also note that the bug dates back to the original 2013 TLP
implementation, commit 6ba8a3b19e76 ("tcp: Tail loss probe (TLP)").

However, this patch will only compile and work correctly with kernels
that have tp->tlp_retrans, which was added only in v5.8 in 2020 in
commit 76be93fc0702 ("tcp: allow at most one TLP probe per flight").
So we associate this fix with that later commit.

Fixes: 76be93fc0702 ("tcp: allow at most one TLP probe per flight")
Signed-off-by: Neal Cardwell <ncardwell@google.com>
Reviewed-by: Eric Dumazet <edumazet@google.com>
Cc: Yuchung Cheng <ycheng@google.com>
Cc: Kevin Yang <yyd@google.com>
Link: https://patch.msgid.link/20240703171246.1739561-1-ncardwell.sw@gmail.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 net/ipv4/tcp_input.c | 11 ++++++++++-
 net/ipv4/tcp_timer.c |  2 --
 2 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 9a66c37958451..9254705afa869 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -1948,8 +1948,16 @@ void tcp_clear_retrans(struct tcp_sock *tp)
 static inline void tcp_init_undo(struct tcp_sock *tp)
 {
 	tp->undo_marker = tp->snd_una;
+
 	/* Retransmission still in flight may cause DSACKs later. */
-	tp->undo_retrans = tp->retrans_out ? : -1;
+	/* First, account for regular retransmits in flight: */
+	tp->undo_retrans = tp->retrans_out;
+	/* Next, account for TLP retransmits in flight: */
+	if (tp->tlp_high_seq && tp->tlp_retrans)
+		tp->undo_retrans++;
+	/* Finally, avoid 0, because undo_retrans==0 means "can undo now": */
+	if (!tp->undo_retrans)
+		tp->undo_retrans = -1;
 }
 
 static bool tcp_is_rack(const struct sock *sk)
@@ -2028,6 +2036,7 @@ void tcp_enter_loss(struct sock *sk)
 
 	tcp_set_ca_state(sk, TCP_CA_Loss);
 	tp->high_seq = tp->snd_nxt;
+	tp->tlp_high_seq = 0;
 	tcp_ecn_queue_cwr(tp);
 
 	/* F-RTO RFC5682 sec 3.1 step 1: retransmit SND.UNA if no previous
diff --git a/net/ipv4/tcp_timer.c b/net/ipv4/tcp_timer.c
index d8d28ba169b4d..cebbac092f322 100644
--- a/net/ipv4/tcp_timer.c
+++ b/net/ipv4/tcp_timer.c
@@ -441,8 +441,6 @@ void tcp_retransmit_timer(struct sock *sk)
 	if (!tp->packets_out || WARN_ON_ONCE(tcp_rtx_queue_empty(sk)))
 		return;
 
-	tp->tlp_high_seq = 0;
-
 	if (!tp->snd_wnd && !sock_flag(sk, SOCK_DEAD) &&
 	    !((1 << sk->sk_state) & (TCPF_SYN_SENT | TCPF_SYN_RECV))) {
 		/* Receiver dastardly shrinks window. Our retransmits
-- 
2.43.0




  parent reply	other threads:[~2024-07-16 15:35 UTC|newest]

Thread overview: 74+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-07-16 15:30 [PATCH 4.19 00/66] 4.19.318-rc1 review Greg Kroah-Hartman
2024-07-16 15:30 ` [PATCH 4.19 01/66] media: dvb: as102-fe: Fix as10x_register_addr packing Greg Kroah-Hartman
2024-07-16 15:30 ` [PATCH 4.19 02/66] media: dvb-usb: dib0700_devices: Add missing release_firmware() Greg Kroah-Hartman
2024-07-16 15:30 ` [PATCH 4.19 03/66] IB/core: Implement a limit on UMAD receive List Greg Kroah-Hartman
2024-07-16 15:30 ` [PATCH 4.19 04/66] drm/amd/display: Skip finding free audio for unknown engine_id Greg Kroah-Hartman
2024-07-16 15:30 ` [PATCH 4.19 05/66] media: dw2102: Dont translate i2c read into write Greg Kroah-Hartman
2024-07-16 15:30 ` [PATCH 4.19 06/66] sctp: prefer struct_size over open coded arithmetic Greg Kroah-Hartman
2024-07-16 15:30 ` [PATCH 4.19 07/66] firmware: dmi: Stop decoding on broken entry Greg Kroah-Hartman
2024-07-16 15:30 ` [PATCH 4.19 08/66] Input: ff-core - prefer struct_size over open coded arithmetic Greg Kroah-Hartman
2024-07-16 15:30 ` [PATCH 4.19 09/66] net: dsa: mv88e6xxx: Correct check for empty list Greg Kroah-Hartman
2024-07-16 15:30 ` [PATCH 4.19 10/66] media: dvb-frontends: tda18271c2dd: Remove casting during div Greg Kroah-Hartman
2024-07-16 15:30 ` [PATCH 4.19 11/66] media: s2255: Use refcount_t instead of atomic_t for num_channels Greg Kroah-Hartman
2024-07-16 15:30 ` [PATCH 4.19 12/66] media: dvb-frontends: tda10048: Fix integer overflow Greg Kroah-Hartman
2024-07-16 15:30 ` [PATCH 4.19 13/66] i2c: i801: Annotate apanel_addr as __ro_after_init Greg Kroah-Hartman
2024-07-16 15:30 ` [PATCH 4.19 14/66] powerpc/64: Set _IO_BASE to POISON_POINTER_DELTA not 0 for CONFIG_PCI=n Greg Kroah-Hartman
2024-07-16 15:30 ` [PATCH 4.19 15/66] orangefs: fix out-of-bounds fsid access Greg Kroah-Hartman
2024-07-16 15:30 ` [PATCH 4.19 16/66] powerpc/xmon: Check cpu id in commands "c#", "dp#" and "dx#" Greg Kroah-Hartman
2024-07-16 15:30 ` [PATCH 4.19 17/66] jffs2: Fix potential illegal address access in jffs2_free_inode Greg Kroah-Hartman
2024-07-16 15:30 ` [PATCH 4.19 18/66] s390/pkey: Wipe sensitive data on failure Greg Kroah-Hartman
2024-07-16 15:30 ` [PATCH 4.19 19/66] tcp: take care of compressed acks in tcp_add_reno_sack() Greg Kroah-Hartman
2024-07-16 15:30 ` [PATCH 4.19 20/66] tcp: tcp_mark_head_lost is only valid for sack-tcp Greg Kroah-Hartman
2024-07-16 15:30 ` [PATCH 4.19 21/66] tcp: add ece_ack flag to reno sack functions Greg Kroah-Hartman
2024-07-16 15:30 ` [PATCH 4.19 22/66] net: tcp better handling of reordering then loss cases Greg Kroah-Hartman
2024-07-16 15:30 ` [PATCH 4.19 23/66] UPSTREAM: tcp: fix DSACK undo in fast recovery to call tcp_try_to_open() Greg Kroah-Hartman
2024-07-16 15:30 ` [PATCH 4.19 24/66] tcp_metrics: validate source addr length Greg Kroah-Hartman
2024-07-16 15:31 ` [PATCH 4.19 25/66] bonding: Fix out-of-bounds read in bond_option_arp_ip_targets_set() Greg Kroah-Hartman
2024-07-16 15:31 ` [PATCH 4.19 26/66] selftests: fix OOM in msg_zerocopy selftest Greg Kroah-Hartman
2024-07-16 15:31 ` [PATCH 4.19 27/66] selftests: make order checking verbose " Greg Kroah-Hartman
2024-07-16 15:31 ` [PATCH 4.19 28/66] inet_diag: Initialize pad field in struct inet_diag_req_v2 Greg Kroah-Hartman
2024-07-16 15:31 ` [PATCH 4.19 29/66] nilfs2: fix inode number range checks Greg Kroah-Hartman
2024-07-16 15:31 ` [PATCH 4.19 30/66] nilfs2: add missing check for inode numbers on directory entries Greg Kroah-Hartman
2024-07-16 15:31 ` [PATCH 4.19 31/66] mm: optimize the redundant loop of mm_update_owner_next() Greg Kroah-Hartman
2024-07-16 15:31 ` [PATCH 4.19 32/66] Bluetooth: Fix incorrect pointer arithmatic in ext_adv_report_evt Greg Kroah-Hartman
2024-07-16 15:31 ` [PATCH 4.19 33/66] can: kvaser_usb: Explicitly initialize family in leafimx driver_info struct Greg Kroah-Hartman
2024-07-16 15:31 ` [PATCH 4.19 34/66] fsnotify: Do not generate events for O_PATH file descriptors Greg Kroah-Hartman
2024-07-16 15:31 ` [PATCH 4.19 35/66] Revert "mm/writeback: fix possible divide-by-zero in wb_dirty_limits(), again" Greg Kroah-Hartman
2024-07-16 15:31 ` [PATCH 4.19 36/66] drm/nouveau: fix null pointer dereference in nouveau_connector_get_modes Greg Kroah-Hartman
2024-07-16 15:31 ` [PATCH 4.19 37/66] drm/amdgpu/atomfirmware: silence UBSAN warning Greg Kroah-Hartman
2024-07-16 15:31 ` [PATCH 4.19 38/66] bnx2x: Fix multiple UBSAN array-index-out-of-bounds Greg Kroah-Hartman
2024-07-16 15:31 ` [PATCH 4.19 39/66] media: dw2102: fix a potential buffer overflow Greg Kroah-Hartman
2024-07-16 15:31 ` [PATCH 4.19 40/66] i2c: pnx: Fix potential deadlock warning from del_timer_sync() call in isr Greg Kroah-Hartman
2024-07-16 15:31 ` [PATCH 4.19 41/66] nilfs2: fix incorrect inode allocation from reserved inodes Greg Kroah-Hartman
2024-07-16 15:31 ` [PATCH 4.19 42/66] drm/i915: make find_fw_domain work on intel_uncore Greg Kroah-Hartman
2024-07-16 15:31 ` Greg Kroah-Hartman [this message]
2024-07-16 15:31 ` [PATCH 4.19 44/66] net: lantiq_etop: add blank line after declaration Greg Kroah-Hartman
2024-07-16 15:31 ` [PATCH 4.19 45/66] net: ethernet: lantiq_etop: fix double free in detach Greg Kroah-Hartman
2024-07-16 15:31 ` [PATCH 4.19 46/66] ppp: reject claimed-as-LCP but actually malformed packets Greg Kroah-Hartman
2024-07-16 15:31 ` [PATCH 4.19 47/66] s390: Mark psw in __load_psw_mask() as __unitialized Greg Kroah-Hartman
2024-07-16 15:31 ` [PATCH 4.19 48/66] ARM: davinci: Convert comma to semicolon Greg Kroah-Hartman
2024-07-16 15:31 ` [PATCH 4.19 49/66] USB: serial: option: add Telit generic core-dump composition Greg Kroah-Hartman
2024-07-16 15:31 ` [PATCH 4.19 50/66] USB: serial: option: add Telit FN912 rmnet compositions Greg Kroah-Hartman
2024-07-16 15:31 ` [PATCH 4.19 51/66] USB: serial: option: add Fibocom FM350-GL Greg Kroah-Hartman
2024-07-16 15:31 ` [PATCH 4.19 52/66] USB: serial: option: add support for Foxconn T99W651 Greg Kroah-Hartman
2024-07-16 15:31 ` [PATCH 4.19 53/66] USB: serial: option: add Netprisma LCUK54 series modules Greg Kroah-Hartman
2024-07-16 15:31 ` [PATCH 4.19 54/66] USB: serial: option: add Rolling RW350-GL variants Greg Kroah-Hartman
2024-07-16 15:31 ` [PATCH 4.19 55/66] USB: Add USB_QUIRK_NO_SET_INTF quirk for START BP-850k Greg Kroah-Hartman
2024-07-16 15:31 ` [PATCH 4.19 56/66] usb: gadget: configfs: Prevent OOB read/write in usb_string_copy() Greg Kroah-Hartman
2024-07-16 15:31 ` [PATCH 4.19 57/66] USB: core: Fix duplicate endpoint bug by clearing reserved bits in the descriptor Greg Kroah-Hartman
2024-07-16 15:31 ` [PATCH 4.19 58/66] hpet: Support 32-bit userspace Greg Kroah-Hartman
2024-07-16 15:31 ` [PATCH 4.19 59/66] libceph: fix race between delayed_work() and ceph_monc_stop() Greg Kroah-Hartman
2024-07-16 15:31 ` [PATCH 4.19 60/66] tcp: refactor tcp_retransmit_timer() Greg Kroah-Hartman
2024-07-16 15:31 ` [PATCH 4.19 61/66] net: tcp: fix unexcepted socket die when snd_wnd is 0 Greg Kroah-Hartman
2024-07-16 15:31 ` [PATCH 4.19 62/66] tcp: use signed arithmetic in tcp_rtx_probe0_timed_out() Greg Kroah-Hartman
2024-07-16 15:31 ` [PATCH 4.19 63/66] tcp: avoid too many retransmit packets Greg Kroah-Hartman
2024-07-16 15:31 ` [PATCH 4.19 64/66] SUNRPC: Fix RPC client cleaned up the freed pipefs dentries Greg Kroah-Hartman
2024-07-16 15:31 ` [PATCH 4.19 65/66] nilfs2: fix kernel bug on rename operation of broken directory Greg Kroah-Hartman
2024-07-16 15:31 ` [PATCH 4.19 66/66] i2c: rcar: bring hardware to known state when probing Greg Kroah-Hartman
2024-07-16 20:11 ` [PATCH 4.19 00/66] 4.19.318-rc1 review Pavel Machek
2024-07-16 21:00 ` Naresh Kamboju
2024-07-17  6:21   ` Greg Kroah-Hartman
2024-07-17  8:54 ` Frank Scheiner
2024-07-17  9:30   ` Greg KH
2024-07-17  9:34     ` Frank Scheiner
2024-07-17 15:57 ` Shuah Khan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240716152739.812069497@linuxfoundation.org \
    --to=gregkh@linuxfoundation.org \
    --cc=edumazet@google.com \
    --cc=kuba@kernel.org \
    --cc=ncardwell@google.com \
    --cc=patches@lists.linux.dev \
    --cc=sashal@kernel.org \
    --cc=stable@vger.kernel.org \
    --cc=ycheng@google.com \
    --cc=yyd@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox