All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.