shithub: riscv

Download patch

ref: 4de9d8a511c1403ef849147e82b54af3bdbd4e3e
parent: 2a622dfcec55ff2b791aa01f12d80ce8ec1225f4
author: cinap_lenrek <cinap_lenrek@felloff.net>
date: Sat Jan 4 19:12:16 EST 2025 

gefs: fix waserror() bugs


--- a/sys/src/cmd/gefs/check.c
+++ b/sys/src/cmd/gefs/check.c
@@ -233,6 +233,10 @@
 	while(1){
 		if(!btnext(&s, &s.kv))
 			break;
+		if(waserror()){
+			btexit(&s);
+			nexterror();
+		}
 		bp = unpackbp(s.kv.v, s.kv.nv);
 		if(isfree(bp.addr)){
 			fprint(2, "free block in use: %B\n", bp);
@@ -240,6 +244,7 @@
 		}
 		b = getblk(bp, GBraw);
 		dropblk(b);
+		poperror();
 	}
 	btexit(&s);
 	return 0;
@@ -288,15 +293,27 @@
 		if((t = opensnap(name, nil)) == nil){
 			fprint(2, "invalid snap label %s\n", name);
 			ok = 0;
+			poperror();
 			break;
 		}
+		if(waserror()){
+			closesnap(t);
+			nexterror();
+		}
 		fprint(fd, "checking snap %s: %B\n", name, t->bp);
 		b = getroot(t, &height);
+		if(waserror()){
+			dropblk(b);
+			nexterror();
+		}
 		if(checktree(fd, b, height-1, nil, 0))
 			ok = 0;
 		if(checkdata(fd, t))
 			ok = 0;
 		dropblk(b);
+		poperror();
+		closesnap(t);
+		poperror();
 		poperror();
 	}
 	btexit(&s);
--- a/sys/src/cmd/gefs/fs.c
+++ b/sys/src/cmd/gefs/fs.c
@@ -109,6 +109,10 @@
 	 *  can take a consistent snapshot.
          */
 	qlock(&fs->mutlk);
+	if(waserror()){
+		qunlock(&fs->mutlk);
+		nexterror();
+	}
 	tracem("packb");
 	for(mnt = agetp(&fs->mounts); mnt != nil; mnt = mnt->next)
 		updatesnap(&mnt->root, mnt->root, mnt->name, mnt->flag);
@@ -150,6 +154,7 @@
 	finalize(fs->sb1);
 	fs->snap.dirty = 0;
 	qunlock(&fs->mutlk);
+	poperror();
 
 	/*
 	 * pass 1: sync block headers; if we crash here,
@@ -429,6 +434,7 @@
 	epochend(id);
 	qunlock(&fs->mutlk);
 	qlock(&de->trunclk);
+	/* should we also return when the fs becomes read-only? */
 	while(de->trunc)
 		rsleep(&de->truncrz);
 	qunlock(&de->trunclk);
@@ -495,6 +501,11 @@
 	else
 		seq = 0;
 	b = newdblk(f->mnt->root, f->qpath, seq);
+	if(waserror()){
+		freeblk(f->mnt->root, b);
+		dropblk(b);
+		nexterror();
+	}
 	t = nil;
 	r = f->mnt->root;
 	if(btlookup(r, m, &kv, buf, sizeof(buf))){
@@ -515,10 +526,12 @@
 			memset(b->buf+fo+n, 0, Blksz-fo-n);
 	}
 	enqueue(b);
+	poperror();
 
 	packbp(m->v, m->nv, &b->bp);
 	*ret = b->bp;
 	dropblk(b);
+
 	return n;
 }
 
@@ -537,7 +550,10 @@
 			goto Out;
 		}
 	}
-
+	if(waserror()){
+		unlock(&mnt->dtablk);
+		nexterror();
+	}
 	de = emalloc(sizeof(Dent), 1);
 	de->Xdir = *d;
 	de->ref = 1;
@@ -545,18 +561,13 @@
 	de->qid = d->qid;
 	de->length = d->length;
 	de->truncrz.l = &de->trunclk;
-
-	if((e = packdkey(de->buf, sizeof(de->buf), pqid, d->name)) == nil){
-		free(de);
-		de = nil;
-		goto Out;
-	}
+	e = packdkey(de->buf, sizeof(de->buf), pqid, d->name);
 	de->k = de->buf;
 	de->nk = e - de->buf;
 	de->name = de->buf + 11;
 	de->next = mnt->dtab[h];
 	mnt->dtab[h] = de;
-
+	poperror();
 Out:
 	unlock(&mnt->dtablk);
 	return de;
@@ -626,9 +637,10 @@
 			goto Out;
 		}
 	}
