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: [ 31/34] tcp: call tcp_replace_ts_recent() from tcp_ack()
Date: Mon, 29 Apr 2013 12:02:38 -0700	[thread overview]
Message-ID: <20130429184704.402950634@linuxfoundation.org> (raw)
In-Reply-To: <20130429184700.845644077@linuxfoundation.org>

3.4-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 |   65 +++++++++++++++++++++++++--------------------------
 1 file changed, 32 insertions(+), 33 deletions(-)

--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -116,6 +116,7 @@ int sysctl_tcp_abc __read_mostly;
 #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)
@@ -3707,6 +3708,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)
 {
@@ -3756,6 +3778,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.
@@ -4053,27 +4081,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)
@@ -5577,14 +5584,10 @@ slow_path:
 		return 0;
 
 step5:
-	if (th->ack && tcp_ack(sk, skb, FLAG_SLOWPATH) < 0)
+	if (th->ack &&
+	    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. */
@@ -5948,7 +5951,8 @@ int tcp_rcv_state_process(struct sock *s
 
 	/* step 5: check the ACK field */
 	if (th->ack) {
-		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:
@@ -6055,11 +6059,6 @@ int tcp_rcv_state_process(struct sock *s
 	} else
 		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);
-
 	/* step 6: check the URG bit */
 	tcp_urg(sk, skb, th);
 



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

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-04-29 19:02 [ 00/34] 3.4.43-stable review Greg Kroah-Hartman
2013-04-29 19:02 ` [ 01/34] aio: fix possible invalid memory access when DEBUG is enabled Greg Kroah-Hartman
2013-04-29 19:02 ` [ 02/34] TTY: do not update atime/mtime on read/write Greg Kroah-Hartman
2013-04-29 19:02 ` [ 03/34] TTY: fix atime/mtime regression Greg Kroah-Hartman
2013-04-29 19:02 ` [ 04/34] sparc64: Fix race in TLB batch processing Greg Kroah-Hartman
2013-04-29 19:02 ` [ 05/34] atm: update msg_namelen in vcc_recvmsg() Greg Kroah-Hartman
2013-04-29 19:02 ` [ 06/34] ax25: fix info leak via msg_name in ax25_recvmsg() Greg Kroah-Hartman
2013-04-29 19:02 ` [ 07/34] Bluetooth: fix possible info leak in bt_sock_recvmsg() Greg Kroah-Hartman
2013-04-29 19:02 ` [ 08/34] Bluetooth: RFCOMM - Fix missing msg_namelen update in rfcomm_sock_recvmsg() Greg Kroah-Hartman
2013-04-29 19:02 ` [ 09/34] caif: Fix missing msg_namelen update in caif_seqpkt_recvmsg() Greg Kroah-Hartman
2013-04-29 19:02 ` [ 10/34] irda: Fix missing msg_namelen update in irda_recvmsg_dgram() Greg Kroah-Hartman
2013-04-29 19:02 ` [ 11/34] iucv: Fix missing msg_namelen update in iucv_sock_recvmsg() Greg Kroah-Hartman
2013-04-29 19:02 ` [ 12/34] llc: Fix missing msg_namelen update in llc_ui_recvmsg() Greg Kroah-Hartman
2013-04-29 19:02 ` [ 13/34] netrom: fix info leak via msg_name in nr_recvmsg() Greg Kroah-Hartman
2013-04-29 19:02 ` [ 14/34] NFC: llcp: fix info leaks via msg_name in llcp_sock_recvmsg() Greg Kroah-Hartman
2013-04-29 19:02 ` [ 15/34] rose: fix info leak via msg_name in rose_recvmsg() Greg Kroah-Hartman
2013-04-29 19:02 ` [ 16/34] tipc: fix info leaks via msg_name in recv_msg/recv_stream Greg Kroah-Hartman
2013-04-29 19:02 ` [ 17/34] netrom: fix invalid use of sizeof in nr_recvmsg() Greg Kroah-Hartman
2013-04-29 19:02 ` [ 18/34] cbq: incorrect processing of high limits Greg Kroah-Hartman
2013-04-29 19:02 ` [ 19/34] net IPv6 : Fix broken IPv6 routing table after loopback down-up Greg Kroah-Hartman
2013-04-29 19:02 ` [ 20/34] net: count hw_addr syncs so that unsync works properly Greg Kroah-Hartman
2013-04-29 19:02 ` [ 21/34] atl1e: limit gso segment size to prevent generation of wrong ip length fields Greg Kroah-Hartman
2013-04-29 19:02 ` [ 22/34] bonding: fix bonding_masters race condition in bond unloading Greg Kroah-Hartman
2013-04-29 19:02 ` [ 23/34] bonding: IFF_BONDING is not stripped on enslave failure Greg Kroah-Hartman
2013-04-29 19:02 ` [ 24/34] af_unix: If we dont care about credentials coallesce all messages Greg Kroah-Hartman
2013-04-29 19:02 ` [ 25/34] netfilter: dont reset nf_trace in nf_reset() Greg Kroah-Hartman
2013-04-29 19:02 ` [ 26/34] rtnetlink: Call nlmsg_parse() with correct header length Greg Kroah-Hartman
2013-04-29 19:02 ` [ 27/34] tcp: incoming connections might use wrong route under synflood Greg Kroah-Hartman
2013-04-29 19:02 ` [ 28/34] tcp: Reallocate headroom if it would overflow csum_start Greg Kroah-Hartman
2013-04-29 19:02 ` [ 29/34] esp4: fix error return code in esp_output() Greg Kroah-Hartman
2013-04-29 19:02 ` [ 30/34] net: sctp: sctp_auth_key_put: use kzfree instead of kfree Greg Kroah-Hartman
2013-04-29 19:02 ` Greg Kroah-Hartman [this message]
2013-04-29 19:02 ` [ 32/34] net: rate-limit warn-bad-offload splats Greg Kroah-Hartman
2013-04-29 19:02 ` [ 33/34] net: fix incorrect credentials passing Greg Kroah-Hartman
2013-04-29 19:02 ` [ 34/34] net: drop dst before queueing fragments Greg Kroah-Hartman
2013-04-30  1:54 ` [ 00/34] 3.4.43-stable review Shuah Khan
2013-04-30  1:54   ` 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=20130429184704.402950634@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.