code: plan9front

Info  •  Files  •  Log

devip: dont hold ifc wlock during medium bind/unbind

Wlock()'ing the ifc causes a deadlock with Medium
bind/unbind as the routine can walk /net, while
ndb/dns or ndb/cs are currently blocked enumerating
/net/ipifc/*.

The fix is to have a fake medium, called "unbound",
that is set temporarily during the call of Medium
bind and unbind.

That way, the interface rwlock can be released while
bind/unbind is in progress.

The ipifcunbind() routine will refuse to unbind a
ifc that is currently assigned to the "unbound"
medium, preventing any accidents.

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

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

@@ -107,7 +107,6 @@

 /*

  *  called to bind an IP ifc to an ethernet device

- *  called with ifc wlock'd

  */

 static void

 etherbind(Ipifc *ifc, int argc, char **argv)

@@ -200,9 +199,6 @@

 	kproc("recvarpproc", recvarpproc, ifc);

 }

-/*

- *  called with ifc wlock'd

- */

 static void

 etherunbind(Ipifc *ifc)

 {

@@ -210,7 +206,6 @@

 	while(waserror())

 		;

-

 	/* wait for readers to start */

 	while(er->arpp == (void*)-1 || er->read4p == (void*)-1 || er->read6p == (void*)-1)

 		tsleep(&up->sleep, return0, 0, 300);

@@ -221,19 +216,14 @@

 		postnote(er->read6p, 1, "unbind", 0);

 	if(er->arpp != nil)

 		postnote(er->arpp, 1, "unbind", 0);

-

 	poperror();

-	wunlock(ifc);

 	while(waserror())

 		;

-

 	/* wait for readers to die */

 	while(er->arpp != nil || er->read4p != nil || er->read6p != nil)

 		tsleep(&up->sleep, return0, 0, 300);

-

 	poperror();

-	wlock(ifc);

 	if(er->mchan4 != nil)

 		cclose(er->mchan4);

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

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

@@ -129,6 +129,9 @@

 	return *mp;

 }

+/* same as nullmedium, to prevent unbind while bind or unbind is in progress */

+extern Medium unboundmedium;

+

 /*

  *  attach a device (or pkt driver) to the interface.

  *  called with c locked

@@ -154,14 +157,20 @@

 		wunlock(ifc);

 		return Ebound;

 	}

+	ifc->m = &unboundmedium;	/* fake until bound */

+	ifc->arg = nil;

+	wunlock(ifc);

+

 	if(waserror()){

+		wlock(ifc);

+		ifc->m = nil;

 		wunlock(ifc);

 		nexterror();

 	}

-

-	/* do medium specific binding */

 	(*m->bind)(ifc, argc, argv);

+	poperror();

+	wlock(ifc);

 	/* set the bound device name */

 	if(argc > 2)

 		strncpy(ifc->dev, argv[2], sizeof(ifc->dev));

@@ -188,9 +197,7 @@

 	qreopen(c->rq);

 	qreopen(c->eq);

 	qreopen(c->sq);

-

 	wunlock(ifc);

-	poperror();

 	return nil;

 }

@@ -205,34 +212,28 @@

 	wlock(ifc);

 	m = ifc->m;

-	if(m == nil){

+	if(m == nil || m == &unboundmedium){

 		wunlock(ifc);

 		return Eunbound;

 	}

-	/* disassociate logical interfaces (before zeroing ifc->arg) */

+	/* disassociate logical interfaces */

 	while(ifc->lifc != nil)

 		ipifcremlifc(ifc, &ifc->lifc);

+

 	/* disassociate device */

 	if(m->unbind != nil){

-		extern Medium nullmedium;

-

-		/*

-		 * unbind() might unlock the ifc, so change the medium

-		 * to the nullmedium to prevent packets from getting

-		 * sent while the medium is shutting down.

-		 */

-		ifc->m = &nullmedium;

-

+		ifc->m = &unboundmedium;	/* fake until unbound */

+		wunlock(ifc);

 		if(!waserror()){

 			(*m->unbind)(ifc);

 			poperror();

 		}

+		wlock(ifc);

 	}

 	memset(ifc->dev, 0, sizeof(ifc->dev));

-	ifc->arg = nil;

 	ifc->reflect = 0;

 	ifc->reassemble = 0;

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

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

@@ -44,7 +44,6 @@

 	while(waserror())

 		;

-

 	/* wat for reader to start */

 	while(lb->readp == (void*)-1)

 		tsleep(&up->sleep, return0, 0, 300);

@@ -51,19 +50,14 @@

 	if(lb->readp != nil)

 		postnote(lb->readp, 1, "unbind", 0);

-

 	poperror();

-	wunlock(ifc);

 	while(waserror())

 		;

-

 	/* wait for reader to die */

 	while(lb->readp != nil)

 		tsleep(&up->sleep, return0, 0, 300);

-

 	poperror();

-	wlock(ifc);

 	/* clean up */

 	qfree(lb->q);

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

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

@@ -35,7 +35,6 @@

 /*

  *  called to bind an IP ifc to a generic network device

- *  called with ifc qlock'd

  */

 static void

 netdevbind(Ipifc *ifc, int argc, char **argv)

@@ -58,9 +57,6 @@

 	kproc("netdevread", netdevread, ifc);

 }

-/*

- *  called with ifc wlock'd

- */

 static void

 netdevunbind(Ipifc *ifc)

 {

@@ -68,7 +64,6 @@

 	while(waserror())

 		;

-

 	/* wait for reader to start */

 	while(er->readp == (void*)-1)

 		tsleep(&up->sleep, return0, 0, 300);

@@ -75,19 +70,14 @@

 	if(er->readp != nil)

 		postnote(er->readp, 1, "unbind", 0);

-

 	poperror();

-	wunlock(ifc);

 	while(waserror())

 		;

-

 	/* wait for reader to die */

 	while(er->readp != nil)

 		tsleep(&up->sleep, return0, 0, 300);

-

 	poperror();

-	wlock(ifc);

 	if(er->mchan != nil)

 		cclose(er->mchan);

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

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

@@ -33,6 +33,15 @@

 .bwrite=	nullbwrite,

 };

+/* used in ipifc to prevent unbind while bind is in progress */

+Medium unboundmedium =

+{

+.name=		"unbound",

+.bind=		nullbind,

+.unbind=	nullunbind,

+.bwrite=	nullbwrite,

+};

+

 void

 nullmediumlink(void)

 {

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

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

@@ -29,7 +29,6 @@

 /*

  *  called to bind an IP ifc to an packet device

- *  called with ifc wlock'd

  */

 static void

 pktbind(Ipifc*, int argc, char **argv)

@@ -37,9 +36,6 @@

 	USED(argc, argv);

 }

-/*

- *  called with ifc wlock'd

- */

 static void

 pktunbind(Ipifc*)

 {