git: 9front

Download patch

ref: ebc8c29205756646a158d37e66a980f8c2e8dfd1
parent: 9e2ca31787261392f29f221ca586fb7c09bc92e0
author: cinap_lenrek <cinap_lenrek@felloff.net>
date: Sun Dec 28 21:55:30 EST 2025

kernel: fix waserror() handling for bindmount() and sysunmount()

The waserror() label must only rely on variables set
*before* the block (or declare it as volatile).

The following code is not correct:

	foo = nil;
	if(waserror(){
		free(foo);
		nexterror();
	}
	...
	foo = malloc(...);
	poperror();

Instead, do:

	foo = malloc(...);
	if(waserror()){
		free(foo);
		nexterror();
	}
	...
	poperror();

Rewrite bindmount() and sysunmount() with separate error
labels for each resource after allocation.

Also, make poperror() order consistent so it appears after
the releasing code of the resource.

--- a/sys/src/9/port/sysfile.c
+++ b/sys/src/9/port/sysfile.c
@@ -713,8 +713,8 @@
 		unlock(c);
 	}
 
-	poperror();
 	cclose(c);
+	poperror();
 	return nnn;
 }
 
@@ -793,8 +793,8 @@
 		unlock(c);
 	}
 
-	poperror();
 	cclose(c);
+	poperror();
 	return m;
 }
 
@@ -983,8 +983,8 @@
 	r = devtab[c->type]->stat(c, s, l);
 	if((name = pathlast(c->path)) != nil)
 		r = dirsetname(name, strlen(name), s, r, l);
-	poperror();
 	cclose(c);
+	poperror();
 	return r;
 }
 
@@ -1009,8 +1009,8 @@
 	r = devtab[c->type]->stat(c, s, l);
 	if((name = pathlast(c->path)) != nil)
 		r = dirsetname(name, strlen(name), s, r, l);
-	poperror();
 	cclose(c);
+	poperror();
 	return r;
 }
 
@@ -1031,13 +1031,16 @@
 static int
 bindmount(int ismount, int fd, int afd, char* arg0, char* arg1, int flag, char* spec)
 {
-	int ret;
 	Chan *c0, *c1, *ac, *bc;
+	int ret;
 
 	if((flag&~MMASK) || (flag&MORDER)==(MBEFORE|MAFTER))
 		error(Ebadarg);
 
 	if(ismount){
+		if(!canmount(up->pgrp))
+			error(Enoattach);
+
 		validaddr((uintptr)spec, 1, 0);
 		spec = validnamedup(spec, 1);
 		if(waserror()){
@@ -1044,38 +1047,35 @@
 			free(spec);
 			nexterror();
 		}
-
-		if(!canmount(up->pgrp))
-			error(Enoattach);
-
-		ac = nil;
 		bc = fdtochan(fd, ORDWR, 0, 1);
-		if(waserror()) {
-			if(ac != nil)
-				cclose(ac);
+		if(waserror()){
 			cclose(bc);
 			nexterror();
 		}
-
-		if(afd >= 0)
+		ac = nil;
+		if(afd >= 0){
 			ac = fdtochan(afd, ORDWR, 0, 1);
-
+			if(waserror()){
+				cclose(ac);
+				nexterror();
+			}
+		}
 		c0 = mntattach(bc, ac, spec, flag&MCACHE);
-		poperror();	/* ac bc */
-		if(ac != nil)
+		if(ac != nil){
 			cclose(ac);
+			poperror();
+		}
 		cclose(bc);
+		poperror();
 	}else{
 		spec = nil;
 		validaddr((uintptr)arg0, 1, 0);
 		c0 = namec(arg0, Abind, 0, 0);
 	}
-
 	if(waserror()){
 		cclose(c0);
 		nexterror();
 	}
-
 	validaddr((uintptr)arg1, 1, 0);
 	c1 = namec(arg1, Amount, 0, 0);
 	if(waserror()){
@@ -1082,17 +1082,15 @@
 		cclose(c1);
 		nexterror();
 	}
-
 	ret = cmount(c0, c1, flag, spec);
-
-	poperror();
 	cclose(c1);
 	poperror();
 	cclose(c0);
+	poperror();
 	if(ismount){
-		fdclose(fd, 0);
-		poperror();
 		free(spec);
+		poperror();
+		fdclose(fd, 0);
 	}
 	return ret;
 }
@@ -1147,24 +1145,27 @@
 	name = va_arg(list, char*);
 	old = va_arg(list, char*);
 
-	cmounted = nil;
 	validaddr((uintptr)old, 1, 0);
 	cmount = namec(old, Amount, 0, 0);
-	if(waserror()) {
+	if(waserror()){
 		cclose(cmount);
-		if(cmounted != nil)
-			cclose(cmounted);
 		nexterror();
 	}
-	if(name != nil) {
+	if(name == nil)
+		cunmount(cmount, nil);
+	else{
 		validaddr((uintptr)name, 1, 0);
 		cmounted = namec(name, Aunmount, OREAD, 0);
+		if(waserror()){
+			cclose(cmounted);
+			nexterror();
+		}
+		cunmount(cmount, cmounted);
+		cclose(cmounted);
+		poperror();
 	}
-	cunmount(cmount, cmounted);
-	poperror();
 	cclose(cmount);
-	if(cmounted != nil)
-		cclose(cmounted);
+	poperror();
 	return 0;
 }
 
@@ -1181,7 +1182,7 @@
 	openmode(mode&~OEXCL);	/* error check only; OEXCL okay here */
 	validaddr((uintptr)name, 1, 0);
 	c = namec(name, Acreate, mode, perm);
-	if(waserror()) {
+	if(waserror()){
 		cclose(c);
 		nexterror();
 	}
@@ -1220,8 +1221,8 @@
 	 * so fake it up.  rootclose() is known to be a nop.
 	 */
 	c->type = 0;
-	poperror();
 	cclose(c);
+	poperror();
 	return 0;
 }
 
@@ -1248,8 +1249,8 @@
 		}
 	}
 	l = devtab[c->type]->wstat(c, d, nd);
-	poperror();
 	cclose(c);
+	poperror();
 	return l;
 }
 
@@ -1347,10 +1348,10 @@
 	if((name = pathlast(c->path)) != nil)
 		d->name = name;
 	packoldstat(s, d);
-	poperror();
 	free(d);
 	poperror();
 	cclose(c);
+	poperror();
 	return 0;
 }
 
@@ -1379,10 +1380,10 @@
 	if((name = pathlast(c->path)) != nil)
 		d->name = name;
 	packoldstat(s, d);
-	poperror();
 	free(d);
 	poperror();
 	cclose(c);
+	poperror();
 	return 0;
 }
 
--