- check th_ack against snd_una/max; from Raja Mukerji via hugh@

- limit pool to tcp_sackhole_limit entries (sysctl-able)
- stop sack option processing on pool_get errors
- use SEQ_MIN/SEQ_MAX
ok henning, hshoexer, deraadt

4 files changed, 54 insertions, 22 deletions

diff --git a/sys/netinet/tcp_input.c b/sys/netinet/tcp_input.c
index ee964acbcdf..2f3afeee4e4 100644
--- a/sys/netinet/tcp_input.c
+++ b/sys/netinet/tcp_input.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: tcp_input.c,v 1.182 2005/02/27 13:22:56 markus Exp $	*/
+/*	$OpenBSD: tcp_input.c,v 1.183 2005/03/04 13:21:42 markus Exp $	*/
 /*	$NetBSD: tcp_input.c,v 1.23 1996/02/13 23:43:44 christos Exp $	*/
 
 /*
@@ -127,6 +127,10 @@ struct timeval tcp_ackdrop_ppslim_last;
 #define TSTMP_LT(a,b)	((int)((a)-(b)) < 0)
 #define TSTMP_GEQ(a,b)	((int)((a)-(b)) >= 0)
 
+/* for TCP SACK comparisons */
+#define	SEQ_MIN(a,b)	(SEQ_LT(a,b) ? (a) : (b))
+#define	SEQ_MAX(a,b)	(SEQ_GT(a,b) ? (a) : (b))
+
 /*
  * Neighbor Discovery, Neighbor Unreachability Detection Upper layer hint.
  */
@@ -2265,8 +2269,7 @@ tcp_dooptions(tp, cp, cnt, th, m, iphlen, oi)
 				tp->t_flags |= TF_SACK_PERMIT;
 			break;
 		case TCPOPT_SACK:
-			if (tcp_sack_option(tp, th, cp, optlen))
-				continue;
+			tcp_sack_option(tp, th, cp, optlen);
 			break;
 #endif
 #ifdef TCP_SIGNATURE
@@ -2460,11 +2463,10 @@ tcp_update_sack_list(tp)
 }
 
 /*
- * Process the TCP SACK option.  Returns 1 if tcp_dooptions() should continue,
- * and 0 otherwise, if the option was fine.  tp->snd_holes is an ordered list
+ * Process the TCP SACK option.  tp->snd_holes is an ordered list
  * of holes (oldest to newest, in terms of the sequence space).
  */
-int
+void
 tcp_sack_option(struct tcpcb *tp, struct tcphdr *th, u_char *cp, int optlen)
 {
 	int tmp_olen;
@@ -2472,11 +2474,18 @@ tcp_sack_option(struct tcpcb *tp, struct tcphdr *th, u_char *cp, int optlen)
 	struct sackhole *cur, *p, *temp;
 
 	if (!tp->sack_enable)
-		return (1);
-
+		return;
+	/* SACK without ACK doesn't make sense. */
+	if ((th->th_flags & TH_ACK) == 0)
+	       return;
+	/* Make sure the ACK on this segment is in [snd_una, snd_max]. */
+	if (SEQ_LT(th->th_ack, tp->snd_una) ||
+	    SEQ_GT(th->th_ack, tp->snd_max))
+		return;
 	/* Note: TCPOLEN_SACK must be 2*sizeof(tcp_seq) */
 	if (optlen <= 2 || (optlen - 2) % TCPOLEN_SACK != 0)
-		return (1);
+		return;
+	/* Note: TCPOLEN_SACK must be 2*sizeof(tcp_seq) */
 	tmp_cp = cp + 2;
 	tmp_olen = optlen - 2;
 	if (tp->snd_numholes < 0)
@@ -2513,7 +2522,7 @@ tcp_sack_option(struct tcpcb *tp, struct tcphdr *th, u_char *cp, int optlen)
 			    pool_get(&sackhl_pool, PR_NOWAIT);
 			if (tp->snd_holes == NULL) {
 				/* ENOBUFS, so ignore SACKed block for now*/
-				continue;
+				goto done;
 			}
 			cur = tp->snd_holes;
 			cur->start = th->th_ack;
@@ -2577,7 +2586,7 @@ tcp_sack_option(struct tcpcb *tp, struct tcphdr *th, u_char *cp, int optlen)
 				}
 				/* otherwise, move start of hole forward */
 				cur->start = sack.end;
-				cur->rxmit = max (cur->rxmit, cur->start);
+				cur->rxmit = SEQ_MAX(cur->rxmit, cur->start);
 				p = cur;
 				cur = cur->next;
 				continue;
