shithub: riscv

Download patch

ref: 0298949dd26a6ed269a6d071ba49a5b76f6d645a
parent: b43c416083cfb67262630f61ae98383a7a13a45f
author: cinap_lenrek <cinap_lenrek@felloff.net>
date: Mon Apr 22 14:44:53 EDT 2024

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;
 }