From 3af236b5e36951ba637d59e4d0362cdb0e428bd2 Mon Sep 17 00:00:00 2001
From: cinap_lenrek <cinap_lenrek@felloff.net>
Date: Sun, 9 Aug 2015 18:19:47 +0200
Subject: 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().
---
 sys/src/9/port/pgrp.c | 68 ++++++++++++++++++++++-----------------------------
 1 file changed, 29 insertions(+), 39 deletions(-)

(limited to 'sys/src/9/port/pgrp.c')

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();
 	}
-- 
cgit v1.2.3