shithub: riscv

Download patch

ref: f39abb2923ac5faf55ce283e925671d639f3b9b9
parent: da308716c73a2b06f861e7535df425ca0dd62ee1
author: cinap_lenrek <cinap_lenrek@felloff.net>
date: Sun Sep 17 16:12:20 EDT 2023 

devip: run Medium.unbind() with ifc->conv released, cleanup

Medium.unbind() must run with ifc->conv unlocked
as mediumunbindifc() holds it while determining
if it should also unbind causing a potential
deadlock.

Note that ipifcnind() and Medium.bind() is run
with ifc->conv locked, delaying mediumunbindifc()
after ipifcbind() completes.


--- a/sys/src/9/ip/ipifc.c
+++ b/sys/src/9/ip/ipifc.c
@@ -123,7 +123,6 @@
 
 /* same as nullmedium, to prevent unbind while bind or unbind is in progress */
 extern Medium unboundmedium;
-static char *ipifcunbindmedium(Ipifc *ifc, Medium *m);
 
 /*
  *  attach a device (or pkt driver) to the interface.
@@ -151,26 +150,7 @@
 	}
 	ifc->m = &unboundmedium;	/* fake until bound */
 	ifc->arg = nil;
-	if(m->unbindonclose == 0)
-		c->inuse++;
-	snprint(ifc->dev, sizeof ifc->dev, "%s%d", ifc->m->name, c->x);
-	wunlock(ifc);
 
-	if(waserror()){
-		wlock(ifc);
-		if(m->unbindonclose == 0)
-			c->inuse--;
-		ifc->m = nil;
-		wunlock(ifc);
-		nexterror();
-	}
-	(*m->bind)(ifc, argc, argv);
-	poperror();
-
-	wlock(ifc);
-	/* switch to the real medium */
-	ifc->m = m;
-
 	/* set the bound device name */
 	if(argc > 2)
 		strncpy(ifc->dev, argv[2], sizeof(ifc->dev));
@@ -185,7 +165,23 @@
 
 	/* default router paramters */
 	ifc->rp = c->p->f->v6p->rp;
+	wunlock(ifc);
 
+	if(waserror()){
+		wlock(ifc);
+		ifc->m = nil;
+		wunlock(ifc);
+		nexterror();
+	}
+	(*m->bind)(ifc, argc, argv);
+	poperror();
+
+	wlock(ifc);
+	/* switch to real medium */
+	ifc->m = m;
+	if(m->unbindonclose == 0)
+		c->inuse++;
+
 	/* any ancillary structures (like routes) no longer pertain */
 	ifc->ifcid++;
 
@@ -216,10 +212,12 @@
 	if(m->unbind != nil){
 		ifc->m = &unboundmedium;	/* fake until unbound */
 		wunlock(ifc);
+		qunlock(ifc->conv);
 		if(!waserror()){
 			(*m->unbind)(ifc);
 			poperror();
 		}
+		qlock(ifc->conv);
 		wlock(ifc);
 	}
 
@@ -269,35 +267,21 @@
 mediumunbindifc(Ipifc *ifc)
 {
 	Medium *m;
-	Conv *conv;
 	char *err;
 
-	err = Eunbound;
-
 	rlock(ifc);
 	m = ifc->m;
-	if(m == &unboundmedium){
-		runlock(ifc);
-		return err;
-	}
-	conv = ifc->conv;
 	runlock(ifc);
 
-	assert(conv != nil);
-	assert(m != nil);
-	assert(m->unbindonclose == 0);
-
-	qlock(conv);
-
-	assert(conv->inuse > 0);
-	assert((Ipifc*)conv->ptcl == ifc);
-
-	wlock(ifc);
-	if(ifc->m == m)
-		err = ipifcunbindmedium(ifc, m);
-	wunlock(ifc);
-	qunlock(conv);
-
+	err = Eunbound;
+	if(m != nil && m != &unboundmedium){
+		qlock(ifc->conv);
+		wlock(ifc);
+		if(ifc->m == m)
+			err = ipifcunbindmedium(ifc, m);
+		wunlock(ifc);
+		qunlock(ifc->conv);
+	}
 	return err;
 }
 
@@ -559,7 +543,7 @@
 	}
 
 	wlock(ifc);
-	if(ifc->m == nil){
+	if(ifc->m == nil || ifc->m == &unboundmedium){
 		wunlock(ifc);
 		return Eunbound;
 	}
@@ -908,7 +892,7 @@
 		err = ipifcremove6(ifc, argv, argc);
 	else {
 		wlock(ifc);
-		if(ifc->m == nil){
+		if(ifc->m == nil || ifc->m == &unboundmedium){
 			wunlock(ifc);
 			return Eunbound;
 		}
@@ -1697,7 +1681,7 @@
 		if(nifc == ifc || !canrlock(nifc))
 			continue;
 
-		if(nifc->m == nil
+		if(nifc->m == nil || nifc->m == &unboundmedium
 		|| (lifc = ipremoteonifc(nifc, ip)) == nil
 		|| (lifc->type & Rptpt) != 0
 		|| waserror()){
@@ -1760,7 +1744,7 @@
 		return Ebadarg;
 
 	rlock(ifc);
-	if(ifc->m == nil){
+	if(ifc->m == nil || ifc->m == &unboundmedium){
 		runlock(ifc);
 		return Eunbound;
 	}