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.

6 files changed, 26 insertions, 46 deletions

diff --git a/sys/src/9/ip/ethermedium.c b/sys/src/9/ip/ethermedium.c
index fb6d05279..8a2a50899 100644
--- a/sys/src/9/ip/ethermedium.c
+++ b/sys/src/9/ip/ethermedium.c
@@ -107,7 +107,6 @@ static char *nbmsg = "nonblocking";
 
 /*
  *  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 @@ etherbind(Ipifc *ifc, int argc, char **argv)
 	kproc("recvarpproc", recvarpproc, ifc);
 }
 
-/*
- *  called with ifc wlock'd
- */
 static void
 etherunbind(Ipifc *ifc)
 {
@@ -210,7 +206,6 @@ etherunbind(Ipifc *ifc)
 
 	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 @@ etherunbind(Ipifc *ifc)
 		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);
diff --git a/sys/src/9/ip/ipifc.c b/sys/src/9/ip/ipifc.c
index b125ddf0b..4f6272bf3 100644
--- a/sys/src/9/ip/ipifc.c
+++ b/sys/src/9/ip/ipifc.c
@@ -129,6 +129,9 @@ ipfindmedium(char *name)
 	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 @@ ipifcbind(Conv *c, char **argv, int argc)
 		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 @@ ipifcbind(Conv *c, char **argv, int argc)
 	qreopen(c->rq);
 	qreopen(c->eq);
 	qreopen(c->sq);
-
 	wunlock(ifc);
-	poperror();
 
 	return nil;
 }
@@ -205,34 +212,28 @@ ipifcunbind(Ipifc *ifc)
 
 	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;
diff --git a/sys/src/9/ip/loopbackmedium.c b/sys/src/9/ip/loopbackmedium.c
index 036de1137..f1755152b 100644
--- a/sys/src/9/ip/loopbackmedium.c
+++ b/sys/src/9/ip/loopbackmedium.c
@@ -44,26 +44,20 @@ loopbackunbind(Ipifc *ifc)
 
 	while(waserror())
 		;
-
 	/* wat for reader to start */
 	while(lb->readp == (void*)-1)
 		tsleep(&up->sleep, return0, 0, 300);
 		
 	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);
diff --git a/sys/src/9/ip/netdevmedium.c b/sys/src/9/ip/netdevmedium.c
index 9e265d19b..f60d43b3a 100644
--- a/sys/src/9/ip/netdevmedium.c
+++ b/sys/src/9/ip/netdevmedium.c
@@ -35,7 +35,6 @@ Medium netdevmedium =
 
 /*
  *  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 @@ netdevbind(Ipifc *ifc, int argc, char **argv)
 	kproc("netdevread", netdevread, ifc);
 }
 
-/*
- *  called with ifc wlock'd
- */
 static void
 netdevunbind(Ipifc *ifc)
 {
@@ -68,26 +64,20 @@ netdevunbind(Ipifc *ifc)
 
 	while(waserror())
 		;
-
 	/* wait for reader to start */
 	while(er->readp == (void*)-1)
 		tsleep(&up->sleep, return0, 0, 300);
 
 	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);
diff --git a/sys/src/9/ip/nullmedium.c b/sys/src/9/ip/nullmedium.c
index 094e103f6..3874da583 100644
--- a/sys/src/9/ip/nullmedium.c
+++ b/sys/src/9/ip/nullmedium.c
@@ -33,6 +33,15 @@ Medium nullmedium =
 .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)
 {
diff --git a/sys/src/9/ip/pktmedium.c b/sys/src/9/ip/pktmedium.c
index 15ac6405a..633ba66b2 100644
--- a/sys/src/9/ip/pktmedium.c
+++ b/sys/src/9/ip/pktmedium.c
@@ -29,7 +29,6 @@ Medium pktmedium =
 
 /*
  *  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 @@ pktbind(Ipifc*, int argc, char **argv)
 	USED(argc, argv);
 }
 
-/*
- *  called with ifc wlock'd
- */
 static void
 pktunbind(Ipifc*)
 {