devmnt: fix potential race with mntflushfree(), remove mntstats, 0 vs nil cleanup

when mountmux() completes a request for another process, enforce odering
of the loads and stores to the request prior to writing q->done = 1
so mntflushfree() sees q->done != 0 only when the request has actually
completed. otherwise, the q->done = 1 store could have been reordered
before the load from q->z, reading from already freed request and causing
spurious wakeups.

removing unused mntstats callback.

use nil for pointers instead of 0.
This commit is contained in:
cinap_lenrek 2014-08-08 23:28:47 +02:00
parent bc306a5a63
commit 45333cdc92

View file

@ -74,8 +74,6 @@ Chan* mntchan(void);
char Esbadstat[] = "invalid directory entry received from server"; char Esbadstat[] = "invalid directory entry received from server";
char Enoversion[] = "version not established for mount channel"; char Enoversion[] = "version not established for mount channel";
void (*mntstats)(int, Chan*, uvlong, ulong);
static void static void
mntreset(void) mntreset(void)
@ -213,11 +211,11 @@ mntversion(Chan *c, char *version, int msize, int returnlen)
/* now build Mnt associated with this connection */ /* now build Mnt associated with this connection */
lock(&mntalloc); lock(&mntalloc);
m = mntalloc.mntfree; m = mntalloc.mntfree;
if(m != 0) if(m != nil)
mntalloc.mntfree = m->list; mntalloc.mntfree = m->list;
else { else {
m = malloc(sizeof(Mnt)); m = malloc(sizeof(Mnt));
if(m == 0) { if(m == nil) {
qfree(q); qfree(q);
free(v); free(v);
unlock(&mntalloc); unlock(&mntalloc);
@ -239,8 +237,8 @@ mntversion(Chan *c, char *version, int msize, int returnlen)
free(msg); free(msg);
lock(m); lock(m);
m->queue = 0; m->queue = nil;
m->rip = 0; m->rip = nil;
c->flag |= CMSG; c->flag |= CMSG;
c->mux = m; c->mux = m;
@ -587,7 +585,7 @@ muxclose(Mnt *m)
lock(&mntalloc); lock(&mntalloc);
l = &mntalloc.list; l = &mntalloc.list;
for(f = *l; f; f = f->list) { for(f = *l; f != nil; f = f->list) {
if(f == m) { if(f == m) {
*l = m->list; *l = m->list;
break; break;
@ -815,7 +813,7 @@ mountio(Mnt *m, Mntrpc *r)
/* Gate readers onto the mount point one at a time */ /* Gate readers onto the mount point one at a time */
for(;;) { for(;;) {
lock(m); lock(m);
if(m->rip == 0) if(m->rip == nil)
break; break;
unlock(m); unlock(m);
sleep(r->z, rpcattn, r); sleep(r->z, rpcattn, r);
@ -933,8 +931,8 @@ mntgate(Mnt *m)
Mntrpc *q; Mntrpc *q;
lock(m); lock(m);
m->rip = 0; m->rip = nil;
for(q = m->queue; q; q = q->list) { for(q = m->queue; q != nil; q = q->list) {
if(q->done == 0) if(q->done == 0)
if(wakeup(q->z)) if(wakeup(q->z))
break; break;
@ -950,28 +948,26 @@ mountmux(Mnt *m, Mntrpc *r)
lock(m); lock(m);
l = &m->queue; l = &m->queue;
for(q = *l; q; q = q->list) { for(q = *l; q != nil; q = q->list) {
/* look for a reply to a message */ /* look for a reply to a message */
if(q->request.tag == r->reply.tag) { if(q->request.tag == r->reply.tag) {
*l = q->list; *l = q->list;
if(mntstats != nil) if(q == r) {
(*mntstats)(q->request.type, q->done = 1;
m->c, q->stime, unlock(m);
q->reqlen + r->replen); return;
if(q != r) { }
/* /*
* Completed someone else. * Completed someone else.
* Trade pointers to receive buffer. * Trade pointers to receive buffer.
*/ */
q->reply = r->reply; q->reply = r->reply;
q->b = r->b; q->b = r->b;
r->b = nil; r->b = nil;
z = q->z; z = q->z;
} else coherence();
z = nil; q->done = 1;
q->done = 1; /* hands off */ wakeup(z);
if(z != nil)
wakeup(z);
unlock(m); unlock(m);
return; return;
} }
@ -1014,7 +1010,7 @@ mntflushfree(Mnt *m, Mntrpc *r)
{ {
Mntrpc *fr; Mntrpc *fr;
while(r){ while(r != nil){
fr = r->flushed; fr = r->flushed;
if(!r->done){ if(!r->done){
r->reply.type = Rflush; r->reply.type = Rflush;
@ -1133,7 +1129,7 @@ mntqrm(Mnt *m, Mntrpc *r)
r->done = 1; r->done = 1;
l = &m->queue; l = &m->queue;
for(f = *l; f; f = f->list) { for(f = *l; f != nil; f = f->list) {
if(f == r) { if(f == r) {
*l = r->list; *l = r->list;
break; break;
@ -1190,7 +1186,7 @@ rpcattn(void *v)
Mntrpc *r; Mntrpc *r;
r = v; r = v;
return r->done || r->m->rip == 0; return r->done || r->m->rip == nil;
} }
Dev mntdevtab = { Dev mntdevtab = {