-
-	if((mnt = mallocz(sizeof(*mnt), 1)) == nil)
+	if((mnt = mallocz(sizeof(*mnt), 1)) == nil){
+		qunlock(&fs->mountlk);
 		error(Enomem);
+	}
 	if(waserror()){
 		qunlock(&fs->mountlk);
 		free(mnt);
@@ -774,6 +786,7 @@
  * clunkfid() removes a fid from the
  * connection fid tab and drops reference.
  * Fid must be locked.
+ * Must not raise error().
  */
 static void
 clunkfid(Conn *c, Fid *fid, Amsg **ao)
@@ -875,6 +888,7 @@
 	return 0;
 }
 
+/* fsversion: must not raise error() */
 static void
 fsversion(Fmsg *m)
 {
@@ -901,14 +915,12 @@
 }
 
 static void
-authfree(AuthRpc *auth)
+authfree(AuthRpc *rpc)
 {
-	AuthRpc *rpc;
-
-	if(rpc = auth){
-		close(rpc->afd);
-		auth_freerpc(rpc);
-	}
+	if(rpc == nil)
+		return;
+	close(rpc->afd);
+	auth_freerpc(rpc);
 }
 
 AuthRpc*
@@ -986,6 +998,7 @@
 
 }
 
+/* fsauth: must not raise error() */
 static void
 fsauth(Fmsg *m)
 {
@@ -1005,10 +1018,10 @@
 		rerror(m, Enomem);
 		return;
 	}
-	memset(de, 0, sizeof(Dent));
 	de->auth = authnew();
 	if(de->auth == nil){
-		rerror(m, errmsg());
+		rerror(m, "%r");
+		free(de);
 		return;
 	}
 	de->ref = 0;
@@ -1034,15 +1047,15 @@
 	f.dmode = 0600;
 	nf = dupfid(m->conn, m->afid, &f);
 	if(nf == nil){
-		rerror(m, Efid);
 		authfree(de->auth);
 		free(de);
+		rerror(m, Efid);
 		return;
 	}
+	putfid(nf);
 	r.type = Rauth;
 	r.aqid = de->qid;
 	respond(m, &r);
-	putfid(nf);
 }
 
 static int
@@ -1145,12 +1158,6 @@
 	Fid f, *af, *nf;
 	int uid;
 
-	de = nil;
-	mnt = nil;
-	if(waserror()){
-		rerror(m, errmsg());
-		goto Err;
-	}
 	aname = m->aname;
 	if(aname[0] == '%')
 		aname++;
@@ -1158,6 +1165,10 @@
 		aname = "main";
 	if((mnt = getmount(aname)) == nil)
 		error(Enosnap);
+	if(waserror()){
+		clunkmount(mnt);
+		nexterror();
+	}
 
 	rlock(&fs->userlk);
 	n = m->uname;
@@ -1179,8 +1190,13 @@
 		r.count = 0;
 		if((af = getfid(m->conn, m->afid)) == nil)
 			error(Enofid);
+		if(waserror()){
+			putfid(af);
+			nexterror();
+		}
 		authread(af, &r, nil, 0);
 		putfid(af);
+		poperror();
 		if(af->uid != uid)
 			error(Ebadu);
 		m->conn->authok = 1;	/* none attach allowed now */
@@ -1195,8 +1211,7 @@
 		memset(&d, 0, sizeof(d));
 		filldumpdir(&d);
 	}else{
-		if((p = packdkey(dbuf, sizeof(dbuf), -1ULL, "")) == nil)
-			error(Elength);
+		p = packdkey(dbuf, sizeof(dbuf), -1ULL, "");
 		dk.k = dbuf;
 		dk.nk = p - dbuf;
 		t = agetp(&mnt->root);
@@ -1205,6 +1220,10 @@
 		kv2dir(&kv, &d);
 	}
 	de = getdent(mnt, -1, &d);
+	if(waserror()){
+		clunkdent(mnt, de);
+		nexterror();
+	}
 	memset(&f, 0, sizeof(Fid));
 	f.fid = NOFID;
 	f.mnt = mnt;
@@ -1225,18 +1244,20 @@
 	}
 	if(strcmp(aname, "dump") == 0)
 		f.fromdump = 1;
+
 	nf = dupfid(m->conn, m->fid, &f);
 	if(nf == nil)
 		error(Efid);
-	r.type = Rattach;
-	r.qid = d.qid;
-	respond(m, &r);
 	putfid(nf);
-	poperror();
 
-
-Err:	clunkdent(mnt, de);
+	clunkdent(mnt, de);
+	poperror();
 	clunkmount(mnt);
+	poperror();
+
+	r.type = Rattach;
+	r.qid = d.qid;
+	respond(m, &r);
 }
 
 static int
