code: plan9front

Info  •  Files  •  Log

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;

 	}