code: plan9front

Info  •  Files  •  Log

tcp: fix limbo entry leaks from hell

In limbo() function, once tpriv->nlimbo
reaches Maxlimbo, we'd try to re-use
Limbo entries from the head of the hash
chain. However, theres a special case
where our current chain contains only
a single entry. Then Limbo **l; points
to its next pointer, and writing:
*l = lp; would just yield in the entry
being linked to itself, leaking it.

The for(;;) loop in limborexmit() was wrong,
as the "continue" case would not advance
the lp pointer at all, (such as when
tpriv->nlimbo reaches > 100), we'd stop
cleaning out entries.

Handle Fsnewcall() returning nil case,
have to free Limbo *lp as we just removed
it from the hash table.

Add tpriv->nlimbo as "InLimbo" at the
end of /net/tcp/stats.

--- a/sys/src/9/ip/tcp.c

+++ b/sys/src/9/ip/tcp.c

@@ -1537,8 +1537,7 @@

 	tpriv = s->p->priv;

 	h = hashipa(source, seg->source);

-	for(l = &tpriv->lht[h]; *l != nil; l = &lp->next){

-		lp = *l;

+	for(l = &tpriv->lht[h]; (lp = *l) != nil; l = &lp->next){

 		if(lp->lport != seg->dest || lp->rport != seg->source || lp->version != version)

 			continue;

 		if(ipcmp(lp->raddr, source) != 0)

@@ -1550,19 +1549,16 @@

 		lp->irs = seg->seq;

 		break;

 	}

-	lp = *l;

-	if(lp == nil){

-		if(tpriv->nlimbo >= Maxlimbo && tpriv->lht[h]){

-			lp = tpriv->lht[h];

-			tpriv->lht[h] = lp->next;

-			lp->next = nil;

+	if((lp = *l) == nil){

+		if(tpriv->nlimbo >= Maxlimbo && (lp = tpriv->lht[h]) != nil){

+			if((tpriv->lht[h] = lp->next) == nil)

+				l = &tpriv->lht[h];

 		} else {

-			lp = malloc(sizeof(*lp));

-			if(lp == nil)

+			if((lp = malloc(sizeof(*lp))) == nil)

 				return;

 			tpriv->nlimbo++;

 		}

-		*l = lp;

+		lp->next = nil;

 		lp->version = version;

 		ipmove(lp->laddr, dest);

 		ipmove(lp->raddr, source);

@@ -1572,11 +1568,11 @@

 		lp->rcvscale = seg->ws;

 		lp->irs = seg->seq;

 		lp->iss = (nrand(1<<16)<<16)|nrand(1<<16);

+		*l = lp;

 	}

-

 	if(sndsynack(s->p, lp) < 0){

-		*l = lp->next;

 		tpriv->nlimbo--;

+		*l = lp->next;

 		free(lp);

 	}

 }

@@ -1589,9 +1585,8 @@

 {

 	Tcppriv *tpriv;

 	Limbo **l, *lp;

-	int h;

-	int seen;

 	ulong now;

+	int h;

 	tpriv = tcp->priv;

@@ -1601,34 +1596,17 @@

 		qunlock(tcp);

 		return;

 	}

-	seen = 0;

 	now = NOW;

-	for(h = 0; h < NLHT && seen < tpriv->nlimbo; h++){

-		for(l = &tpriv->lht[h]; *l != nil && seen < tpriv->nlimbo; ){

-			lp = *l;

-			seen++;

-			if(now - lp->lastsend < (lp->rexmits+1)*SYNACK_RXTIMER)

-				continue;

-

-			/* time it out after 1 second */

-			if(++(lp->rexmits) > 5){

-				tpriv->nlimbo--;

-				*l = lp->next;

-				free(lp);

-				continue;

+	for(h = 0; h < NLHT; h++){

+		for(l = &tpriv->lht[h]; (lp = *l) != nil; ){

+			if(now - lp->lastsend >= (lp->rexmits+1)*SYNACK_RXTIMER){

+				if(++(lp->rexmits) > 5 || sndsynack(tcp, lp) < 0){

+					tpriv->nlimbo--;

+					*l = lp->next;

+					free(lp);

+					continue;

+				}

 			}

-

-			/* if we're being attacked, don't bother resending SYN ACK's */

-			if(tpriv->nlimbo > 100)

-				continue;

-

-			if(sndsynack(tcp, lp) < 0){

-				*l = lp->next;

-				tpriv->nlimbo--;

-				free(lp);

-				continue;

-			}

-

 			l = &lp->next;

 		}

 	}

@@ -1645,8 +1623,8 @@

 limborst(Conv *s, Tcp *segp, uchar *src, uchar *dst, uchar version)

 {

 	Limbo *lp, **l;

-	int h;

 	Tcppriv *tpriv;

+	int h;

 	tpriv = s->p->priv;

@@ -1737,8 +1715,10 @@

 		return nil;

 	new = Fsnewcall(s, src, segp->source, dst, segp->dest, version);

-	if(new == nil)

+	if(new == nil){

+		free(lp);

 		return nil;

+	}

 	memmove(new->ptcl, s->ptcl, sizeof(Tcpctl));

 	tcb = (Tcpctl*)new->ptcl;

@@ -3415,6 +3395,7 @@

 	e = p+len;

 	for(i = 0; i < Nstats; i++)

 		p = seprint(p, e, "%s: %llud\n", statnames[i], priv->stats[i]);

+	p = seprint(p, e, "InLimbo: %d\n", priv->nlimbo);

 	return p - buf;

 }