From a25819c43a65b5abd44f42f502718e47fffc6923 Mon Sep 17 00:00:00 2001 From: cinap_lenrek Date: Sat, 11 May 2019 07:22:34 +0200 Subject: [PATCH] devip: avoid media bind/unbind kproc reader startup race, simplify etherbind mark reader process pointers with (void*)-1 to mean not started yet. this avoids the race condition when media unbind happens before the kproc has set its Proc* pointer. then we would not post the note and the reader would continue running after unbind. etherbind can be simplified by reading the #lX/addr file to get the mac address, avoiding the temporary buffer. --- sys/src/9/ip/ethermedium.c | 113 ++++++++++++++++------------------ sys/src/9/ip/loopbackmedium.c | 9 ++- sys/src/9/ip/netdevmedium.c | 7 ++- 3 files changed, 65 insertions(+), 64 deletions(-) diff --git a/sys/src/9/ip/ethermedium.c b/sys/src/9/ip/ethermedium.c index 3602da1b8..41928e89f 100644 --- a/sys/src/9/ip/ethermedium.c +++ b/sys/src/9/ip/ethermedium.c @@ -122,32 +122,49 @@ static char *nbmsg = "nonblocking"; static void etherbind(Ipifc *ifc, int argc, char **argv) { - Chan *mchan4, *cchan4, *achan, *mchan6, *cchan6, *schan; - char addr[Maxpath]; //char addr[2*KNAMELEN]; - char dir[Maxpath]; //char dir[2*KNAMELEN]; - char *buf; - int n; - char *ptr; + char addr[Maxpath], dir[Maxpath]; Etherrock *er; + Chan *c; + int n; if(argc < 2) error(Ebadarg); - mchan4 = cchan4 = achan = mchan6 = cchan6 = nil; - buf = nil; + /* + * get mac address + */ + snprint(addr, sizeof(addr), "%s/addr", argv[2]); + c = namec(addr, Aopen, OREAD, 0); if(waserror()){ - if(mchan4 != nil) - cclose(mchan4); - if(cchan4 != nil) - cclose(cchan4); - if(achan != nil) - cclose(achan); - if(mchan6 != nil) - cclose(mchan6); - if(cchan6 != nil) - cclose(cchan6); - if(buf != nil) - free(buf); + cclose(c); + nexterror(); + } + n = devtab[c->type]->read(c, addr, sizeof(addr)-1, 0); + if(n < 0) + error(Eio); + addr[n] = 0; + if(parsemac(ifc->mac, addr, sizeof(ifc->mac)) != 6) + error("could not find mac address"); + cclose(c); + poperror(); + + er = smalloc(sizeof(*er)); + er->read4p = er->read6p = er->arpp = (void*)-1; + er->mchan4 = er->cchan4 = er->mchan6 = er->cchan6 = er->achan = nil; + er->f = ifc->conv->p->f; + + if(waserror()){ + if(er->mchan4 != nil) + cclose(er->mchan4); + if(er->cchan4 != nil) + cclose(er->cchan4); + if(er->mchan6 != nil) + cclose(er->mchan6); + if(er->cchan6 != nil) + cclose(er->cchan6); + if(er->achan != nil) + cclose(er->achan); + free(er); nexterror(); } @@ -158,39 +175,12 @@ etherbind(Ipifc *ifc, int argc, char **argv) * this device. */ snprint(addr, sizeof(addr), "%s!0x800", argv[2]); /* ETIP4 */ - mchan4 = chandial(addr, nil, dir, &cchan4); + er->mchan4 = chandial(addr, nil, dir, &er->cchan4); /* * make it non-blocking */ - devtab[cchan4->type]->write(cchan4, nbmsg, strlen(nbmsg), 0); - - /* - * get mac address and speed - */ - snprint(addr, sizeof(addr), "%s/stats", argv[2]); - buf = smalloc(512); - schan = namec(addr, Aopen, OREAD, 0); - if(waserror()){ - cclose(schan); - nexterror(); - } - n = devtab[schan->type]->read(schan, buf, 511, 0); - cclose(schan); - poperror(); - buf[n] = 0; - - ptr = strstr(buf, "addr: "); - if(!ptr) - error(Eio); - ptr += 6; - parsemac(ifc->mac, ptr, 6); - - /* - * open arp conversation - */ - snprint(addr, sizeof(addr), "%s!0x806", argv[2]); /* ETARP */ - achan = chandial(addr, nil, nil, nil); + devtab[er->cchan4->type]->write(er->cchan4, nbmsg, strlen(nbmsg), 0); /* * open ipv6 conversation @@ -199,25 +189,22 @@ etherbind(Ipifc *ifc, int argc, char **argv) * this device. */ snprint(addr, sizeof(addr), "%s!0x86DD", argv[2]); /* ETIP6 */ - mchan6 = chandial(addr, nil, dir, &cchan6); + er->mchan6 = chandial(addr, nil, dir, &er->cchan6); /* * make it non-blocking */ - devtab[cchan6->type]->write(cchan6, nbmsg, strlen(nbmsg), 0); + devtab[er->cchan6->type]->write(er->cchan6, nbmsg, strlen(nbmsg), 0); - er = smalloc(sizeof(*er)); - er->mchan4 = mchan4; - er->cchan4 = cchan4; - er->achan = achan; - er->mchan6 = mchan6; - er->cchan6 = cchan6; - er->f = ifc->conv->p->f; - ifc->arg = er; - - free(buf); + /* + * open arp conversation + */ + snprint(addr, sizeof(addr), "%s!0x806", argv[2]); /* ETARP */ + er->achan = chandial(addr, nil, nil, nil); poperror(); + ifc->arg = er; + kproc("etherread4", etherread4, ifc); kproc("etherread6", etherread6, ifc); kproc("recvarpproc", recvarpproc, ifc); @@ -231,6 +218,10 @@ etherunbind(Ipifc *ifc) { Etherrock *er = ifc->arg; + /* wait for readers to start */ + while(er->arpp == (void*)-1 || er->read4p == (void*)-1 || er->read6p == (void*)-1) + tsleep(&up->sleep, return0, 0, 300); + if(er->read4p != nil) postnote(er->read4p, 1, "unbind", 0); if(er->read6p != nil) diff --git a/sys/src/9/ip/loopbackmedium.c b/sys/src/9/ip/loopbackmedium.c index c66c31092..26389f390 100644 --- a/sys/src/9/ip/loopbackmedium.c +++ b/sys/src/9/ip/loopbackmedium.c @@ -28,6 +28,7 @@ loopbackbind(Ipifc *ifc, int, char**) LB *lb; lb = smalloc(sizeof(*lb)); + lb->readp = (void*)-1; lb->f = ifc->conv->p->f; lb->q = qopen(1024*1024, Qmsg, nil, nil); ifc->arg = lb; @@ -41,11 +42,15 @@ loopbackunbind(Ipifc *ifc) { LB *lb = ifc->arg; - if(lb->readp) + /* 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); /* wait for reader to die */ - while(lb->readp != 0) + while(lb->readp != nil) tsleep(&up->sleep, return0, 0, 300); /* clean up */ diff --git a/sys/src/9/ip/netdevmedium.c b/sys/src/9/ip/netdevmedium.c index 25111fd41..f3d9ac1a4 100644 --- a/sys/src/9/ip/netdevmedium.c +++ b/sys/src/9/ip/netdevmedium.c @@ -49,6 +49,7 @@ netdevbind(Ipifc *ifc, int argc, char **argv) mchan = namec(argv[2], Aopen, ORDWR, 0); er = smalloc(sizeof(*er)); + er->readp = (void*)-1; er->mchan = mchan; er->f = ifc->conv->p->f; @@ -65,10 +66,14 @@ netdevunbind(Ipifc *ifc) { Netdevrock *er = ifc->arg; + /* 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); - /* wait for readers to die */ + /* wait for reader to die */ while(er->readp != nil) tsleep(&up->sleep, return0, 0, 300);