tcp: bugs and cleanup updated to 2.6.33-rc5

PATCH In review William Allen Simpson 1 participant 9 messages Jan 23, 2010

Subsystems (4)

Networking Network drivers TCP Networking IPsec

lore

Share & tools

lore

William Allen Simpson

author

2010-01-23 05:27 UTC #0 lore raw

cover tcp: bugs and cleanup updated to 2.6.33-rc5

Combination of patches reported in October, November, December, and January. These patches fix perceived bugs, and other cleanup.

This code has had previous review and several months of limited testing.

Some portions were removed during the various TCPCT part 1 patch splits, then were cut off by the sudden unexpected end of that merge window. [03 Dec 2009] I've restarted the sub-numbering (again).

Of particular interest are the TCPCT header extensions that already appear in the next phase of testing with other platforms. These patches allow correct reception without data corruption.

The remainder of the original TCPCT part 2 will be merged with part 3.

These patches are against the current linux-2.6 tree.

William Allen Simpson

author

2010-01-23 05:32 UTC #1 lore raw

PATCH 1/7 net: tcp_header_len_th and tcp_option_len_th

Redefine two TCP header functions to accept TCP header pointer. When subtracting, return signed int to allow error checking.

These functions will be used in subsequent patches that implement additional features.

Signed-off-by [email protected]

---
 include/linux/tcp.h |   12 ++++++++++++
 1 files changed, 12 insertions(+), 0 deletions(-)

include/linux/tcp.h +12 -0

index 7fee8a4..d0133cf 100644
@@ -223,6 +223,18 @@ static inline unsigned int tcp_optlen(const struct sk_buff *skb)
 	return (tcp_hdr(skb)->doff - 5) * 4;
 }
 
+/* Length of fixed header plus standard options. */
+static inline unsigned int tcp_header_len_th(const struct tcphdr *th)
+{
+	return th->doff * 4;
+}
+
+/* Length of standard options only.  This could be negative. */
+static inline int tcp_option_len_th(const struct tcphdr *th)
+{
+	return (int)(th->doff * 4) - sizeof(*th);
+}
+
 /* This defines a selective acknowledgement block. */
 struct tcp_sack_block_wire {
 	__be32	start_seq;

Signature · 1 line

1.6.3.3

William Allen Simpson

author

2010-01-23 05:48 UTC #2 lore raw

PATCH 2/7 net: remove old tcp_optlen function

In reply to William Allen Simpson

The tcp_optlen() function returns a potential *negative* unsigned.

In the only two existing files using the old tcp_optlen() function, clean up confusing and inconsistent mixing of both byte and word offsets, and other coding style issues. Document assumptions.

Quoth David Miller:
This is transmit, and the packets can only come from the Linux TCP stack, not some external entity.

You're being way too anal here, and adding these checks to drivers would be just a lot of rediculious bloat. [sic]

Therefore, there are *no* checks for bad TCP and IP header sizes, nor any semantic changes. The drivers should function exactly as existing.

Untested. No response from testers in 13+ weeks.

Requires:
net: tcp_header_len_th and tcp_option_len_th

Signed-off-by [email protected]

CC Michael Chan <[email protected]>

---
 drivers/net/bnx2.c  |   29 +++++++++++++-----------
 drivers/net/tg3.c   |   60 +++++++++++++++++++++++---------------------------
 include/linux/tcp.h |    5 ----
 3 files changed, 44 insertions(+), 50 deletions(-)

drivers/net/bnx2.c +16 ~~-13~~

index 65df1de..45452c5 100644
@@ -6352,6 +6352,8 @@ bnx2_vlan_rx_register(struct net_device *dev, struct vlan_group *vlgrp)
 /* Called with netif_tx_lock.
  * bnx2_tx_int() runs without netif_tx_lock unless it needs to call
  * netif_wake_queue().
+ *
+ * No TCP or IP length checking, per David Miller (see commit log).
  */
 static netdev_tx_t
 bnx2_start_xmit(struct sk_buff *skb, struct net_device *dev)

@@ -6396,19 +6398,19 @@ bnx2_start_xmit(struct sk_buff *skb, struct net_device *dev)
 			(TX_BD_FLAGS_VLAN_TAG | (vlan_tx_tag_get(skb) << 16));
 	}
 #endif