@@ -2591,7 +2600,7 @@ tcp_sack_option(struct tcpcb *tp, struct tcphdr *th, u_char *cp, int optlen)
 					    sack.start);
 #endif /* TCP_FACK */
 				cur->end = sack.start;
-				cur->rxmit = min(cur->rxmit, cur->end);
+				cur->rxmit = SEQ_MIN(cur->rxmit, cur->end);
 				cur->dups++;
 				if (((sack.end - cur->end)/tp->t_maxseg) >=
 				    tcprexmtthresh)
@@ -2609,7 +2618,7 @@ tcp_sack_option(struct tcpcb *tp, struct tcphdr *th, u_char *cp, int optlen)
 				temp = (struct sackhole *)
 				    pool_get(&sackhl_pool, PR_NOWAIT);
 				if (temp == NULL)
-					continue; /* ENOBUFS */
+					goto done; /* ENOBUFS */
 #if defined(TCP_SACK) && defined(TCP_FACK)
 				if (SEQ_GT(cur->rxmit, sack.end))
 					tp->retran_data -=
@@ -2624,9 +2633,9 @@ tcp_sack_option(struct tcpcb *tp, struct tcphdr *th, u_char *cp, int optlen)
 				temp->start = sack.end;
 				temp->end = cur->end;
 				temp->dups = cur->dups;
-				temp->rxmit = max(cur->rxmit, temp->start);
+				temp->rxmit = SEQ_MAX(cur->rxmit, temp->start);
 				cur->end = sack.start;
-				cur->rxmit = min(cur->rxmit, cur->end);
+				cur->rxmit = SEQ_MIN(cur->rxmit, cur->end);
 				cur->dups++;
 				if (((sack.end - cur->end)/tp->t_maxseg) >=
 					tcprexmtthresh)
@@ -2646,7 +2655,7 @@ tcp_sack_option(struct tcpcb *tp, struct tcphdr *th, u_char *cp, int optlen)
 			temp = (struct sackhole *)
 			    pool_get(&sackhl_pool, PR_NOWAIT);
 			if (temp == NULL)
-				continue; /* ENOBUFS */
+				goto done; /* ENOBUFS */
 			temp->start = tp->rcv_lastsack;
 			temp->end = sack.start;
 			temp->dups = min(tcprexmtthresh,
@@ -2660,6 +2669,7 @@ tcp_sack_option(struct tcpcb *tp, struct tcphdr *th, u_char *cp, int optlen)
 			tp->snd_numholes++;
 		}
 	}
+done:
 #if defined(TCP_SACK) && defined(TCP_FACK)
 	/*
 	 * Update retran_data and snd_awnd.  Go through the list of
@@ -2675,7 +2685,7 @@ tcp_sack_option(struct tcpcb *tp, struct tcphdr *th, u_char *cp, int optlen)
 	    tp->retran_data;
 #endif /* TCP_FACK */
 
-	return (0);
+	return;
 }
 
 /*
diff --git a/sys/netinet/tcp_subr.c b/sys/netinet/tcp_subr.c
index 5eb89f30462..b4c6805ddc3 100644
--- a/sys/netinet/tcp_subr.c
+++ b/sys/netinet/tcp_subr.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: tcp_subr.c,v 1.87 2005/02/27 13:22:56 markus Exp $	*/
+/*	$OpenBSD: tcp_subr.c,v 1.88 2005/03/04 13:21:42 markus Exp $	*/
 /*	$NetBSD: tcp_subr.c,v 1.22 1996/02/13 23:44:00 christos Exp $	*/
 
 /*
@@ -149,6 +149,9 @@ int	tcp_syn_bucket_limit = 3*TCP_SYN_BUCKET_SIZE;
 struct	syn_cache_head tcp_syn_cache[TCP_SYN_HASH_SIZE];
 
 int tcp_reass_limit = NMBCLUSTERS / 2; /* hardlimit for tcpqe_pool */
+#ifdef TCP_SACK
+int tcp_sackhole_limit = 32*1024; /* hardlimit for sackhl_pool */
+#endif
 
 #ifdef INET6
 extern int ip6_defhlim;
@@ -180,6 +183,7 @@ tcp_init()
 #ifdef TCP_SACK
 	pool_init(&sackhl_pool, sizeof(struct sackhole), 0, 0, 0, "sackhlpl",
 	    NULL);
+	pool_sethardlimit(&sackhl_pool, tcp_sackhole_limit, NULL, 0);
 #endif /* TCP_SACK */
 	in_pcbinit(&tcbtable, tcbhashsize);
 
