code: plan9front

Download patch

ref: 365e63b36a413703f5fcd0ed5ccbd79c3a53afbf
parent: e687d254784b0c4ab12636f7a942f447418d5e82
author: cinap_lenrek <cinap_lenrek@felloff.net>
date: Sun Oct 10 10:27:08 EDT 2021 

devip: properly rlock() the routelock for v4lookup() and v6lookup()

i'm not confident about mutating the route tree
pointers and have concurrent readers walking the
pointer chains.

given that most route lookups are bypassed now
for non-routing case and we are not building a
high performance router here, lets play it safe.


--- a/sys/src/9/ip/iproute.c
+++ b/sys/src/9/ip/iproute.c
@@ -47,14 +47,10 @@
 		l = &v6freelist;
 	}
 
-	r = *l;
-	if(r != nil){
+	if((r = *l) != nil)
 		*l = r->mid;
-	} else {
-		r = malloc(n);
-		if(r == nil)
-			panic("out of routing nodes");
-	}
+	else
+		r = smalloc(n);
 	memset(r, 0, n);
 	r->type = type;
 	r->ifc = nil;
@@ -388,7 +384,7 @@
 static void
 routeadd(Fs *f, Route *r)
 {
-	Route **h, **e, *p;
+	Route **h, **e, *p, *q;
 
 	if(r->type & Rv4){
 		h = &f->v4root[V4H(r->v4.address)];
@@ -414,8 +410,9 @@
 		addnode(f, h, p);
 		while((p = f->queue) != nil) {
 			f->queue = p->mid;
-			walkadd(f, h, p->left);
+			q = p->left;
 			freeroute(p);
+			walkadd(f, h, q);
 		}
 	}
 
@@ -428,7 +425,7 @@
 static void
 routerem(Fs *f, Route *r)
 {
-	Route **h, **e, **l, *p;
+	Route **h, **e, **l, *p, *q;
 
 	if(r->type & Rv4){
 		h = &f->v4root[V4H(r->v4.address)];
@@ -452,8 +449,9 @@
 
 		while((p = f->queue) != nil) {
 			f->queue = p->mid;
-			walkadd(f, h, p->left);
+			q = p->left;
 			freeroute(p);
+			walkadd(f, h, q);
 		}
 	}
 
@@ -605,9 +603,11 @@
 	&& q->ref > 0)
 		return q;
 
+	q = nil;
 	la = nhgetl(a);
 	ls = nhgetl(s);
-	q = nil;
+
+	rlock(&routelock);
 	for(p = f->v4root[V4H(la)]; p != nil;){
 		if(la < p->v4.address){
 			p = p->left;
@@ -630,15 +630,15 @@
 		q = p;
 		p = p->mid;
 	}
-
-	if(q == nil || q->ref == 0 || routefindipifc(q, f) == nil)
-		return nil;
-
-	if(rh != nil){
-		rh->r = q;
-		rh->rgen = v4routegeneration;
+	if(q != nil){
+		if(routefindipifc(q, f) == nil)
+			q = nil;
+		else if(rh != nil){
+			rh->r = q;
+			rh->rgen = v4routegeneration;
+		}
 	}
-
+	runlock(&routelock);
 	return q;
 }
 
@@ -665,12 +665,13 @@
 	&& q->ref > 0)
 		return q;
 
+	q = nil;
 	for(h = 0; h < IPllen; h++){
 		la[h] = nhgetl(a+4*h);
 		ls[h] = nhgetl(s+4*h);
 	}
 
-	q = nil;
+	rlock(&routelock);
 	for(p = f->v6root[V6H(la)]; p != nil;){
 		for(h = 0; h < IPllen; h++){
 			x = la[h];
@@ -722,15 +723,15 @@
 		p = p->mid;
 next:		;
 	}
-
-	if(q == nil || q->ref == 0 || routefindipifc(q, f) == nil)
-		return nil;
-
-	if(rh != nil){
-		rh->r = q;
-		rh->rgen = v6routegeneration;
+	if(q != nil){
+		if(routefindipifc(q, f) == nil)
+			q = nil;
+		else if(rh != nil){
+			rh->r = q;
+			rh->rgen = v6routegeneration;
+		}
 	}
-	
+	runlock(&routelock);
 	return q;
 }