-	if ((mss = skb_shinfo(skb)->gso_size)) {
-		u32 tcp_opt_len;
-		struct iphdr *iph;
+	mss = skb_shinfo(skb)->gso_size;
+	if (mss != 0) {
+		struct tcphdr *th = tcp_hdr(skb);
+		int tcp_opt_words = th->doff - (sizeof(*th) >> 2);
+		/* assumes positive tcp_opt_words without checking */
 
 		vlan_tag_flags |= TX_BD_FLAGS_SW_LSO;
 
-		tcp_opt_len = tcp_optlen(skb);
-
 		if (skb_shinfo(skb)->gso_type & SKB_GSO_TCPV6) {
 			u32 tcp_off = skb_transport_offset(skb) -
 				      sizeof(struct ipv6hdr) - ETH_HLEN;
 
-			vlan_tag_flags |= ((tcp_opt_len >> 2) << 8) |
+			vlan_tag_flags |= (tcp_opt_words << 8) |
 					  TX_BD_FLAGS_SW_FLAGS;
 			if (likely(tcp_off == 0))
 				vlan_tag_flags &= ~TX_BD_FLAGS_TCP6_OFF0_MSK;

@@ -6421,14 +6423,15 @@ bnx2_start_xmit(struct sk_buff *skb, struct net_device *dev)
 				mss |= (tcp_off & 0xc) << TX_BD_TCP6_OFF2_SHL;
 			}
 		} else {
-			iph = ip_hdr(skb);
-			if (tcp_opt_len || (iph->ihl > 5)) {
-				vlan_tag_flags |= ((iph->ihl - 5) +
-						   (tcp_opt_len >> 2)) << 8;
-			}
+			struct iphdr *iph = ip_hdr(skb);
+			int ip_opt_words = iph->ihl - (sizeof(*iph) >> 2);
+			/* assumes positive ip_opt_words without checking */
+			int opt_words = ip_opt_words + tcp_opt_words;
+
+			if (opt_words > 0)
+				vlan_tag_flags |= opt_words << 8;
 		}
-	} else
-		mss = 0;
+	}
 
 	mapping = pci_map_single(bp->pdev, skb->data, len, PCI_DMA_TODEVICE);
 	if (pci_dma_mapping_error(bp->pdev, mapping)) {

drivers/net/tg3.c +28 ~~-32~~

126 lines not on this page · raw message

include/linux/tcp.h +0 -5

index d0133cf..74728f7 100644
@@ -218,11 +218,6 @@ static inline unsigned int tcp_hdrlen(const struct sk_buff *skb)
 	return tcp_hdr(skb)->doff * 4;
 }
 