diff --git a/sys/netinet/tcp_usrreq.c b/sys/netinet/tcp_usrreq.c
index c897627d691..0fef0fbebbe 100644
--- a/sys/netinet/tcp_usrreq.c
+++ b/sys/netinet/tcp_usrreq.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: tcp_usrreq.c,v 1.88 2005/02/22 17:28:49 mcbride Exp $	*/
+/*	$OpenBSD: tcp_usrreq.c,v 1.89 2005/03/04 13:21:42 markus Exp $	*/
 /*	$NetBSD: tcp_usrreq.c,v 1.20 1996/02/13 23:44:16 christos Exp $	*/
 
 /*
@@ -945,6 +945,20 @@ tcp_sysctl(name, namelen, oldp, oldlenp, newp, newlen)
 			tcp_reass_limit = nval;
 		}
 		return (0);
+#ifdef TCP_SACK
+	case TCPCTL_SACKHOLE_LIMIT:
+		nval = tcp_sackhole_limit;
+		error = sysctl_int(oldp, oldlenp, newp, newlen, &nval);
+		if (error)
+			return (error);
+		if (nval != tcp_sackhole_limit) {
+			error = pool_sethardlimit(&sackhl_pool, nval, NULL, 0);
+			if (error)
+				return (error);
+			tcp_sackhole_limit = nval;
+		}
+		return (0);
+#endif
 	default:
 		if (name[0] < TCPCTL_MAXID)
 			return (sysctl_int_arr(tcpctl_vars, name, namelen,
diff --git a/sys/netinet/tcp_var.h b/sys/netinet/tcp_var.h
index a7f99f7b4a5..17feb553634 100644
--- a/sys/netinet/tcp_var.h
+++ b/sys/netinet/tcp_var.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: tcp_var.h,v 1.70 2005/02/27 13:22:56 markus Exp $	*/
+/*	$OpenBSD: tcp_var.h,v 1.71 2005/03/04 13:21:42 markus Exp $	*/
 /*	$NetBSD: tcp_var.h,v 1.17 1996/02/13 23:44:24 christos Exp $	*/
 
 /*
@@ -472,7 +472,8 @@ struct	tcpstat {
 #define	TCPCTL_RFC3390	       17 /* enable/disable RFC3390 increased cwnd */
 #define	TCPCTL_REASS_LIMIT     18 /* max entries for tcp reass queues */
 #define	TCPCTL_DROP	       19 /* drop tcp connection */
-#define	TCPCTL_MAXID	       20
+#define	TCPCTL_SACKHOLE_LIMIT  20 /* max entries for tcp sack queues */
+#define	TCPCTL_MAXID	       21
 
 #define	TCPCTL_NAMES { \
 	{ 0, 0 }, \
@@ -495,6 +496,7 @@ struct	tcpstat {
 	{ "rfc3390", 	CTLTYPE_INT }, \
 	{ "reasslimit", 	CTLTYPE_INT }, \
 	{ "drop", 	CTLTYPE_STRUCT }, \
+	{ "sackholelimit", 	CTLTYPE_INT }, \
 }
 
 #define	TCPCTL_VARS { \
@@ -517,6 +519,7 @@ struct	tcpstat {
 	&tcp_syn_bucket_limit, \
 	&tcp_do_rfc3390, \
 	NULL, \
+	NULL, \
 	NULL \
 }
 
@@ -535,6 +538,7 @@ extern	int tcp_ack_on_push;	/* ACK immediately on PUSH */
 #ifdef TCP_SACK
 extern	int tcp_do_sack;	/* SACK enabled/disabled */
 extern	struct pool sackhl_pool;
+extern	int tcp_sackhole_limit;	/* max entries for tcp sack queues */
 #endif
 extern	int tcp_do_ecn;		/* RFC3168 ECN enabled/disabled? */
 extern	int tcp_do_rfc3390;	/* RFC3390 Increasing TCP's Initial Window */
@@ -607,7 +611,7 @@ int	 tcp_usrreq(struct socket *,
 void	 tcp_xmit_timer(struct tcpcb *, int);
 void	 tcpdropoldhalfopen(struct tcpcb *, u_int16_t);
 #ifdef TCP_SACK
-int	 tcp_sack_option(struct tcpcb *,struct tcphdr *,u_char *,int);
+void	 tcp_sack_option(struct tcpcb *,struct tcphdr *,u_char *,int);
 void	 tcp_update_sack_list(struct tcpcb *tp);
 void	 tcp_del_sackholes(struct tcpcb *, struct tcphdr *);
 void	 tcp_clean_sackreport(struct tcpcb *tp);