All of lore.kernel.org
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: linux-kernel@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	stable@vger.kernel.org, Yuchung Cheng <ycheng@google.com>,
	Eric Dumazet <edumazet@google.com>,
	Neal Cardwell <ncardwell@google.com>,
	"David S. Miller" <davem@davemloft.net>
Subject: [ 36/42] tcp: call tcp_replace_ts_recent() from tcp_ack()
Date: Mon, 29 Apr 2013 12:02:18 -0700	[thread overview]
Message-ID: <20130429184756.467096252@linuxfoundation.org> (raw)
In-Reply-To: <20130429184752.435249613@linuxfoundation.org>

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

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


From: Eric Dumazet <edumazet@google.com>

[ Upstream commit 12fb3dd9dc3c64ba7d64cec977cca9b5fb7b1d4e ]

commit bd090dfc634d (tcp: tcp_replace_ts_recent() should not be called
from tcp_validate_incoming()) introduced a TS ecr bug in slow path
processing.

1 A > B P. 1:10001(10000) ack 1 <nop,nop,TS val 1001 ecr 200>
2 B < A . 1:1(0) ack 1 win 257 <sack 9001:10001,TS val 300 ecr 1001>
3 A > B . 1:1001(1000) ack 1 win 227 <nop,nop,TS val 1002 ecr 200>
4 A > B . 1001:2001(1000) ack 1 win 227 <nop,nop,TS val 1002 ecr 200>

(ecr 200 should be ecr 300 in packets 3 & 4)

Problem is tcp_ack() can trigger send of new packets (retransmits),
reflecting the prior TSval, instead of the TSval contained in the
currently processed incoming packet.

Fix this by calling tcp_replace_ts_recent() from tcp_ack() after the
checks, but before the actions.

Reported-by: Yuchung Cheng <ycheng@google.com>
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Neal Cardwell <ncardwell@google.com>
Acked-by: Neal Cardwell <ncardwell@google.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 net/ipv4/tcp_input.c |   64 ++++++++++++++++++++++++---------------------------
 1 file changed, 31 insertions(+), 33 deletions(-)

--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -116,6 +116,7 @@ int sysctl_tcp_early_retrans __read_most
 #define FLAG_DSACKING_ACK	0x800 /* SACK blocks contained D-SACK info */
 #define FLAG_NONHEAD_RETRANS_ACKED	0x1000 /* Non-head rexmitted data was ACKed */
 #define FLAG_SACK_RENEGING	0x2000 /* snd_una advanced to a sacked seq */
+#define FLAG_UPDATE_TS_RECENT	0x4000 /* tcp_replace_ts_recent() */
 
 #define FLAG_ACKED		(FLAG_DATA_ACKED|FLAG_SYN_ACKED)
 #define FLAG_NOT_DUP		(FLAG_DATA|FLAG_WIN_UPDATE|FLAG_ACKED)
@@ -3572,6 +3573,27 @@ static void tcp_send_challenge_ack(struc
 	}
 }
 