@@ -1269,7 +1290,7 @@
 fswalk(Fmsg *m)
 {
 	char *name, kbuf[Maxent], kvbuf[Kvmax];
-	int duid, dgid, dmode, duped;
+	int duid, dgid, dmode;
 	vlong up, upup, prev;
 	Dent *dent, *dir;
 	Fid *o, *f;
@@ -1282,16 +1303,13 @@
 	Key k;
 	int i;
 
-	if((o = getfid(m->conn, m->fid)) == nil){
-		rerror(m, Enofid);
-		return;
-	}
+	if((o = getfid(m->conn, m->fid)) == nil)
+		error(Enofid);
 	rlock(o);
 	if(waserror()){
-		rerror(m, errmsg());
 		runlock(o);
 		putfid(o);
-		return;
+		nexterror();
 	}
 	if(o->mode != -1)
 		error(Einuse);
@@ -1323,7 +1341,7 @@
 			}
 			findparent(t, up, &prev, &name, kbuf, sizeof(kbuf));
 		}else if(d.qid.path == Qdump){
-			mnt = getmount(m->wname[i]);
+			mnt = getmount(name);	/* mnt leaked on error() */
 			name = "";
 			prev = -1ULL;
 			t = mnt->root;
@@ -1342,31 +1360,39 @@
 	r.nwqid = i;
 	if(i == 0 && m->nwname != 0)
 		error(Esrch);
-	f = o;
-	duped = 0;
 	if(m->fid != m->newfid && i == m->nwname){
 		if((f = dupfid(m->conn, m->newfid, o)) == nil)
 			error(Efid);
-		duped = 1;
+		runlock(o);
+		putfid(o);
+	} else {
+		runlock(o);
+		f = o;
 	}
-	runlock(o);
-
+	poperror();
+	if(waserror()){
+		putfid(f);
+		nexterror();
+	}
 	if(i > 0 && i == m->nwname){
 		wlock(f);
 		ao = nil;
 		if(waserror()){
-			if(duped)
+			if(o != f)
 				clunkfid(m->conn, f, &ao);
 			assert(ao == nil);
 			wunlock(f);
 			nexterror();
 		}
+		dent = getdent(mnt, up, &d);
+		if(waserror()){
+			clunkdent(mnt, dent);
+			nexterror();
+		}
 		if(up == -1ULL){
 			/* the root contains itself, I guess */
-			dent = getdent(mnt, up, &d);
 			dir = getdent(mnt, up, &d);
 		}else{
-			dent = getdent(mnt, up, &d);
 			findparent(t, up, &upup, &name, kbuf, sizeof(kbuf));
 			dkey(&k, upup, name, kbuf, sizeof(kbuf));
 			if(!btlookup(t, &k, &kv, kvbuf, sizeof(kvbuf)))
@@ -1374,6 +1400,7 @@
 			kv2dir(&kv, &d);
 			dir = getdent(mnt, upup, &d);
 		}
+		poperror();	/* clunkdent(mnt, dent) */
 		clunkdent(f->mnt, f->dent);
 		clunkdent(f->mnt, f->dir);
 		if(mnt != f->mnt){
@@ -1388,14 +1415,12 @@
 		f->duid = duid;
 		f->dgid = dgid;
 		f->dmode = dmode;
-		poperror();
 		wunlock(f);
+		poperror();
 	}
-	if(duped)
-		putfid(o);
 	putfid(f);
-	respond(m, &r);
 	poperror();
+	respond(m, &r);
 }
 
 static void
@@ -1406,25 +1431,24 @@
 	Fid *f;
 	int n;
 
-	if((f = getfid(m->conn, m->fid)) == nil){
-		rerror(m, Enofid);
-		return;
-	}
+	if((f = getfid(m->conn, m->fid)) == nil)
+		error(Enofid);
+	rlock(f->dent);
 	if(waserror()){
-		rerror(m, errmsg());
+		runlock(f->dent);
 		putfid(f);
-		return;
+		nexterror();
 	}
-	rlock(f->dent);
-	if((n = dir2statbuf(f->dent, buf, sizeof(buf))) == -1)
+	n = dir2statbuf(f->dent, buf, sizeof(buf));
+	if(n == -1)
 		error(Efs);
 	runlock(f->dent);
+	putfid(f);
+	poperror();
 	r.type = Rstat;
 	r.stat = (uchar*)buf;
 	r.nstat = n;
 	respond(m, &r);
-	poperror();
-	putfid(f);
 }
 
 static void
@@ -1447,18 +1471,18 @@
 
 	*ao = nil;
 	rename = 0;
