kernel: fix Mheadache

there was a race between cunmount() and walk() on Mhead.from as Mhead.from was
unconditionally freed when we cunmount(), but findmount might have already
returned the Mhead in walk(). we have to ensure that Mhead.from is not freed
before the Mhead itself (now done in putmhead() once the reference count of the
Mhead drops to zero).

the Mhead struct contained two unused locks, removing.

no need to hold Pgrp.ns lock in closegrp() as nobody can get to it (refcount
droped to zero).

avoid cclose() and freemount() while holding Mhead.lock or Pgrp.ns locks as
it might block on a hung up fileserver.

remove the debug prints...

cleanup: use nil for pointers, remove redundant nil checks before putmhead().

1 files changed, 29 insertions, 39 deletions

diff --git a/sys/src/9/port/pgrp.c b/sys/src/9/port/pgrp.c
index eb65c8adf..430052db9 100644
--- a/sys/src/9/port/pgrp.c
+++ b/sys/src/9/port/pgrp.c
@@ -67,29 +67,24 @@ closergrp(Rgrp *r)
 void
 closepgrp(Pgrp *p)
 {
-	Mhead **h, **e, *f, *next;
+	Mhead **h, **e, *f;
+	Mount *m;
 
-	if(decref(p) != 0)
+	if(decref(p))
 		return;
 
-	qlock(&p->debug);
-	wlock(&p->ns);
-	p->pgrpid = -1;
-
 	e = &p->mnthash[MNTHASH];
 	for(h = p->mnthash; h < e; h++) {
-		for(f = *h; f; f = next) {
+		while((f = *h) != nil){
+			*h = f->hash;
 			wlock(&f->lock);
-			cclose(f->from);
-			mountfree(f->mount);
+			m = f->mount;
 			f->mount = nil;
-			next = f->hash;
 			wunlock(&f->lock);
+			mountfree(m);
 			putmhead(f);
 		}
 	}
-	wunlock(&p->ns);
-	qunlock(&p->debug);
 	free(p);
 }
 
@@ -98,12 +93,12 @@ pgrpinsert(Mount **order, Mount *m)
 {
 	Mount *f;
 
-	m->order = 0;
-	if(*order == 0) {
+	m->order = nil;
+	if(*order == nil) {
 		*order = m;
 		return;
 	}
-	for(f = *order; f; f = f->order) {
+	for(f = *order; f != nil; f = f->order) {
 		if(m->mountid < f->mountid) {
 			m->order = f;
 			*order = m;
@@ -125,17 +120,17 @@ pgrpcpy(Pgrp *to, Pgrp *from)
 	Mhead *f, **tom, **l, *mh;
 
 	wlock(&from->ns);
-	order = 0;
+	order = nil;
 	tom = to->mnthash;
 	for(i = 0; i < MNTHASH; i++) {
 		l = tom++;
-		for(f = from->mnthash[i]; f; f = f->hash) {
+		for(f = from->mnthash[i]; f != nil; f = f->hash) {
 			rlock(&f->lock);
 			mh = newmhead(f->from);
 			*l = mh;
 			l = &mh->hash;
 			link = &mh->mount;
-			for(m = f->mount; m; m = m->next) {
+			for(m = f->mount; m != nil; m = m->next) {
 				n = newmount(mh, m->to, m->mflag, m->spec);
 				m->copy = n;
 				pgrpinsert(&order, m);
@@ -148,7 +143,7 @@ pgrpcpy(Pgrp *to, Pgrp *from)
 	/*
 	 * Allocate mount ids in the same sequence as the parent group
 	 */
-	for(m = order; m; m = m->order)
+	for(m = order; m != nil; m = m->order)
 		m->copy->mountid = incref(&mountid);
 	wunlock(&from->ns);
 }
@@ -184,7 +179,7 @@ dupfgrp(Fgrp *f)
 
 	new->maxfd = f->maxfd;
 	for(i = 0; i <= f->maxfd; i++) {
-		if(c = f->fd[i]){
+		if((c = f->fd[i]) != nil){
 			incref(c);
 			new->fd[i] = c;
 		}
@@ -200,10 +195,7 @@ closefgrp(Fgrp *f)
 	int i;
 	Chan *c;
 
-	if(f == 0)
-		return;
-
-	if(decref(f) != 0)
+	if(f == nil || decref(f))
 		return;
 
 	/*
@@ -212,7 +204,7 @@ closefgrp(Fgrp *f)
 	 */
 	up->closingfgrp = f;
 	for(i = 0; i <= f->maxfd; i++)
-		if(c = f->fd[i]){
+		if((c = f->fd[i]) != nil){
 			f->fd[i] = nil;
 			cclose(c);
 		}
@@ -246,7 +238,7 @@ forceclosefgrp(void)
 
 	f = up->closingfgrp;
 	for(i = 0; i <= f->maxfd; i++)
-		if(c = f->fd[i]){
+		if((c = f->fd[i]) != nil){
 			f->fd[i] = nil;
 			ccloseq(c);
 		}
@@ -259,12 +251,12 @@ newmount(Mhead *mh, Chan *to, int flag, char *spec)
 	Mount *m;
 
 	m = smalloc(sizeof(Mount));
-	m->to = to;
 	m->head = mh;
+	m->to = to;
 	incref(to);
 	m->mountid = incref(&mountid);
 	m->mflag = flag;
-	if(spec != 0)
+	if(spec != nil)
 		kstrdup(&m->spec, spec);
 
 	return m;
@@ -275,28 +267,26 @@ mountfree(Mount *m)
 {
 	Mount *f;
 
-	while(m) {
-		f = m->next;
-		cclose(m->to);
-		m->mountid = 0;
-		free(m->spec);
-		free(m);
-		m = f;
+	while((f = m) != nil) {
+		m = m->next;
+		cclose(f->to);
+		free(f->spec);
+		free(f);
 	}
 }
 
 void
 resrcwait(char *reason)
 {
+	static ulong lastwhine;
 	ulong now;
 	char *p;
-	static ulong lastwhine;
 
-	if(up == 0)
+	if(up == nil)
 		panic("resrcwait");
 
 	p = up->psstate;
-	if(reason) {
+	if(reason != nil) {
 		if(waserror()){
 			up->psstate = p;
 			nexterror();
@@ -310,7 +300,7 @@ resrcwait(char *reason)
 		}
 	}
 	tsleep(&up->sleep, return0, 0, 100+nrand(200));
-	if(reason) {
+	if(reason != nil) {
 		up->psstate = p;
 		poperror();
 	}