+static void tcp_store_ts_recent(struct tcp_sock *tp)
+{
+	tp->rx_opt.ts_recent = tp->rx_opt.rcv_tsval;
+	tp->rx_opt.ts_recent_stamp = get_seconds();
+}
+
+static void tcp_replace_ts_recent(struct tcp_sock *tp, u32 seq)
+{
+	if (tp->rx_opt.saw_tstamp && !after(seq, tp->rcv_wup)) {
+		/* PAWS bug workaround wrt. ACK frames, the PAWS discard
+		 * extra check below makes sure this can only happen
+		 * for pure ACK frames.  -DaveM
+		 *
+		 * Not only, also it occurs for expired timestamps.
+		 */
+
+		if (tcp_paws_check(&tp->rx_opt, 0))
+			tcp_store_ts_recent(tp);
+	}
+}
+
 /* This routine deals with incoming acks, but not outgoing ones. */
 static int tcp_ack(struct sock *sk, const struct sk_buff *skb, int flag)
 {
@@ -3624,6 +3646,12 @@ static int tcp_ack(struct sock *sk, cons
 	prior_fackets = tp->fackets_out;
 	prior_in_flight = tcp_packets_in_flight(tp);
 
+	/* ts_recent update must be made after we are sure that the packet
+	 * is in window.
+	 */
+	if (flag & FLAG_UPDATE_TS_RECENT)
+		tcp_replace_ts_recent(tp, TCP_SKB_CB(skb)->seq);
+
 	if (!(flag & FLAG_SLOWPATH) && after(ack, prior_snd_una)) {
 		/* Window is constant, pure forward advance.
 		 * No more checks are required.
@@ -3940,27 +3968,6 @@ const u8 *tcp_parse_md5sig_option(const
 EXPORT_SYMBOL(tcp_parse_md5sig_option);
 #endif
 
-static inline void tcp_store_ts_recent(struct tcp_sock *tp)
-{
-	tp->rx_opt.ts_recent = tp->rx_opt.rcv_tsval;
-	tp->rx_opt.ts_recent_stamp = get_seconds();
-}
-
-static inline void tcp_replace_ts_recent(struct tcp_sock *tp, u32 seq)
-{
-	if (tp->rx_opt.saw_tstamp && !after(seq, tp->rcv_wup)) {
-		/* PAWS bug workaround wrt. ACK frames, the PAWS discard
-		 * extra check below makes sure this can only happen
-		 * for pure ACK frames.  -DaveM
-		 *
-		 * Not only, also it occurs for expired timestamps.
-		 */
-
-		if (tcp_paws_check(&tp->rx_opt, 0))
-			tcp_store_ts_recent(tp);
-	}
-}
-
 /* Sorry, PAWS as specified is broken wrt. pure-ACKs -DaveM
  *
  * It is not fatal. If this ACK does _not_ change critical state (seqs, window)
@@ -5556,14 +5563,9 @@ slow_path:
 		return 0;
 
 step5:
-	if (tcp_ack(sk, skb, FLAG_SLOWPATH) < 0)
+	if (tcp_ack(sk, skb, FLAG_SLOWPATH | FLAG_UPDATE_TS_RECENT) < 0)
 		goto discard;
 
-	/* ts_recent update must be made after we are sure that the packet
-	 * is in window.
-	 */
-	tcp_replace_ts_recent(tp, TCP_SKB_CB(skb)->seq);
-
 	tcp_rcv_rtt_measure_ts(sk, skb);
 
 	/* Process urgent data. */
@@ -5997,7 +5999,8 @@ int tcp_rcv_state_process(struct sock *s
 
 	/* step 5: check the ACK field */
 	if (true) {
-		int acceptable = tcp_ack(sk, skb, FLAG_SLOWPATH) > 0;
+		int acceptable = tcp_ack(sk, skb, FLAG_SLOWPATH |
+						  FLAG_UPDATE_TS_RECENT) > 0;
 
 		switch (sk->sk_state) {
 		case TCP_SYN_RECV:
@@ -6148,11 +6151,6 @@ int tcp_rcv_state_process(struct sock *s
 		}
 	}
 
-	/* ts_recent update must be made after we are sure that the packet
-	 * is in window.
-	 */
-	tcp_replace_ts_recent(tp, TCP_SKB_CB(skb)->seq);
-
 	/* step 6: check the URG bit */
 	tcp_urg(sk, skb, th);
 



  parent reply	other threads:[~2013-04-29 19:32 UTC|newest]

Thread overview: 57+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-04-29 19:01 [ 00/42] 3.8.11-stable review Greg Kroah-Hartman
2013-04-29 19:01 ` [ 01/42] aio: fix possible invalid memory access when DEBUG is enabled Greg Kroah-Hartman
2013-04-29 19:01 ` [ 02/42] TTY: do not update atime/mtime on read/write Greg Kroah-Hartman
2013-04-30  0:14   ` Simon Kirby
2013-04-30  0:21     ` Greg Kroah-Hartman
2013-04-30  0:36       ` Simon Kirby
2013-04-30  1:37         ` Greg Kroah-Hartman
2013-04-30 23:50           ` Simon Kirby
2013-05-01  0:57           ` Linus Torvalds
2013-05-01  1:41             ` Linus Torvalds
2013-05-01  5:23               ` Jiri Slaby
2013-05-01 13:05               ` Wolfram Gloger
2013-05-02 16:11               ` Simon Kirby
2013-04-29 19:01 ` [ 03/42] TTY: fix atime/mtime regression Greg Kroah-Hartman
2013-04-30 12:02   ` Wolfram Gloger
2013-04-29 19:01 ` [ 04/42] sparc64: Fix race in TLB batch processing Greg Kroah-Hartman
2013-04-29 19:01 ` [ 05/42] atm: update msg_namelen in vcc_recvmsg() Greg Kroah-Hartman
2013-04-29 19:01 ` [ 06/42] ax25: fix info leak via msg_name in ax25_recvmsg() Greg Kroah-Hartman
2013-04-29 19:01 ` [ 07/42] Bluetooth: fix possible info leak in bt_sock_recvmsg() Greg Kroah-Hartman
2013-04-29 19:01 ` [ 08/42] Bluetooth: RFCOMM - Fix missing msg_namelen update in rfcomm_sock_recvmsg() Greg Kroah-Hartman
2013-04-29 19:01 ` [ 09/42] Bluetooth: SCO - Fix missing msg_namelen update in sco_sock_recvmsg() Greg Kroah-Hartman
2013-04-29 19:01 ` [ 10/42] caif: Fix missing msg_namelen update in caif_seqpkt_recvmsg() Greg Kroah-Hartman
2013-04-29 19:01 ` [ 11/42] irda: Fix missing msg_namelen update in irda_recvmsg_dgram() Greg Kroah-Hartman
2013-04-29 19:01 ` [ 12/42] iucv: Fix missing msg_namelen update in iucv_sock_recvmsg() Greg Kroah-Hartman
2013-04-29 19:01 ` [ 13/42] l2tp: fix info leak in l2tp_ip6_recvmsg() Greg Kroah-Hartman
2013-04-29 19:01 ` [ 14/42] llc: Fix missing msg_namelen update in llc_ui_recvmsg() Greg Kroah-Hartman
2013-04-29 19:01 ` [ 15/42] netrom: fix info leak via msg_name in nr_recvmsg() Greg Kroah-Hartman
2013-04-29 19:01 ` [ 16/42] NFC: llcp: fix info leaks via msg_name in llcp_sock_recvmsg() Greg Kroah-Hartman
2013-04-29 19:01 ` [ 17/42] rose: fix info leak via msg_name in rose_recvmsg() Greg Kroah-Hartman
2013-04-29 19:02 ` [ 18/42] tipc: fix info leaks via msg_name in recv_msg/recv_stream Greg Kroah-Hartman
2013-04-29 19:02 ` [ 19/42] cbq: incorrect processing of high limits Greg Kroah-Hartman
2013-04-29 19:02 ` [ 20/42] net IPv6 : Fix broken IPv6 routing table after loopback down-up Greg Kroah-Hartman
2013-04-29 19:02 ` [ 21/42] net: count hw_addr syncs so that unsync works properly Greg Kroah-Hartman
2013-04-29 19:02 ` [ 22/42] atl1e: limit gso segment size to prevent generation of wrong ip length fields Greg Kroah-Hartman
2013-04-29 19:02 ` [ 23/42] bonding: fix bonding_masters race condition in bond unloading Greg Kroah-Hartman
2013-04-29 19:02 ` [ 24/42] bonding: IFF_BONDING is not stripped on enslave failure Greg Kroah-Hartman
2013-04-29 19:02 ` [ 25/42] bonding: fix l23 and l34 load balancing in forwarding path Greg Kroah-Hartman
2013-04-29 19:02 ` [ 26/42] af_unix: If we dont care about credentials coallesce all messages Greg Kroah-Hartman
2013-04-29 19:02 ` [ 27/42] netfilter: dont reset nf_trace in nf_reset() Greg Kroah-Hartman
2013-04-29 19:02 ` [ 28/42] ipv6/tcp: Stop processing ICMPv6 redirect messages Greg Kroah-Hartman
2013-04-29 19:02 ` [ 29/42] rtnetlink: Call nlmsg_parse() with correct header length Greg Kroah-Hartman
2013-04-29 19:02 ` [ 30/42] tcp: incoming connections might use wrong route under synflood Greg Kroah-Hartman
2013-04-29 19:02 ` [ 31/42] tcp: Reallocate headroom if it would overflow csum_start Greg Kroah-Hartman
2013-04-29 19:02 ` [ 32/42] net: mvmdio: add select PHYLIB Greg Kroah-Hartman
2013-04-29 19:02 ` [ 33/42] esp4: fix error return code in esp_output() Greg Kroah-Hartman
2013-04-29 19:02 ` [ 34/42] net: mvneta: fix improper tx queue usage in mvneta_tx() Greg Kroah-Hartman
2013-04-29 19:02 ` [ 35/42] net: cdc_mbim: remove bogus sizeof() Greg Kroah-Hartman
2013-04-29 19:02 ` Greg Kroah-Hartman [this message]
2013-04-29 19:02 ` [ 37/42] net: rate-limit warn-bad-offload splats Greg Kroah-Hartman
2013-04-29 19:02 ` [ 38/42] net: fix incorrect credentials passing Greg Kroah-Hartman
2013-04-29 19:02 ` [ 39/42] net: drop dst before queueing fragments Greg Kroah-Hartman
2013-04-29 19:02 ` [ 40/42] tracing: Fix selftest function recursion accounting Greg Kroah-Hartman
2013-04-29 19:02 ` [ 41/42] ARM: 7699/1: sched_clock: Add more notrace to prevent recursion Greg Kroah-Hartman
2013-04-29 19:02 ` [ 42/42] ARM: 7692/1: iop3xx: move IOP3XX_PERIPHERAL_VIRT_BASE Greg Kroah-Hartman
2013-04-30  1:54 ` [ 00/42] 3.8.11-stable review Shuah Khan
2013-04-30  1:54   ` Shuah Khan
2013-04-30  2:02   ` Greg Kroah-Hartman

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=20130429184756.467096252@linuxfoundation.org \
    --to=gregkh@linuxfoundation.org \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=ncardwell@google.com \
    --cc=stable@vger.kernel.org \
    --cc=ycheng@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.