-	if((f = getfid(m->conn, m->fid)) == nil){
-		rerror(m, Enofid);
-		return;
+	if((f = getfid(m->conn, m->fid)) == nil)
+		error(Enofid);
+	if(waserror()){
+		putfid(f);
+		nexterror();
 	}
 	de = f->dent;
 	truncwait(de, id);
 	wlock(de);
 	if(waserror()){
-		rerror(m, errmsg());
-		freeamsg(*ao);
-		*ao = nil;
-		goto Err;
+		wunlock(de);
+		nexterror();
 	}
 	if(de->gone)
 		error(Ephase);
@@ -1497,8 +1521,12 @@
 			error(Ewstatl);
 		if(d.length != de->length){
 			if(d.length < de->length){
-				if((*ao = malloc(sizeof(Amsg))) == nil)
-					error(Enomem);
+				*ao = emalloc(sizeof(Amsg), 1);
+				if(waserror()){
+					freeamsg(*ao);
+					*ao = nil;
+					nexterror();
+				}
 				qlock(&de->trunclk);
 				de->trunc = 1;
 				qunlock(&de->trunclk);
@@ -1636,15 +1664,18 @@
 	if(rename)
 		cpkey(de, &k, de->buf, sizeof(de->buf));
 
-	r.type = Rwstat;
-	respond(m, &r);
+	if(*ao != nil)
+		poperror();
+	wunlock(de);
 	poperror();
-
-Err:	wunlock(de);
 	putfid(f);
+	poperror();
+
+	r.type = Rwstat;
+	respond(m, &r);
 }
 
-
+/* fsclunk: must not raise error() */
 static void
 fsclunk(Fmsg *m, Amsg **ao)
 {
@@ -1658,9 +1689,10 @@
 	wlock(f);
 	clunkfid(m->conn, f, ao);
 	wunlock(f);
+	putfid(f);
+
 	r.type = Rclunk;
 	respond(m, &r);
-	putfid(f);
 }
 
 static void
@@ -1676,40 +1708,30 @@
 	Fid *f;
 	Xdir d;
 
-	if((e = okname(m->name)) != nil){
-		rerror(m, e);
-		return;
+	if((e = okname(m->name)) != nil)
+		error(e);
+	if(m->perm & (DMMOUNT|DMAUTH))
+		error(Ebotch);
+	if((f = getfid(m->conn, m->fid)) == nil)
+		error(Enofid);
+	if(waserror()){
+		putfid(f);
+		nexterror();
 	}
-	if(m->perm & (DMMOUNT|DMAUTH)){
-		rerror(m, Ebotch);
-		return;
-	}
-	if((f = getfid(m->conn, m->fid)) == nil){
-		rerror(m, Enofid);
-		return;
-	}
 	wlock(f);
-
 	if(waserror()){
-		rerror(m, errmsg());
-		goto Err;
-		
+		wunlock(f);
+		nexterror();
 	}
-	if(f->mode != -1){
-		rerror(m, Einuse);
-		goto Out;
-	}
+	if(f->mode != -1)
+		error(Einuse);
 	de = f->dent;
-	if(walk1(f->mnt->root, f->qpath, m->name, &old, &oldlen) == 0){
-		rerror(m, Eexist);
-		goto Out;
-	}
-
+	if(walk1(f->mnt->root, f->qpath, m->name, &old, &oldlen) == 0)
+		error(Eexist);
 	rlock(de);
 	if(fsaccess(f, de->mode, de->uid, de->gid, DMWRITE) == -1){
-		rerror(m, Eperm);
 		runlock(de);
-		goto Out;
+		error(Eperm);
 	}
 	duid = de->uid;
 	dgid = de->gid;
@@ -1751,8 +1773,7 @@
 			sysfatal("ream: pack super");
 		mb[nm].k = upkbuf;
 		mb[nm].nk = p - upkbuf;
-		if((p = packdkey(upvbuf, sizeof(upvbuf), f->qpath, d.name)) == nil)
-			sysfatal("ream: pack super");
+		p = packdkey(upvbuf, sizeof(upvbuf), f->qpath, d.name);
 		mb[nm].v = upvbuf;
 		mb[nm].nv = p - upvbuf;
 		nm++;
@@ -1761,9 +1782,16 @@
 	assert(nm <= nelem(mb));
 	upsert(f->mnt, mb, nm);
 
+	if(m->mode & ORCLOSE){
+		f->rclose = emalloc(sizeof(Amsg), 1);
+		if(waserror()){
+			free(f->rclose);
+			f->rclose = nil;
+			nexterror();
+		}
+	}
 	de = getdent(f->mnt, f->qpath, &d);
 	clunkdent(f->mnt, f->dent);
-	f->mode = mode2bits(m->mode);
 	f->pqpath = f->qpath;
 	f->qpath = d.qid.path;
 	f->dent = de;
@@ -1770,17 +1798,20 @@
 	f->duid = duid;
 	f->dgid = dgid;
 	f->dmode = dmode;
+	f->mode = mode2bits(m->mode);
 	if(m->mode & ORCLOSE)
-		f->rclose = emalloc(sizeof(Amsg), 1);
+		poperror();	/* free(f->rclose) */
 
 	r.type = Rcreate;
 	r.qid = d.qid;
 	r.iounit = f->iounit;
-	respond(m, &r);
-Out:	poperror();
-Err:	wunlock(f);
+
+	wunlock(f);
+	poperror();
 	putfid(f);
-	return;
+	poperror();
+
+	respond(m, &r);
 }
 
 static char*
@@ -1816,10 +1847,8 @@
 	Kvp kv;
 	Fid *f;
 
-	if((f = getfid(m->conn, m->fid)) == nil){
-		rerror(m, Enofid);
-		return;
-	}
+	if((f = getfid(m->conn, m->fid)) == nil)
+		error(Enofid);
 	t = f->mnt->root;
 	nm = 0;
 	wlock(f);
@@ -1827,10 +1856,12 @@
 	truncwait(f->dent, id);
 	wlock(f->dent);
 	if(waserror()){
-		rerror(m, errmsg());
+		wunlock(f->dent);
+		wunlock(f);
+		putfid(f);
 		freeamsg(*ao);
 		*ao = nil;
-		goto Err;
+		nexterror();
 	}
 	if(f->dent->gone)
 		error(Ephase);
@@ -1880,14 +1911,13 @@
 	assert(nm <= nelem(mb));
 	upsert(f->mnt, mb, nm);
 	f->dent->gone = 1;
-	r.type = Rremove;
-	respond(m, &r);
-	poperror();
-Err:
 	wunlock(f->dent);
 	wunlock(f);
 	putfid(f);
-	return;
+	poperror();
+
+	r.type = Rremove;
+	respond(m, &r);
 }
 
 static void
@@ -1903,14 +1933,11 @@
 	Msg mb;
 
 	mbits = mode2bits(m->mode);
-	if((f = getfid(m->conn, m->fid)) == nil){
-		rerror(m, Enofid);
-		return;
-	}
+	if((f = getfid(m->conn, m->fid)) == nil)
+		error(Enofid);
 	if(waserror()){
-		rerror(m, errmsg());
 		putfid(f);
-		return;
+		nexterror();
 	}
 	if(m->mode & OTRUNC)
 		truncwait(f->dent, id);
@@ -1938,20 +1965,26 @@
 	if(fsaccess(f, d.mode, d.uid, d.gid, mbits) == -1)
 		error(Eperm);
 	f->dent->length = d.length;
-	poperror();
 	wunlock(f->dent);
-	r.type = Ropen;
-	r.qid = d.qid;
-	r.iounit = f->iounit;
+	poperror();
 
 	wlock(f);
-	if(f->mode != -1){
+	if(waserror()){
 		wunlock(f);
+		nexterror();
+	}
+	if(f->mode != -1)
 		error(Einuse);
+	if(m->mode & ORCLOSE){
+		f->rclose = emalloc(sizeof(Amsg), 1);
+		if(waserror()){
+			free(f->rclose);
+			f->rclose = nil;
+			nexterror();
+		}
 	}
 	if((m->mode & OTRUNC) && !(f->dent->mode & DMAPPEND)){
 		wlock(f->dent);
-
 		if(waserror()){
 			wunlock(f->dent);
 			freeamsg(*ao);
@@ -1991,11 +2024,18 @@
 	}
 	f->mode = mode2bits(m->mode);
 	if(m->mode & ORCLOSE)
-		f->rclose = emalloc(sizeof(Amsg), 1);
+		poperror();	/* free(f->rclose) */
+
+	r.type = Ropen;
+	r.qid = d.qid;
+	r.iounit = f->iounit;
+
 	wunlock(f);
 	poperror();
-	respond(m, &r);
 	putfid(f);
+	poperror();
+
+	respond(m, &r);
 }
 
 static void
@@ -2019,9 +2059,8 @@
 		s = emalloc(sizeof(Scan), 1);
 		pfx[0] = Klabel;
 		btnewscan(s, pfx, 1);
-		if(f->scan != nil){
+		if(f->scan != nil)
 			free(f->scan);
-		}
 		f->scan = s;
 	}
 	if(s->donescan){
@@ -2047,23 +2086,27 @@
 	while(1){
 		if(!btnext(s, &s->kv))
 			break;
+		if(waserror()){
+			btexit(s);
+			nexterror();
+		}
 		memcpy(d.name, s->kv.k+1, s->kv.nk-1);
 		d.name[s->kv.nk-1] = 0;
 		d.qid.path = UNPACK64(s->kv.v + 1);
 		if((ns = dir2statbuf(&d, p, n)) == -1){
 			s->overflow = 1;
+			poperror();
 			break;
 		}
 		p += ns;
 		n -= ns;
+		poperror();
 	}
 	btexit(s);
 	r->count = p - r->data;
-
 Out:
-	poperror();
 	wunlock(f);
-	return;
+	poperror();
 }
 
 static void
@@ -2112,18 +2155,24 @@
 	while(1){
 		if(!btnext(s, &s->kv))
 			break;
+		if(waserror()){
+			btexit(s);
+			nexterror();
+		}
 		if((ns = kv2statbuf(&s->kv, p, n)) == -1){
 			s->overflow = 1;
+			poperror();
 			break;
 		}
 		p += ns;
 		n -= ns;
+		poperror();
 	}
-	r->count = p - r->data;
 	btexit(s);
+	r->count = p - r->data;
 Out:
-	poperror();
 	wunlock(f);
+	poperror();
 }
 
 static void
