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.
This commit is contained in:
cinap_lenrek 2022-02-16 22:31:31 +00:00
parent 755880b19f
commit 7289f371a0
6 changed files with 26 additions and 46 deletions

View file

@ -107,7 +107,6 @@ static char *nbmsg = "nonblocking";
/* /*
* called to bind an IP ifc to an ethernet device * called to bind an IP ifc to an ethernet device
* called with ifc wlock'd
*/ */
static void static void
etherbind(Ipifc *ifc, int argc, char **argv) etherbind(Ipifc *ifc, int argc, char **argv)
@ -200,9 +199,6 @@ etherbind(Ipifc *ifc, int argc, char **argv)
kproc("recvarpproc", recvarpproc, ifc); kproc("recvarpproc", recvarpproc, ifc);
} }
/*
* called with ifc wlock'd
*/
static void static void
etherunbind(Ipifc *ifc) etherunbind(Ipifc *ifc)
{ {
@ -210,7 +206,6 @@ etherunbind(Ipifc *ifc)
while(waserror()) while(waserror())
; ;
/* wait for readers to start */ /* wait for readers to start */
while(er->arpp == (void*)-1 || er->read4p == (void*)-1 || er->read6p == (void*)-1) while(er->arpp == (void*)-1 || er->read4p == (void*)-1 || er->read6p == (void*)-1)
tsleep(&up->sleep, return0, 0, 300); tsleep(&up->sleep, return0, 0, 300);
@ -221,19 +216,14 @@ etherunbind(Ipifc *ifc)
postnote(er->read6p, 1, "unbind", 0); postnote(er->read6p, 1, "unbind", 0);
if(er->arpp != nil) if(er->arpp != nil)
postnote(er->arpp, 1, "unbind", 0); postnote(er->arpp, 1, "unbind", 0);
poperror(); poperror();
wunlock(ifc);
while(waserror()) while(waserror())
; ;
/* wait for readers to die */ /* wait for readers to die */
while(er->arpp != nil || er->read4p != nil || er->read6p != nil) while(er->arpp != nil || er->read4p != nil || er->read6p != nil)
tsleep(&up->sleep, return0, 0, 300); tsleep(&up->sleep, return0, 0, 300);
poperror(); poperror();
wlock(ifc);
if(er->mchan4 != nil) if(er->mchan4 != nil)
cclose(er->mchan4); cclose(er->mchan4);

View file

@ -129,6 +129,9 @@ ipfindmedium(char *name)
return *mp; 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. * attach a device (or pkt driver) to the interface.
* called with c locked * called with c locked
@ -154,14 +157,20 @@ ipifcbind(Conv *c, char **argv, int argc)
wunlock(ifc); wunlock(ifc);
return Ebound; return Ebound;
} }
ifc->m = &unboundmedium; /* fake until bound */
ifc->arg = nil;
wunlock(ifc);
if(waserror()){ if(waserror()){
wlock(ifc);
ifc->m = nil;
wunlock(ifc); wunlock(ifc);
nexterror(); nexterror();
} }
/* do medium specific binding */
(*m->bind)(ifc, argc, argv); (*m->bind)(ifc, argc, argv);
poperror();
wlock(ifc);
/* set the bound device name */ /* set the bound device name */
if(argc > 2) if(argc > 2)
strncpy(ifc->dev, argv[2], sizeof(ifc->dev)); strncpy(ifc->dev, argv[2], sizeof(ifc->dev));
@ -188,9 +197,7 @@ ipifcbind(Conv *c, char **argv, int argc)
qreopen(c->rq); qreopen(c->rq);
qreopen(c->eq); qreopen(c->eq);
qreopen(c->sq); qreopen(c->sq);
wunlock(ifc); wunlock(ifc);
poperror();
return nil; return nil;
} }
@ -205,34 +212,28 @@ ipifcunbind(Ipifc *ifc)
wlock(ifc); wlock(ifc);
m = ifc->m; m = ifc->m;
if(m == nil){ if(m == nil || m == &unboundmedium){
wunlock(ifc); wunlock(ifc);
return Eunbound; return Eunbound;
} }
/* disassociate logical interfaces (before zeroing ifc->arg) */ /* disassociate logical interfaces */
while(ifc->lifc != nil) while(ifc->lifc != nil)
ipifcremlifc(ifc, &ifc->lifc); ipifcremlifc(ifc, &ifc->lifc);
/* disassociate device */ /* disassociate device */
if(m->unbind != nil){ if(m->unbind != nil){
extern Medium nullmedium; ifc->m = &unboundmedium; /* fake until unbound */
wunlock(ifc);
/*
* 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;
if(!waserror()){ if(!waserror()){
(*m->unbind)(ifc); (*m->unbind)(ifc);
poperror(); poperror();
} }
wlock(ifc);
} }
memset(ifc->dev, 0, sizeof(ifc->dev)); memset(ifc->dev, 0, sizeof(ifc->dev));
ifc->arg = nil;
ifc->reflect = 0; ifc->reflect = 0;
ifc->reassemble = 0; ifc->reassemble = 0;

View file

@ -44,26 +44,20 @@ loopbackunbind(Ipifc *ifc)
while(waserror()) while(waserror())
; ;
/* wat for reader to start */ /* wat for reader to start */
while(lb->readp == (void*)-1) while(lb->readp == (void*)-1)
tsleep(&up->sleep, return0, 0, 300); tsleep(&up->sleep, return0, 0, 300);
if(lb->readp != nil) if(lb->readp != nil)
postnote(lb->readp, 1, "unbind", 0); postnote(lb->readp, 1, "unbind", 0);
poperror(); poperror();
wunlock(ifc);
while(waserror()) while(waserror())
; ;
/* wait for reader to die */ /* wait for reader to die */
while(lb->readp != nil) while(lb->readp != nil)
tsleep(&up->sleep, return0, 0, 300); tsleep(&up->sleep, return0, 0, 300);
poperror(); poperror();
wlock(ifc);
/* clean up */ /* clean up */
qfree(lb->q); qfree(lb->q);

View file

@ -35,7 +35,6 @@ Medium netdevmedium =
/* /*
* called to bind an IP ifc to a generic network device * called to bind an IP ifc to a generic network device
* called with ifc qlock'd
*/ */
static void static void
netdevbind(Ipifc *ifc, int argc, char **argv) netdevbind(Ipifc *ifc, int argc, char **argv)
@ -58,9 +57,6 @@ netdevbind(Ipifc *ifc, int argc, char **argv)
kproc("netdevread", netdevread, ifc); kproc("netdevread", netdevread, ifc);
} }
/*
* called with ifc wlock'd
*/
static void static void
netdevunbind(Ipifc *ifc) netdevunbind(Ipifc *ifc)
{ {
@ -68,26 +64,20 @@ netdevunbind(Ipifc *ifc)
while(waserror()) while(waserror())
; ;
/* wait for reader to start */ /* wait for reader to start */
while(er->readp == (void*)-1) while(er->readp == (void*)-1)
tsleep(&up->sleep, return0, 0, 300); tsleep(&up->sleep, return0, 0, 300);
if(er->readp != nil) if(er->readp != nil)
postnote(er->readp, 1, "unbind", 0); postnote(er->readp, 1, "unbind", 0);
poperror(); poperror();
wunlock(ifc);
while(waserror()) while(waserror())
; ;
/* wait for reader to die */ /* wait for reader to die */
while(er->readp != nil) while(er->readp != nil)
tsleep(&up->sleep, return0, 0, 300); tsleep(&up->sleep, return0, 0, 300);
poperror(); poperror();
wlock(ifc);
if(er->mchan != nil) if(er->mchan != nil)
cclose(er->mchan); cclose(er->mchan);

View file

@ -33,6 +33,15 @@ Medium nullmedium =
.bwrite= nullbwrite, .bwrite= nullbwrite,
}; };
/* used in ipifc to prevent unbind while bind is in progress */
Medium unboundmedium =
{
.name= "unbound",
.bind= nullbind,
.unbind= nullunbind,
.bwrite= nullbwrite,
};
void void
nullmediumlink(void) nullmediumlink(void)
{ {

View file

@ -29,7 +29,6 @@ Medium pktmedium =
/* /*
* called to bind an IP ifc to an packet device * called to bind an IP ifc to an packet device
* called with ifc wlock'd
*/ */
static void static void
pktbind(Ipifc*, int argc, char **argv) pktbind(Ipifc*, int argc, char **argv)
@ -37,9 +36,6 @@ pktbind(Ipifc*, int argc, char **argv)
USED(argc, argv); USED(argc, argv);
} }
/*
* called with ifc wlock'd
*/
static void static void
pktunbind(Ipifc*) pktunbind(Ipifc*)
{ {