-static inline unsigned int tcp_optlen(const struct sk_buff *skb)
-{
-	return (tcp_hdr(skb)->doff - 5) * 4;
-}
-
 /* Length of fixed header plus standard options. */
 static inline unsigned int tcp_header_len_th(const struct tcphdr *th)
 {

Signature · 1 line

1.6.3.3

William Allen Simpson

author

2010-01-23 05:56 UTC #3 lore raw

PATCH v5 3/7 tcp: harmonize tcp_vx_rcv header length assumptions

In reply to William Allen Simpson

Harmonize tcp_v4_rcv() and tcp_v6_rcv() -- better document tcp doff and header length assumptions, and carefully compare implementations.

Reduces multiply/shifts, marginally improving speed.

Removes redundant tcp header length checks before checksumming.

Instead, assumes (and documents) that any backlog processing and transform policies will carefully preserve the header, and will ensure the socket buffer length remains >= the header size.

Stand-alone patch, originally developed for TCPCT.

Signed-off-by [email protected]

CC Andi Kleen <[email protected]>

---
 include/net/xfrm.h  |    7 ++++++
 net/ipv4/tcp_ipv4.c |   45 +++++++++++++++++++++-----------------
 net/ipv6/tcp_ipv6.c |   59 ++++++++++++++++++++++++++++----------------------
 3 files changed, 65 insertions(+), 46 deletions(-)

include/net/xfrm.h +7 -0

index 6d85861..89c5465 100644
@@ -975,6 +975,13 @@ xfrm_state_addr_cmp(struct xfrm_tmpl *tmpl, struct xfrm_state *x, unsigned short
 }
 
 #ifdef CONFIG_XFRM
+/*
+ * For transport, the policy is checked before the presumed more expensive
+ * checksum. The transport header has already been checked for size, and is
+ * guaranteed to be contiguous. These policies must not alter the header or
+ * its position in the buffer, and should not shorten the buffer length
+ * without ensuring the length remains >= the header size.
+ */
 extern int __xfrm_policy_check(struct sock *, int dir, struct sk_buff *skb, unsigned short family);
 
 static inline int __xfrm_policy_check2(struct sock *sk, int dir,

net/ipv4/tcp_ipv4.c +25 ~~-20~~

109 lines not on this page · raw message

net/ipv6/tcp_ipv6.c +33 ~~-26~~

126 lines not on this page · raw message

Signature · 1 line

1.6.3.3

William Allen Simpson

author

2010-01-23 06:02 UTC #4 lore raw

PATCH v4 4/7 tcp: input header length, prediction, and timestamp bugs

In reply to William Allen Simpson

Fix incorrect header prediction flags documentation.

Relieve register pressure in (the i386) fast path by accessing skb->len directly, instead of carrying a rarely used len parameter.

Eliminate unused len parameters in two other functions.

Don't use output calculated tp->tcp_header_len for input decisions. While the output header is usually the same as the input (same options in both directions), that's a poor assumption. In particular, Sack will be different. Newer options are not guaranteed.

Moreover, in the fast path, that only saved a shift or two. The other efficiencies in this patch more than make up the difference.

Instead, use tp->rx_opt.tstamp_ok to accurately predict header length.

Likewise, use tp->rx_opt.tstamp_ok for received MSS calculations.

Don't use "sizeof(struct tcphdr) + TCPOLEN_TSTAMP_ALIGNED" to guess that the timestamp is present. This may have been OK in the days with fewer possible options, but various combinations of newer options may yield the same header length. (This bug is in 3 places.)

Instead, use tp->rx_opt.saw_tstamp to determine a timestamp is present.

There's no need to test buffer length against header length, already checked by tcp_v[4,6]_rcv(). Straighten code for minor efficiency gain.

Stand-alone patch, originally developed for TCPCT.

Requires:
net: tcp_header_len_th and tcp_option_len_th
tcp: harmonize tcp_vx_rcv header length assumptions

Signed-off-by [email protected]

CC Andi Kleen <[email protected]>

---
 include/linux/tcp.h      |    6 ++-
 include/net/tcp.h        |   15 +++++--
 net/ipv4/tcp_input.c     |   94 ++++++++++++++++++----------------------------
 net/ipv4/tcp_ipv4.c      |    4 +-
 net/ipv4/tcp_minisocks.c |    3 +-
 net/ipv4/tcp_probe.c     |    2 +-
 net/ipv6/tcp_ipv6.c      |    4 +-
 7 files changed, 58 insertions(+), 70 deletions(-)

include/linux/tcp.h +5 -1

index 74728f7..2987ee8 100644
@@ -301,7 +301,11 @@ struct tcp_sock {
 
 /*
  *	Header prediction flags
- *	0x5?10 << 16 + snd_wnd in net byte order
+ *	S << 28 + TCP_FLAG_ACK + snd_wnd, in net byte order
+ *		(PSH flag is ignored)
+ *		S is 5 (no options), or 8 (timestamp aligned)
+ *	otherwise, 0 to turn it off -- for instance, when there are
+ *	holes in receive space.
  */
 	__be32	pred_flags;

include/net/tcp.h +10 -5

index 34f5cc2..6b0d7e9 100644
@@ -310,13 +310,11 @@ extern int			tcp_ioctl(struct sock *sk,
 
 extern int			tcp_rcv_state_process(struct sock *sk, 
 						      struct sk_buff *skb,
-						      struct tcphdr *th,
-						      unsigned len);
+						      struct tcphdr *th);
 
 extern int			tcp_rcv_established(struct sock *sk, 
 						    struct sk_buff *skb,
-						    struct tcphdr *th, 
-						    unsigned len);
+						    struct tcphdr *th);
 
 extern void			tcp_rcv_space_adjust(struct sock *sk);

@@ -533,9 +531,16 @@ static inline u32 __tcp_set_rto(const struct tcp_sock *tp)
 	return (tp->srtt >> 3) + tp->rttvar;
 }
 
+static inline u16 __tcp_fast_path_header_length(const struct tcp_sock *tp)
+{
+	return tp->rx_opt.tstamp_ok
+		? sizeof(struct tcphdr) + TCPOLEN_TSTAMP_ALIGNED
+		: sizeof(struct tcphdr);
+}
+
 static inline void __tcp_fast_path_on(struct tcp_sock *tp, u32 snd_wnd)
 {
-	tp->pred_flags = htonl((tp->tcp_header_len << 26) |
+	tp->pred_flags = htonl((__tcp_fast_path_header_length(tp) << (28 - 2)) |
 			       ntohl(TCP_FLAG_ACK) |
 			       snd_wnd);
 }

net/ipv4/tcp_input.c +37 ~~-57~~

166 lines not on this page · raw message

net/ipv4/tcp_ipv4.c +2 -2

index 0a76e41..f999e06 100644
@@ -1551,7 +1551,7 @@ int tcp_v4_do_rcv(struct sock *sk, struct sk_buff *skb)
 
 	if (sk->sk_state == TCP_ESTABLISHED) { /* Fast path */
 		TCP_CHECK_TIMER(sk);
-		if (tcp_rcv_established(sk, skb, tcp_hdr(skb), skb->len)) {
+		if (tcp_rcv_established(sk, skb, tcp_hdr(skb))) {
 			rsk = sk;
 			goto reset;
 		}

@@ -1578,7 +1578,7 @@ int tcp_v4_do_rcv(struct sock *sk, struct sk_buff *skb)
 	}
 
 	TCP_CHECK_TIMER(sk);
-	if (tcp_rcv_state_process(sk, skb, tcp_hdr(skb), skb->len)) {
+	if (tcp_rcv_state_process(sk, skb, tcp_hdr(skb))) {
 		rsk = sk;
 		goto reset;
 	}

net/ipv4/tcp_minisocks.c +1 -2

index f206ee5..37b7536 100644
@@ -718,8 +718,7 @@ int tcp_child_process(struct sock *parent, struct sock *child,
 	int state = child->sk_state;
 
 	if (!sock_owned_by_user(child)) {
-		ret = tcp_rcv_state_process(child, skb, tcp_hdr(skb),
-					    skb->len);
+		ret = tcp_rcv_state_process(child, skb, tcp_hdr(skb));
 		/* Wakeup parent, send SIGIO */
 		if (state == TCP_SYN_RECV && child->sk_state != state)
 			parent->sk_data_ready(parent, 0);

net/ipv4/tcp_probe.c +1 -1

index bb110c5..7bcd3ba 100644
@@ -88,7 +88,7 @@ static inline int tcp_probe_avail(void)
  * Note: arguments must match tcp_rcv_established()!
  */
 static int jtcp_rcv_established(struct sock *sk, struct sk_buff *skb,
-			       struct tcphdr *th, unsigned len)
+			       struct tcphdr *th)
 {
 	const struct tcp_sock *tp = tcp_sk(sk);
 	const struct inet_sock *inet = inet_sk(sk);

net/ipv6/tcp_ipv6.c +2 -2

index b76939a..3d08a4d 100644
@@ -1586,7 +1586,7 @@ static int tcp_v6_do_rcv(struct sock *sk, struct sk_buff *skb)
 
 	if (sk->sk_state == TCP_ESTABLISHED) { /* Fast path */
 		TCP_CHECK_TIMER(sk);
-		if (tcp_rcv_established(sk, skb, tcp_hdr(skb), skb->len))
+		if (tcp_rcv_established(sk, skb, tcp_hdr(skb)))
 			goto reset;
 		TCP_CHECK_TIMER(sk);
 		if (opt_skb)

@@ -1618,7 +1618,7 @@ static int tcp_v6_do_rcv(struct sock *sk, struct sk_buff *skb)
 	}
 
 	TCP_CHECK_TIMER(sk);
-	if (tcp_rcv_state_process(sk, skb, tcp_hdr(skb), skb->len))
+	if (tcp_rcv_state_process(sk, skb, tcp_hdr(skb)))
 		goto reset;
 	TCP_CHECK_TIMER(sk);
 	if (opt_skb)

Signature · 1 line

1.6.3.3

William Allen Simpson

author

2010-01-23 06:09 UTC #5 lore raw

PATCH v3 5/7 TCPCT part 2e: accept SYNACK data

In reply to William Allen Simpson

When accompanied by cookie option, Initiator (client) queues incoming SYNACK transaction data.

This is a straightforward re-implementation of an earlier (year-old) patch that no longer applies cleanly, with permission of the original author (Adam Langley). The patch was previously reviewed:

http://thread.gmane.org/gmane.linux.network/102586

This function will also be used in subsequent patches that implement additional features.

Requires:
TCPCT part 1g: Responder Cookie => Initiator
net: tcp_header_len_th and tcp_option_len_th

Signed-off-by [email protected]

---
 net/ipv4/tcp_input.c |   26 +++++++++++++++++++++++++-
 1 files changed, 25 insertions(+), 1 deletions(-)

net/ipv4/tcp_input.c +25 -1

index 8e0f6ae..165040e 100644
@@ -5395,6 +5395,12 @@ discard:
 	return 0;
 }
 
+/*
+ * Returns:
+ *	+1 on reset,
+ *	0 success and/or SYNACK data,
+ *	-1 on discard.
+ */
 static int tcp_rcv_synsent_state_process(struct sock *sk, struct sk_buff *skb,
 					 struct tcphdr *th)
 {

@@ -5403,6 +5409,7 @@ static int tcp_rcv_synsent_state_process(struct sock *sk, struct sk_buff *skb,
 	struct tcp_sock *tp = tcp_sk(sk);
 	struct tcp_cookie_values *cvp = tp->cookie_values;
 	int saved_clamp = tp->rx_opt.mss_clamp;
+	int queued = 0;
 
 	tcp_parse_options(skb, &tp->rx_opt, &hash_location, 0);

@@ -5509,6 +5516,7 @@ static int tcp_rcv_synsent_state_process(struct sock *sk, struct sk_buff *skb,
 					- TCPOLEN_COOKIE_BASE;
 			int cookie_pair_size = cookie_size
 					     + cvp->cookie_desired;
+			int tcp_header_len = tcp_header_len_th(th);
 
 			/* A cookie extension option was sent and returned.
 			 * Note that each incoming SYNACK replaces the

@@ -5524,6 +5532,19 @@ static int tcp_rcv_synsent_state_process(struct sock *sk, struct sk_buff *skb,
 				       hash_location, cookie_size);
 				cvp->cookie_pair_size = cookie_pair_size;
 			}
+
+			queued = skb->len - tcp_header_len;
+			if (queued > 0) {
+				/* Queue incoming transaction data. */
+				__skb_pull(skb, tcp_header_len);
+				__skb_queue_tail(&sk->sk_receive_queue, skb);
+				skb_set_owner_r(skb, sk);
+				sk->sk_data_ready(sk, 0);
+				cvp->s_data_in = 1; /* true */
+				tp->rcv_nxt = TCP_SKB_CB(skb)->end_seq;
+				tp->rcv_wup = TCP_SKB_CB(skb)->end_seq;
+				tp->copied_seq = TCP_SKB_CB(skb)->seq + 1;
+			}
 		}
 
 		smp_mb();

@@ -5577,11 +5598,14 @@ static int tcp_rcv_synsent_state_process(struct sock *sk, struct sk_buff *skb,
 						  TCP_DELACK_MAX, TCP_RTO_MAX);
 
 discard:
-			__kfree_skb(skb);
+			if (queued <= 0)
+				__kfree_skb(skb);
 			return 0;
 		} else {
 			tcp_send_ack(sk);
 		}
+		if (queued > 0)
+			return 0;
 		return -1;
 	}

Signature · 1 line

1.6.3.3

William Allen Simpson

author

2010-01-23 06:14 UTC #6 lore raw

PATCH v3 6/7 TCPCT part 2f: cleanup tcp_parse_options

In reply to William Allen Simpson

Split switch, shift cases to the left, fix most lines beyond column 80.

Prepare (future) error return.

Requires:
TCPCT part 1g: Responder Cookie => Initiator
net: tcp_header_len_th and tcp_option_len_th

Signed-off-by [email protected]

---
 include/net/tcp.h        |    3 +-
 net/ipv4/syncookies.c    |    5 +-
 net/ipv4/tcp_input.c     |  217 ++++++++++++++++++++++++++--------------------
 net/ipv4/tcp_ipv4.c      |   10 ++-
 net/ipv4/tcp_minisocks.c |   14 ++-
 net/ipv6/syncookies.c    |    5 +-
 net/ipv6/tcp_ipv6.c      |    6 +-
 7 files changed, 154 insertions(+), 106 deletions(-)

include/net/tcp.h +2 -1

index 6b0d7e9..420e872 100644
@@ -403,7 +403,8 @@ extern int			tcp_recvmsg(struct kiocb *iocb, struct sock *sk,
 					    size_t len, int nonblock, 
 					    int flags, int *addr_len);
 
-extern void			tcp_parse_options(struct sk_buff *skb,
+extern int			tcp_parse_options(struct sk_buff *skb,
+						  const struct tcphdr *th,
 						  struct tcp_options_received *opt_rx,
 						  u8 **hvpp,
 						  int estab);

net/ipv4/syncookies.c +4 -1

index 66fd80e..3bed530 100644
@@ -254,6 +254,7 @@ struct sock *cookie_v4_check(struct sock *sk, struct sk_buff *skb,
 			     struct ip_options *opt)
 {
 	struct tcp_options_received tcp_opt;
+	int parsed;
 	u8 *hash_location;
 	struct inet_request_sock *ireq;
 	struct tcp_request_sock *treq;

@@ -279,7 +280,9 @@ struct sock *cookie_v4_check(struct sock *sk, struct sk_buff *skb,
 
 	/* check for timestamp cookie support */
 	memset(&tcp_opt, 0, sizeof(tcp_opt));
-	tcp_parse_options(skb, &tcp_opt, &hash_location, 0);
+	parsed = tcp_parse_options(skb, th, &tcp_opt, &hash_location, 0);
+	if (parsed < 0)
+		goto out;
 
 	if (tcp_opt.saw_tstamp)
 		cookie_check_timestamp(&tcp_opt);

net/ipv4/tcp_input.c +122 ~~-95~~

269 lines not on this page · raw message

net/ipv4/tcp_ipv4.c +7 -3

index f999e06..3f0813f 100644
@@ -1215,6 +1215,7 @@ int tcp_v4_conn_request(struct sock *sk, struct sk_buff *skb)
 {
 	struct tcp_extend_values tmp_ext;
 	struct tcp_options_received tmp_opt;
+	int parsed;
 	u8 *hash_location;
 	struct request_sock *req;
 	struct inet_request_sock *ireq;

@@ -1265,7 +1266,10 @@ int tcp_v4_conn_request(struct sock *sk, struct sk_buff *skb)
 	tcp_clear_options(&tmp_opt);
 	tmp_opt.mss_clamp = TCP_MSS_DEFAULT;
 	tmp_opt.user_mss  = tp->rx_opt.user_mss;
-	tcp_parse_options(skb, &tmp_opt, &hash_location, 0);
+	parsed = tcp_parse_options(skb, tcp_hdr(skb), &tmp_opt, &hash_location,
+				   0);
+	if (parsed < 0)
+		goto drop_and_free;
 
 	if (tmp_opt.cookie_plus > 0 &&
 	    tmp_opt.saw_tstamp &&

@@ -1278,7 +1282,7 @@ int tcp_v4_conn_request(struct sock *sk, struct sk_buff *skb)
 		int l = tmp_opt.cookie_plus - TCPOLEN_COOKIE_BASE;
 
 		if (tcp_cookie_generator(&tmp_ext.cookie_bakery[0]) != 0)
-			goto drop_and_release;
+			goto drop_and_free;
 
 		/* Secret recipe starts with IP addresses */
 		*mess++ ^= daddr;

@@ -1299,7 +1303,7 @@ int tcp_v4_conn_request(struct sock *sk, struct sk_buff *skb)
 		tmp_ext.cookie_out_never = 1; /* true */
 		tmp_ext.cookie_plus = 0;
 	} else {
-		goto drop_and_release;
+		goto drop_and_free;
 	}
 	tmp_ext.cookie_in_always = tp->rx_opt.cookie_in_always;

net/ipv4/tcp_minisocks.c +10 -4

index 37b7536..0d42635 100644
@@ -97,9 +97,12 @@ tcp_timewait_state_process(struct inet_timewait_sock *tw, struct sk_buff *skb,
 
 	tmp_opt.saw_tstamp = 0;
 	if (th->doff > (sizeof(*th) >> 2) && tcptw->tw_ts_recent_stamp) {
-		tcp_parse_options(skb, &tmp_opt, &hash_location, 0);
+		int parsed = tcp_parse_options(skb, th, &tmp_opt,
+					       &hash_location, 0);
 
-		if (tmp_opt.saw_tstamp) {
+		if (parsed < 0) {
+			paws_reject = 1; /* true */
+		} else if (tmp_opt.saw_tstamp) {
 			tmp_opt.ts_recent	= tcptw->tw_ts_recent;
 			tmp_opt.ts_recent_stamp	= tcptw->tw_ts_recent_stamp;
 			paws_reject = tcp_paws_reject(&tmp_opt, th->rst);

@@ -528,9 +531,12 @@ struct sock *tcp_check_req(struct sock *sk, struct sk_buff *skb,
 
 	tmp_opt.saw_tstamp = 0;
 	if (th->doff > (sizeof(struct tcphdr)>>2)) {
-		tcp_parse_options(skb, &tmp_opt, &hash_location, 0);
+		int parsed = tcp_parse_options(skb, th, &tmp_opt,
+					       &hash_location, 0);
 
-		if (tmp_opt.saw_tstamp) {
+		if (parsed < 0) {
+			paws_reject = 1; /* true */
+		} else if (tmp_opt.saw_tstamp) {
 			tmp_opt.ts_recent = req->ts_recent;
 			/* We do not store true stamp, but it is not required,
 			 * it can be estimated (approximately)

net/ipv6/syncookies.c +4 -1

index 7208a06..3072500 100644
@@ -160,6 +160,7 @@ static inline int cookie_check(struct sk_buff *skb, __u32 cookie)
 struct sock *cookie_v6_check(struct sock *sk, struct sk_buff *skb)
 {
 	struct tcp_options_received tcp_opt;
+	int parsed;
 	u8 *hash_location;
 	struct inet_request_sock *ireq;
 	struct inet6_request_sock *ireq6;

@@ -187,7 +188,9 @@ struct sock *cookie_v6_check(struct sock *sk, struct sk_buff *skb)
 
 	/* check for timestamp cookie support */
 	memset(&tcp_opt, 0, sizeof(tcp_opt));
-	tcp_parse_options(skb, &tcp_opt, &hash_location, 0);
+	parsed = tcp_parse_options(skb, th, &tcp_opt, &hash_location, 0);
+	if (parsed < 0)
+		goto out;
 
 	if (tcp_opt.saw_tstamp)
 		cookie_check_timestamp(&tcp_opt);

net/ipv6/tcp_ipv6.c +5 -1

index 3d08a4d..e15e4f6 100644
@@ -1164,6 +1164,7 @@ static int tcp_v6_conn_request(struct sock *sk, struct sk_buff *skb)
 {
 	struct tcp_extend_values tmp_ext;
 	struct tcp_options_received tmp_opt;
+	int parsed;
 	u8 *hash_location;
 	struct request_sock *req;
 	struct inet6_request_sock *treq;

@@ -1207,7 +1208,10 @@ static int tcp_v6_conn_request(struct sock *sk, struct sk_buff *skb)
 	tcp_clear_options(&tmp_opt);
 	tmp_opt.mss_clamp = IPV6_MIN_MTU - sizeof(struct tcphdr) - sizeof(struct ipv6hdr);
 	tmp_opt.user_mss = tp->rx_opt.user_mss;
-	tcp_parse_options(skb, &tmp_opt, &hash_location, 0);
+	parsed = tcp_parse_options(skb, tcp_hdr(skb), &tmp_opt, &hash_location,
+				   0);
+	if (parsed < 0)
+		goto drop_and_free;
 
 	if (tmp_opt.cookie_plus > 0 &&
 	    tmp_opt.saw_tstamp &&

Signature · 1 line

1.6.3.3

William Allen Simpson

author

2010-01-23 06:28 UTC #7 lore raw

PATCH v4 7/7 TCPCT part 2g: parse cookie pair and 64-bit timestamp

In reply to William Allen Simpson

Parse cookie pair extended option (previously defined).

Define and parse 64-bit timestamp extended option (and minor cleanup). However, only 32-bits are used at this time (permitted by specification).

Every bit is sacred. Use as few bits as possible in the tcp_sock structure, at the expense of performance.

Requires:
net: tcp_header_len_th and tcp_option_len_th
TCPCT part 2f: cleanup tcp_parse_options

Signed-off-by [email protected]

---
 include/linux/tcp.h  |   10 ++++-
 include/net/tcp.h    |   45 ++++++++++---------
 net/ipv4/tcp_input.c |  119 ++++++++++++++++++++++++++++++++++++++++++++++----
 3 files changed, 142 insertions(+), 32 deletions(-)

Quoted 1 line

From bf57376550e7d5ae3d2342a619fdfc8ae2162e3b Mon Sep 17 00:00:00 2001

include/linux/tcp.h +9 -1

index 2987ee8..b71be6c 100644
@@ -260,13 +260,21 @@ struct tcp_options_received {
 	u8	num_sacks;	/* Number of SACK blocks		*/
 	u16	user_mss;	/* mss requested by user in ioctl	*/
 	u16	mss_clamp;	/* Maximal mss, negotiated at connection setup */
+
+	/* When the options are extended beyond the maximum 40 bytes,
+	 * then this holds the additional data offset (in 32-bit words).
+	 */
+	u16	extended:12,	/* Up to 3,315 = 13 (40/3) by 255	*/
+		saw_tstamp64:1,	/* Seen on recent packet		*/
+		tstamp64_ok:1,	/* Verified with cookie pair		*/
+		__unused:2;
 };
 
 static inline void tcp_clear_options(struct tcp_options_received *rx_opt)
 {
 	rx_opt->tstamp_ok = rx_opt->sack_ok = 0;
 	rx_opt->wscale_ok = rx_opt->snd_wscale = 0;
-	rx_opt->cookie_plus = 0;
+	rx_opt->tstamp64_ok = 0;
 }
 
 /* This is the max number of SACKS that we'll generate and process. It's safe

include/net/tcp.h +23 ~~-22~~

62 lines not on this page · raw message

net/ipv4/tcp_input.c +110 -9

196 lines not on this page · raw message

Signature · 1 line

1.6.3.3

William Allen Simpson

author

2010-01-23 06:49 UTC #8 lore raw

Re: [PATCH v5 7/7] TCPCT part 2g: parse cookie pair and 64-bit timestamp

In reply to William Allen Simpson

Parse cookie pair extended option (previously defined).

Define and parse 64-bit timestamp extended option (and minor cleanup). However, only 32-bits are used at this time (permitted by specification).

Every bit is sacred. Use as few bits as possible in the tcp_sock structure, at the expense of performance.

[v5 fixed trivial error]

Requires:
net: tcp_header_len_th and tcp_option_len_th
TCPCT part 2f: cleanup tcp_parse_options

Signed-off-by [email protected]

---
 include/linux/tcp.h  |   10 ++++-
 include/net/tcp.h    |   45 ++++++++++---------
 net/ipv4/tcp_input.c |  119 ++++++++++++++++++++++++++++++++++++++++++++++----
 3 files changed, 142 insertions(+), 32 deletions(-)

include/linux/tcp.h +9 -1

index 2987ee8..b71be6c 100644
@@ -260,13 +260,21 @@ struct tcp_options_received {
 	u8	num_sacks;	/* Number of SACK blocks		*/
 	u16	user_mss;	/* mss requested by user in ioctl	*/
 	u16	mss_clamp;	/* Maximal mss, negotiated at connection setup */
+
+	/* When the options are extended beyond the maximum 40 bytes,
+	 * then this holds the additional data offset (in 32-bit words).
+	 */
+	u16	extended:12,	/* Up to 3,315 = 13 (40/3) by 255	*/
+		saw_tstamp64:1,	/* Seen on recent packet		*/
+		tstamp64_ok:1,	/* Verified with cookie pair		*/
+		__unused:2;
 };
 
 static inline void tcp_clear_options(struct tcp_options_received *rx_opt)
 {
 	rx_opt->tstamp_ok = rx_opt->sack_ok = 0;
 	rx_opt->wscale_ok = rx_opt->snd_wscale = 0;
-	rx_opt->cookie_plus = 0;
+	rx_opt->tstamp64_ok = 0;
 }
 
 /* This is the max number of SACKS that we'll generate and process. It's safe

include/net/tcp.h +23 ~~-22~~

62 lines not on this page · raw message

net/ipv4/tcp_input.c +110 -9

196 lines not on this page · raw message

Signature · 1 line

1.6.3.3