@@ -2142,6 +2191,11 @@
 		runlock(f);
 		return;
 	}
+	if(waserror()){
+		runlock(e);
+		runlock(f);
+		nexterror();
+	}
 	p = r->data;
 	c = m->count;
 	o = m->offset;
@@ -2150,15 +2204,16 @@
 		c = e->length - m->offset;
 	while(c != 0){
 		n = readb(t, f, p, o, c, e->length);
-		r->count += n;
-		if(n == 0)
+		if(n <= 0)
 			break;
 		p += n;
 		o += n;
 		c -= n;
+		r->count += n;
 	}
 	runlock(e);
 	runlock(f);
+	poperror();
 }
 
 static void
@@ -2167,18 +2222,15 @@
 	Fcall r;
 	Fid *f;
 
-	if((f = getfid(m->conn, m->fid)) == nil){
-		rerror(m, Enofid);
-		return;
-	}
+	if((f = getfid(m->conn, m->fid)) == nil)
+		error(Enofid);
 	r.type = Rread;
 	r.count = 0;
 	r.data = nil;
 	if(waserror()){
-		rerror(m, errmsg());
 		free(r.data);
 		putfid(f);
-		return;
+		nexterror();
 	}	
 	r.data = emalloc(m->count, 0);
 	if(f->dent->qid.type & QTAUTH)
@@ -2189,10 +2241,10 @@
 		readdir(m, f, &r);
 	else
 		readfile(m, f, &r);
+	putfid(f);
+	poperror();
 	respond(m, &r);
 	free(r.data);
-	poperror();
-	putfid(f);
 }
 
 static void
@@ -2208,28 +2260,32 @@
 	Tree *t;
 	Fid *f;
 
-	if((f = getfid(m->conn, m->fid)) == nil){
-		rerror(m, Enofid);
+	if((f = getfid(m->conn, m->fid)) == nil)
+		error(Enofid);
+	if(f->dent->qid.type & QTAUTH){
+		if(waserror()){
+			putfid(f);
+			nexterror();
+		}
+		authwrite(f, &r, m->data, m->count);
+		putfid(f);
+		poperror();
+		respond(m, &r);
 		return;
-	}
+	}	
 	wlock(f);
 	truncwait(f->dent, id);
 	wlock(f->dent);
 	if(waserror()){
-		rerror(m, errmsg());
 		wunlock(f->dent);
 		wunlock(f);
 		putfid(f);
-		return;
+		nexterror();
 	}
 	if(!(f->mode & DMWRITE))
 		error(Einuse);
 	if(f->dent->gone)
 		error(Ephase);
-	if(f->dent->qid.type & QTAUTH){
-		authwrite(f, &r, m->data, m->count);
-		goto Out;
-	}	
 
 	w = 0;
 	p = m->data;
@@ -2253,11 +2309,11 @@
 			nexterror();
 		}
 		n = writeb(f, &kv[i], &bp[i], p, o, c, f->dent->length);
-		poperror();
 		w += n;
 		p += n;
 		o += n;
 		c -= n;
+		poperror();
 	}
 
 	p = sbuf;
@@ -2285,14 +2341,16 @@
 
 	r.type = Rwrite;
 	r.count = w;
-Out:
-	poperror();
- 	respond(m, &r);
+
 	wunlock(f->dent);
 	wunlock(f);
 	putfid(f);	
+	poperror();
+
+ 	respond(m, &r);
 }
 
+/* fsflush: must not raise error() */
 void
 fsflush(Fmsg *m)
 {
@@ -2489,13 +2547,18 @@
 		qlock(&fs->mutlk);
 		epochstart(id);
 		fs->snap.dirty = 1;
-		switch(m->type){
-		case Tcreate:	fscreate(m);		break;
-		case Twrite:	fswrite(m, id);		break;
-		case Twstat:	fswstat(m, id, &a);	break;
-		case Tremove:	fsremove(m, id, &a);	break;
-		case Topen:	fsopen(m, id, &a);	break;
-		default:	abort();		break;
+		if(waserror())
+			rerror(m, "%s", errmsg());
+		else {
+			switch(m->type){
+			case Tcreate:	fscreate(m);		break;
+			case Twrite:	fswrite(m, id);		break;
+			case Twstat:	fswstat(m, id, &a);	break;
+			case Tremove:	fsremove(m, id, &a);	break;
+			case Topen:	fsopen(m, id, &a);	break;
+			default:	abort();		break;
+			}
+			poperror();
 		}
 		assert(estacksz() == 0);
 		epochend(id);
@@ -2515,12 +2578,18 @@
 	while(1){
 		m = chrecv(ch);
 		epochstart(id);
-		switch(m->type){
-		case Tattach:	fsattach(m);		break;
-		case Twalk:	fswalk(m);		break;
-		case Tread:	fsread(m);		break;
-		case Tstat:	fsstat(m);		break;
-		case Topen:	fsopen(m, id, nil);	break;
+		if(waserror())
+			rerror(m, "%s", errmsg());
+		else {
+			switch(m->type){
+			case Tattach:	fsattach(m);		break;
+			case Twalk:	fswalk(m);		break;
+			case Tread:	fsread(m);		break;
+			case Tstat:	fsstat(m);		break;
+			case Topen:	fsopen(m, id, nil);	break;
+			default:	abort();		break;
+			}
+			poperror();
 		}
 		assert(estacksz() == 0);
 		epochend(id);
@@ -2527,7 +2596,7 @@
 	}
 }
 
-void
+static void
 freetree(Bptr rb, vlong pred)
 {
 	Bptr bp;
@@ -2540,7 +2609,7 @@
 		for(i = 0; i < b->nval; i++){
 			getval(b, i, &kv);
 			bp = unpackbp(kv.v, kv.nv);
-			freetree(bp, pred);
+			freetree(bp, pred);	/* leak b on error() */
 			qlock(&fs->mutlk);
 			qunlock(&fs->mutlk);
 			epochclean();
@@ -2547,7 +2616,7 @@
 		}
 	}
 	if(rb.gen > pred)
-		freebp(nil, rb);
+		freebp(nil, rb);	/* wouldn't freeblk() be better here? */
 	dropblk(b);
 }
 
@@ -2562,7 +2631,7 @@
  * need to hold the mutlk, other than when we free or kill
  * blocks via epochclean.
  */
-void
+static void
 sweeptree(Tree *t)
 {
 	char pfx[1];
@@ -2642,12 +2711,18 @@
 				qunlock(a);
 				epochclean();
 			}
-			sync();
 
+			sync();	/* oldhd blocks leaked on error() */
+
 			for(i = 0; i < fs->narena; i++){
 				for(bp = oldhd[i]; bp.addr != -1; bp = nb){
 					qlock(&fs->mutlk);
 					epochstart(id);
+					if(waserror()){
+						epochend(id);
+						qunlock(&fs->mutlk);
+						nexterror();
+					}
 					b = getblk(bp, 0);
 					nb = b->logp;
 					freeblk(nil, b);
@@ -2654,6 +2729,7 @@
 					dropblk(b);
 					epochend(id);
 					qunlock(&fs->mutlk);
+					poperror();
 					epochclean();
 				}
 			}
@@ -2681,21 +2757,22 @@
 			}
 
 			qlock(&fs->mutlk);
+			epochstart(id);
 			if(waserror()){
+				epochend(id);
 				qunlock(&fs->mutlk);
 				nexterror();
 			}
-			epochstart(id);
 			snapfs(am, &t);
 			epochend(id);
-			poperror();
 			qunlock(&fs->mutlk);
+			poperror();
 
-			sync();
+			sync();	/* t leaked on error() */
 
 			if(t != nil){
 				epochwait();
-				sweeptree(t);
+				sweeptree(t);	/* t leaked on error() */
 				closesnap(t);
 			}
 			poperror();
@@ -2721,8 +2798,16 @@
 				nm++;
 			}
 			qlock(&fs->mutlk);
+			epochstart(id);
+			if(waserror()){
+				epochend(id);
+				qunlock(&fs->mutlk);
+				nexterror();
+			}
 			upsert(am->mnt, mb, nm);
+			epochend(id);
 			qunlock(&fs->mutlk);
+			poperror();
 			/* fallthrough */
 		case AOclear:
 			if(agetl(&fs->rdonly)){
@@ -2735,8 +2820,15 @@
 				ainc(&fs->rdonly);
 				break;
 			}
-			if(am->dent != nil)
+			if(am->dent != nil){
 				qlock(&am->dent->trunclk);
+				if(waserror()){
+					/* what should truncwait() do now? */
+					rwakeup(&am->dent->truncrz);
+					qunlock(&am->dent->trunclk);
+					nexterror();
+				}
+			}
 			fs->snap.dirty = 1;
 			nm = 0;
 			for(off = am->off; off < am->end; off += Blksz){
@@ -2750,16 +2842,17 @@
 				mb[nm].nv = 0;
 				if(++nm >= nelem(mb) || off + Blksz >= am->end){
 					qlock(&fs->mutlk);
+					epochstart(id);
 					if(waserror()){
+						epochend(id);
 						qunlock(&fs->mutlk);
 						nexterror();
 					}
-					epochstart(id);
 					upsert(am->mnt, mb, nm);
 					epochend(id);
 					qunlock(&fs->mutlk);
-					epochclean();
 					poperror();
+					epochclean();
 					nm = 0;
 				}
 			}
@@ -2767,6 +2860,7 @@
 				am->dent->trunc = 0;
 				rwakeup(&am->dent->truncrz);
 				qunlock(&am->dent->trunclk);
+				poperror();
 			}
 			poperror();
 			break;
--- a/sys/src/cmd/gefs/pack.c
+++ b/sys/src/cmd/gefs/pack.c
@@ -154,8 +154,10 @@
 		u = uid2user(nogroupid);
 	if((m = uid2user(d->muid)) == nil)
 		m = uid2user(noneid);
-	if(u == nil || g == nil || m == nil)
+	if(u == nil || g == nil || m == nil){
+		runlock(&fs->userlk);
 		error(Eperm);
+	}
 
 	p = buf;
 	nn = strlen(d->name);
--- a/sys/src/cmd/gefs/tree.c
+++ b/sys/src/cmd/gefs/tree.c
@@ -1238,16 +1238,12 @@
 	for(i = 0; i < nmsg; i++)
 		sz += msgsz(&msg[i]);
 	npull = 0;
-	path = nil;
-	npath = 0;
-
 Again:
+	b = getroot(t, &height);
 	if(waserror()){
-		freepath(t, path, npath);
+		dropblk(b);
 		nexterror();
 	}
-
-	b = getroot(t, &height);
 	if(npull == 0 && b->type == Tpivot && !filledbuf(b, nmsg, sz)){
 		fastupsert(t, b, msg, nmsg);
 		poperror();
@@ -1258,9 +1254,14 @@
 	 * split, so we allocate room for one extra
 	 * node in the path.
 	 */
-	npath = 0;
 	if((path = calloc((height + 2), sizeof(Path))) == nil)
 		error(Enomem);
+	poperror();
+	if(waserror()){
+		freepath(t, path, height+2);	/* npath not volatile */
+		nexterror();
+	}
+	npath = 0;
 	path[npath].b = nil;
 	path[npath].idx = -1;
 	path[npath].midx = -1;
@@ -1278,6 +1279,7 @@
 		bp = unpackbp(sep.v, sep.nv);
 		b = getblk(bp, 0);
 		npath++;
+		assert(npath < height+2);
 	}
 	path[npath].b = b;
 	path[npath].idx = -1;
@@ -1347,6 +1349,13 @@
 		dropblk(b);
 		error(Enomem);
 	}
+	if(waserror()){
+		for(i = 0; i < h; i++)
+			dropblk(p[i]);
+		dropblk(b);
+		free(p);
+		nexterror();
+	}
 	ok = 0;
 	p[0] = holdblk(b);
 	for(i = 1; i < h; i++){
@@ -1377,10 +1386,10 @@
 		}
 	}
 	for(i = 0; i < h; i++)
-		if(p[i] != nil)
-			dropblk(p[i]);
+		dropblk(p[i]);
 	dropblk(b);
 	free(p);
+	poperror();
 	return ok;
 }
 
@@ -1417,6 +1426,10 @@
 		dropblk(b);
 		error(Enomem);
 	}
+	if(waserror()){
+		btexit(s);
+		nexterror();
+	}
 	p = s->path;
 	p[0].b = b;
 	for(i = 0; i < s->ht; i++){
@@ -1443,6 +1456,7 @@
 			p[i].vi++;
 	}
 	s->first = 0;
+	poperror();
 }
 
 int
@@ -1459,7 +1473,7 @@
 	h = s->ht;
 	start = h;
 	bufsrc = -1;
-	if(s->donescan)
+	if(p == nil || s->donescan)
 		return 0;
 	if(waserror()){
 		btexit(s);
@@ -1545,4 +1559,6 @@
 	for(i = 0; i < s->ht; i++)
 		dropblk(s->path[i].b);
 	free(s->path);
+	s->path = nil;
+	s->ht = 0;
 }
-- 
⑨