[9front] [PATCH] Permissions for child boards in /srv

[9front] [PATCH] Permissions for child boards in /srv

[Jacob Moody]

This patch allows board stats to be mutated. It also changes
the default child board permissions to 0550.

This results in child service directories being
"private" to just the user who created them by
default.


Thanks,
moody

diff 5ee86cf824c5591aa92118c0cd9d71b005e789d0 uncommitted
--- a/sys/src/9/port/devsrv.c
+++ b/sys/src/9/port/devsrv.c
@@ -13,6 +13,8 @@
 	void 	*link;
 	char 	*name;
 	ulong 	path;
+	char	*owner;
+	ulong	perm;
 };

 typedef struct Srv Srv;
@@ -19,9 +21,6 @@
 struct Srv
 {
 	Link;
-
-	char	*owner;
-	ulong	perm;
 	Chan	*chan;
 };

@@ -140,6 +139,7 @@

 		b = ch->parent;
 		free(ch->name);
+		free(ch->owner);
 		wunlock(ch);
 		free(ch);
 	}
@@ -162,14 +162,17 @@
 	rlock(b);
 	if(waserror()){
 		runlock(b);
-		return -1;
+		nexterror();
 	}
-	if(s == DEVDOTDOT){
+	switch(s){
+	case -2: /* dot */
+		ch = b;
+		goto Child;
+	case DEVDOTDOT:
 		ch = b->parent;
 		if(ch == nil)
 			ch = &root;
 		goto Child;
-		
 	}
 	if(name != nil){
 		if(strcmp("clone", name) == 0)
@@ -194,21 +197,25 @@
 		devdir(c, q, up->genbuf, 0, sp->owner, sp->perm, dp);
 	} else if(ch != nil){
 Child:
+		if(name != nil || s == DEVDOTDOT){
+			devpermcheck(ch->owner, ch->perm, OEXEC);
+			c->aux = ch;
+		}
 		kstrcpy(up->genbuf, ch->name, sizeof up->genbuf);
 		q.vers = ch->id;
 		q.path = NETQID(q.vers, ch->path);
 		q.type = QTDIR;
-		devdir(c, q, up->genbuf, 0, eve, 0555|DMDIR, dp);
-		/* dirread's and stats shouldn't alter c->aux */
-		if(name != nil)
-			c->aux = ch;
+		devdir(c, q, up->genbuf, 0, ch->owner, ch->perm|DMDIR, dp);
 	} else if(0){
 Clone:
 		q.vers = NETID(c->qid.path);
 		q.path = NETQID(q.vers, Qclone);
 		devdir(c, q, "clone", 0, eve, 0444, dp);
-	} else
-		error(Enonexist);
+	} else {
+		runlock(b);
+		poperror();
+		return -1;
+	}

 	runlock(b);
 	poperror();
@@ -220,6 +227,8 @@
 {
 	root.qidpath = Qend;
 	root.name = "#s";
+	root.owner = eve;
+	root.perm = 0555;
 }

 static Chan*
@@ -253,6 +262,17 @@
 static int
 srvstat(Chan *c, uchar *db, int n)
 {
+	Dir d;
+
+	/* devstat cheats for dir stats, we care about our dir perms */
+	if(c->qid.type == QTDIR){
+		srvgen(c, nil, nil, 0, -2, &d);
+		n = convD2M(&d, db, n);
+		if(n == 0)
+			error(Ebadarg);
+		return n;
+	}
+
 	return devstat(c, db, n, 0, 0, srvgen);
 }

@@ -286,16 +306,6 @@
 	Chan *nc;
 	char buf[64];

-	if(c->qid.type == QTDIR){
-		if(omode & ORCLOSE)
-			error(Eperm);
-		if(omode != OREAD)
-			error(Eisdir);
-		c->mode = omode;
-		c->flag |= COPEN;
-		c->offset = 0;
-		return c;
-	}
 	if(omode&OTRUNC)
 		error(Eexist);
 	if(omode&ORCLOSE)
@@ -311,6 +321,8 @@
 		ch = smalloc(sizeof *ch);
 		ch->qidpath = Qend;
 		ch->ref = 1;
+		ch->perm = 0550;
+		kstrdup(&ch->owner, up->user);
 		do {
 			qlock(&boards);
 			ch->id = ++boards.path;
@@ -336,6 +348,19 @@
 		runlock(b);
 		nexterror();
 	}
+	if(c->qid.type == QTDIR){
+		if(omode & ORCLOSE)
+			error(Eperm);
+		if(omode != OREAD)
+			error(Eisdir);
+		devpermcheck(b->owner, b->perm, omode);
+		c->mode = omode;
+		c->flag |= COPEN;
+		c->offset = 0;
+		runlock(b);
+		poperror();
+		return c;
+	}
 	if(b->closed)
 		error(Eexpired);

@@ -459,18 +484,18 @@
 static int
 srvwstat(Chan *c, uchar *dp, int n)
 {
-	Board *b;
+	Board *b, *s;
 	char *strs;
-	Srv *sp;
 	Dir d;
+	Link *lp;

-	if(c->qid.type & QTDIR)
-		error(Eperm);
 	switch(NETTYPE(c->qid.path)){
 	case Qlease:
 	case Qclone:
 		error(Eperm);
 	}
+	if(c->qid.type == QTDIR && c->aux == &root)
+		error(Eperm);

 	strs = smalloc(n);
 	if(waserror()){
@@ -490,28 +515,49 @@
 	if(b->closed)
 		error(Eexpired);

-	sp = lookup(b->srv, nil, c->qid.path);
-	if(sp == nil)
+	if(c->qid.type == QTDIR){
+		lp = b;
+		/* we share ownership of our stats with our parent */
+		assert(b->parent != nil);
+		wlock(b->parent);
+		if(waserror()){
+			wunlock(b->parent);
+			nexterror();
+		}
+	} else
+		lp = lookup(b->srv, nil, c->qid.path);
+	if(lp == nil)
 		error(Enonexist);

-	if(strcmp(sp->owner, up->user) != 0 && !iseve())
+	if(strcmp(lp->owner, up->user) != 0 && !iseve())
 		error(Eperm);

-	if(d.name != nil && *d.name && strcmp(sp->name, d.name) != 0) {
+	if(d.name != nil && *d.name && strcmp(lp->name, d.name) != 0) {
 		if(strchr(d.name, '/') != nil)
 			error(Ebadchar);
 		if(strlen(d.name) >= sizeof(up->genbuf))
 			error(Etoolong);
-		if(lookup(b->srv, d.name, ~0UL) != nil)
+
+		//Ensure new name doesn't conflict with old names
+		if(c->qid.type == QTDIR)
+			s = b->parent;
+		else
+			s = b;
+		if(lookup(s->srv, d.name, ~0UL) != nil)
 			error(Eexist);
-		if(lookup(b->child, d.name, ~0UL) != nil)
+		if(lookup(s->child, d.name, ~0UL) != nil)
 			error(Eexist);
-		kstrdup(&sp->name, d.name);
+		kstrdup(&lp->name, d.name);
 	}
 	if(d.uid != nil && *d.uid)
-		kstrdup(&sp->owner, d.uid);
+		kstrdup(&lp->owner, d.uid);
 	if(d.mode != ~0UL)
-		sp->perm = d.mode & 0777;
+		lp->perm = d.mode & 0777;
+
+	if(c->qid.type == QTDIR){
+		wunlock(b->parent);
+		poperror();
+	}

 	wunlock(b);
 	poperror();

Re: [9front] [PATCH] Permissions for child boards in /srv

[Jacob Moody]

Some more tweaks:

diff 5ee86cf824c5591aa92118c0cd9d71b005e789d0 uncommitted
--- a/sys/src/9/port/devsrv.c
+++ b/sys/src/9/port/devsrv.c
@@ -13,6 +13,8 @@
 	void 	*link;
 	char 	*name;
 	ulong 	path;
+	char	*owner;
+	ulong	perm;
 };

 typedef struct Srv Srv;
@@ -19,9 +21,6 @@
 struct Srv
 {
 	Link;
-
-	char	*owner;
-	ulong	perm;
 	Chan	*chan;
 };

@@ -140,6 +139,7 @@

 		b = ch->parent;
 		free(ch->name);
+		free(ch->owner);
 		wunlock(ch);
 		free(ch);
 	}
@@ -162,14 +162,17 @@
 	rlock(b);
 	if(waserror()){
 		runlock(b);
-		return -1;
+		nexterror();
 	}
-	if(s == DEVDOTDOT){
+	switch(s){
+	case -2: /* dot */
+		ch = b;
+		goto Child;
+	case DEVDOTDOT:
 		ch = b->parent;
 		if(ch == nil)
 			ch = &root;
 		goto Child;
-		
 	}
 	if(name != nil){
 		if(strcmp("clone", name) == 0)
@@ -194,21 +197,25 @@
 		devdir(c, q, up->genbuf, 0, sp->owner, sp->perm, dp);
 	} else if(ch != nil){
 Child:
+		if(name != nil || s == DEVDOTDOT){
+			devpermcheck(ch->owner, ch->perm, OEXEC);
+			c->aux = ch;
+		}
 		kstrcpy(up->genbuf, ch->name, sizeof up->genbuf);
 		q.vers = ch->id;
 		q.path = NETQID(q.vers, ch->path);
 		q.type = QTDIR;
-		devdir(c, q, up->genbuf, 0, eve, 0555|DMDIR, dp);
-		/* dirread's and stats shouldn't alter c->aux */
-		if(name != nil)
-			c->aux = ch;
+		devdir(c, q, up->genbuf, 0, ch->owner, ch->perm|DMDIR, dp);
 	} else if(0){
 Clone:
 		q.vers = NETID(c->qid.path);
 		q.path = NETQID(q.vers, Qclone);
 		devdir(c, q, "clone", 0, eve, 0444, dp);
-	} else
-		error(Enonexist);
+	} else {
+		runlock(b);
+		poperror();
+		return -1;
+	}

 	runlock(b);
 	poperror();
@@ -220,6 +227,8 @@
 {
 	root.qidpath = Qend;
 	root.name = "#s";
+	root.perm = 0777;
+	kstrdup(&root.owner, eve);
 }

 static Chan*
@@ -253,6 +262,17 @@
 static int
 srvstat(Chan *c, uchar *db, int n)
 {
+	Dir d;
+
+	/* devstat cheats for dir stats, we care about our dir perms */
+	if(c->qid.type == QTDIR){
+		srvgen(c, nil, nil, 0, -2, &d);
+		n = convD2M(&d, db, n);
+		if(n == 0)
+			error(Ebadarg);
+		return n;
+	}
+
 	return devstat(c, db, n, 0, 0, srvgen);
 }

@@ -286,16 +306,6 @@
 	Chan *nc;
 	char buf[64];

-	if(c->qid.type == QTDIR){
-		if(omode & ORCLOSE)
-			error(Eperm);
-		if(omode != OREAD)
-			error(Eisdir);
-		c->mode = omode;
-		c->flag |= COPEN;
-		c->offset = 0;
-		return c;
-	}
 	if(omode&OTRUNC)
 		error(Eexist);
 	if(omode&ORCLOSE)
@@ -311,6 +321,8 @@
 		ch = smalloc(sizeof *ch);
 		ch->qidpath = Qend;
 		ch->ref = 1;
+		ch->perm = 0770;
+		kstrdup(&ch->owner, up->user);
 		do {
 			qlock(&boards);
 			ch->id = ++boards.path;
@@ -336,6 +348,19 @@
 		runlock(b);
 		nexterror();
 	}
+	if(c->qid.type == QTDIR){
+		if(omode & ORCLOSE)
+			error(Eperm);
+		if(omode != OREAD)
+			error(Eisdir);
+		devpermcheck(b->owner, b->perm, omode);
+		c->mode = omode;
+		c->flag |= COPEN;
+		c->offset = 0;
+		runlock(b);
+		poperror();
+		return c;
+	}
 	if(b->closed)
 		error(Eexpired);

@@ -369,6 +394,9 @@
 	if(strlen(name) >= sizeof(up->genbuf))
 		error(Etoolong);

+	if(strcmp("clone", name) == 0)
+		error("reserved name");
+
 	sp = smalloc(sizeof *sp);
 	kstrdup(&sp->name, name);
 	kstrdup(&sp->owner, up->user);
@@ -384,6 +412,7 @@
 	}
 	if(b->closed)
 		error(Eexpired);
+	devpermcheck(b->owner, b->perm, OWRITE);
 	if(lookup(b->srv, name, ~0UL) != nil)
 		error(Eexist);
 	if(lookup(b->child, name, ~0UL) != nil)
@@ -432,18 +461,9 @@
 	if(sp == nil)
 		error(Enonexist);

-	/*
-	 * Only eve can remove system services.
-	 */
-	if(strcmp(sp->owner, eve) == 0 && !iseve())
+	if(strcmp(sp->owner, up->user) != 0 && !iseve())
 		error(Eperm);

-	/*
-	 * No removing personal services.
-	 */
-	if((sp->perm&7) != 7 && strcmp(sp->owner, up->user) && !iseve())
-		error(Eperm);
-
 	remove((Link**)&b->srv, nil, c->qid.path);

 	boardclunk(b, 0); //unlock
@@ -459,18 +479,18 @@
 static int
 srvwstat(Chan *c, uchar *dp, int n)
 {
-	Board *b;
+	Board *b, *s;
 	char *strs;
-	Srv *sp;
 	Dir d;
+	Link *lp;

-	if(c->qid.type & QTDIR)
-		error(Eperm);
 	switch(NETTYPE(c->qid.path)){
 	case Qlease:
 	case Qclone:
 		error(Eperm);
 	}
+	if(c->qid.type == QTDIR && c->aux == &root)
+		error(Eperm);

 	strs = smalloc(n);
 	if(waserror()){
@@ -490,29 +510,50 @@
 	if(b->closed)
 		error(Eexpired);

-	sp = lookup(b->srv, nil, c->qid.path);
-	if(sp == nil)
+	if(c->qid.type == QTDIR){
+		lp = b;
+		/* we share ownership of our stats with our parent */
+		assert(b->parent != nil);
+		wlock(b->parent);
+		if(waserror()){
+			wunlock(b->parent);
+			nexterror();
+		}
+	} else
+		lp = lookup(b->srv, nil, c->qid.path);
+	if(lp == nil)
 		error(Enonexist);

-	if(strcmp(sp->owner, up->user) != 0 && !iseve())
+	if(strcmp(lp->owner, up->user) != 0 && !iseve())
 		error(Eperm);

-	if(d.name != nil && *d.name && strcmp(sp->name, d.name) != 0) {
+	if(d.name != nil && *d.name && strcmp(lp->name, d.name) != 0) {
 		if(strchr(d.name, '/') != nil)
 			error(Ebadchar);
 		if(strlen(d.name) >= sizeof(up->genbuf))
 			error(Etoolong);
-		if(lookup(b->srv, d.name, ~0UL) != nil)
+
+		//Ensure new name doesn't conflict with old names
+		if(c->qid.type == QTDIR)
+			s = b->parent;
+		else
+			s = b;
+		if(lookup(s->srv, d.name, ~0UL) != nil)
 			error(Eexist);
-		if(lookup(b->child, d.name, ~0UL) != nil)
+		if(lookup(s->child, d.name, ~0UL) != nil)
 			error(Eexist);
-		kstrdup(&sp->name, d.name);
+		kstrdup(&lp->name, d.name);
 	}
 	if(d.uid != nil && *d.uid)
-		kstrdup(&sp->owner, d.uid);
+		kstrdup(&lp->owner, d.uid);
 	if(d.mode != ~0UL)
-		sp->perm = d.mode & 0777;
+		lp->perm = d.mode & 0777;

+	if(c->qid.type == QTDIR){
+		wunlock(b->parent);
+		poperror();
+	}
+
 	wunlock(b);
 	poperror();

@@ -644,6 +685,7 @@

 	b = &root;
 	wlock(b);
+	kstrdup(&b->owner, new);
 	for(sp = b->srv; sp != nil; sp = sp->link) {
 		if(sp->owner != nil && strcmp(old, sp->owner) == 0)
 			kstrdup(&sp->owner, new);

Re: [9front] [PATCH] Permissions for child boards in /srv

[Jacob Moody]

Found a bug here(and in the committed version), we need
to decref on any remove call regardless of if it succeeds.

I'd like to get this committed soon to fix this bug.


Thanks,
moody

diff 9e363c506eccb783dd4d1d31f14c9d8dba98f65e uncommitted
--- a/sys/src/9/port/devsrv.c
+++ b/sys/src/9/port/devsrv.c
@@ -13,6 +13,8 @@
 	void 	*link;
 	char 	*name;
 	ulong 	path;
+	char	*owner;
+	ulong	perm;
 };

 typedef struct Srv Srv;
@@ -19,9 +21,6 @@
 struct Srv
 {
 	Link;
-
-	char	*owner;
-	ulong	perm;
 	Chan	*chan;
 };

@@ -140,6 +139,7 @@

 		b = ch->parent;
 		free(ch->name);
+		free(ch->owner);
 		wunlock(ch);
 		free(ch);
 	}
@@ -162,14 +162,17 @@
 	rlock(b);
 	if(waserror()){
 		runlock(b);
-		return -1;
+		nexterror();
 	}
-	if(s == DEVDOTDOT){
+	switch(s){
+	case -2: /* dot */
+		ch = b;
+		goto Child;
+	case DEVDOTDOT:
 		ch = b->parent;
 		if(ch == nil)
 			ch = &root;
 		goto Child;
-		
 	}
 	if(name != nil){
 		if(strcmp("clone", name) == 0)
@@ -194,21 +197,25 @@
 		devdir(c, q, up->genbuf, 0, sp->owner, sp->perm, dp);
 	} else if(ch != nil){
 Child:
+		if(name != nil || s == DEVDOTDOT){
+			devpermcheck(ch->owner, ch->perm, OEXEC);
+			c->aux = ch;
+		}
 		kstrcpy(up->genbuf, ch->name, sizeof up->genbuf);
 		q.vers = ch->id;
 		q.path = NETQID(q.vers, ch->path);
 		q.type = QTDIR;
-		devdir(c, q, up->genbuf, 0, eve, 0555|DMDIR, dp);
-		/* dirread's and stats shouldn't alter c->aux */
-		if(name != nil)
-			c->aux = ch;
+		devdir(c, q, up->genbuf, 0, ch->owner, ch->perm|DMDIR, dp);
 	} else if(0){
 Clone:
 		q.vers = NETID(c->qid.path);
 		q.path = NETQID(q.vers, Qclone);
 		devdir(c, q, "clone", 0, eve, 0444, dp);
-	} else
-		error(Enonexist);
+	} else {
+		runlock(b);
+		poperror();
+		return -1;
+	}

 	runlock(b);
 	poperror();
@@ -220,6 +227,8 @@
 {
 	root.qidpath = Qend;
 	root.name = "#s";
+	root.perm = 0777;
+	kstrdup(&root.owner, eve);
 }

 static Chan*
@@ -253,6 +262,17 @@
 static int
 srvstat(Chan *c, uchar *db, int n)
 {
+	Dir d;
+
+	/* devstat cheats for dir stats, we care about our dir perms */
+	if(c->qid.type == QTDIR){
+		srvgen(c, nil, nil, 0, -2, &d);
+		n = convD2M(&d, db, n);
+		if(n == 0)
+			error(Ebadarg);
+		return n;
+	}
+
 	return devstat(c, db, n, 0, 0, srvgen);
 }

@@ -286,16 +306,6 @@
 	Chan *nc;
 	char buf[64];

-	if(c->qid.type == QTDIR){
-		if(omode & ORCLOSE)
-			error(Eperm);
-		if(omode != OREAD)
-			error(Eisdir);
-		c->mode = omode;
-		c->flag |= COPEN;
-		c->offset = 0;
-		return c;
-	}
 	if(omode&OTRUNC)
 		error(Eexist);
 	if(omode&ORCLOSE)
@@ -311,6 +321,8 @@
 		ch = smalloc(sizeof *ch);
 		ch->qidpath = Qend;
 		ch->ref = 1;
+		ch->perm = 0770;
+		kstrdup(&ch->owner, up->user);
 		do {
 			qlock(&boards);
 			ch->id = ++boards.path;
@@ -336,6 +348,19 @@
 		runlock(b);
 		nexterror();
 	}
+	if(c->qid.type == QTDIR){
+		if(omode & ORCLOSE)
+			error(Eperm);
+		if(omode != OREAD)
+			error(Eisdir);
+		devpermcheck(b->owner, b->perm, omode);
+		c->mode = omode;
+		c->flag |= COPEN;
+		c->offset = 0;
+		runlock(b);
+		poperror();
+		return c;
+	}
 	if(b->closed)
 		error(Eexpired);

@@ -369,6 +394,9 @@
 	if(strlen(name) >= sizeof(up->genbuf))
 		error(Etoolong);

+	if(strcmp("clone", name) == 0)
+		error("reserved name");
+
 	sp = smalloc(sizeof *sp);
 	kstrdup(&sp->name, name);
 	kstrdup(&sp->owner, up->user);
@@ -384,6 +412,7 @@
 	}
 	if(b->closed)
 		error(Eexpired);
+	devpermcheck(b->owner, b->perm, OWRITE);
 	if(lookup(b->srv, name, ~0UL) != nil)
 		error(Eexist);
 	if(lookup(b->child, name, ~0UL) != nil)
@@ -414,6 +443,12 @@
 	Board *b;
 	Srv *sp;

+	b = c->aux;
+	wlock(b);
+	if(waserror()){
+		boardclunk(b, 0); //unlock
+		nexterror();
+	}
 	if(c->qid.type == QTDIR)
 		error(Eperm);
 	switch(NETTYPE(c->qid.path)){
@@ -422,28 +457,13 @@
 		error(Eperm);
 	}

-	b = c->aux;
-	wlock(b);
-	if(waserror()){
-		wunlock(b);
-		nexterror();
-	}
 	sp = lookup(b->srv, nil, c->qid.path);
 	if(sp == nil)
 		error(Enonexist);

-	/*
-	 * Only eve can remove system services.
-	 */
-	if(strcmp(sp->owner, eve) == 0 && !iseve())
+	if(strcmp(sp->owner, up->user) != 0 && !iseve())
 		error(Eperm);

-	/*
-	 * No removing personal services.
-	 */
-	if((sp->perm&7) != 7 && strcmp(sp->owner, up->user) && !iseve())
-		error(Eperm);
-
 	remove((Link**)&b->srv, nil, c->qid.path);

 	boardclunk(b, 0); //unlock
@@ -459,18 +479,18 @@
 static int
 srvwstat(Chan *c, uchar *dp, int n)
 {
-	Board *b;
+	Board *b, *s;
 	char *strs;
-	Srv *sp;
 	Dir d;
+	Link *lp;

-	if(c->qid.type & QTDIR)
-		error(Eperm);
 	switch(NETTYPE(c->qid.path)){
 	case Qlease:
 	case Qclone:
 		error(Eperm);
 	}
+	if(c->qid.type == QTDIR && c->aux == &root)
+		error(Eperm);

 	strs = smalloc(n);
 	if(waserror()){
@@ -490,29 +510,50 @@
 	if(b->closed)
 		error(Eexpired);

-	sp = lookup(b->srv, nil, c->qid.path);
-	if(sp == nil)
+	if(c->qid.type == QTDIR){
+		lp = b;
+		/* we share ownership of our stats with our parent */
+		assert(b->parent != nil);
+		wlock(b->parent);
+		if(waserror()){
+			wunlock(b->parent);
+			nexterror();
+		}
+	} else
+		lp = lookup(b->srv, nil, c->qid.path);
+	if(lp == nil)
 		error(Enonexist);

-	if(strcmp(sp->owner, up->user) != 0 && !iseve())
+	if(strcmp(lp->owner, up->user) != 0 && !iseve())
 		error(Eperm);

-	if(d.name != nil && *d.name && strcmp(sp->name, d.name) != 0) {
+	if(d.name != nil && *d.name && strcmp(lp->name, d.name) != 0) {
 		if(strchr(d.name, '/') != nil)
 			error(Ebadchar);
 		if(strlen(d.name) >= sizeof(up->genbuf))
 			error(Etoolong);
-		if(lookup(b->srv, d.name, ~0UL) != nil)
+
+		//Ensure new name doesn't conflict with old names
+		if(c->qid.type == QTDIR)
+			s = b->parent;
+		else
+			s = b;
+		if(lookup(s->srv, d.name, ~0UL) != nil)
 			error(Eexist);
-		if(lookup(b->child, d.name, ~0UL) != nil)
+		if(lookup(s->child, d.name, ~0UL) != nil)
 			error(Eexist);
-		kstrdup(&sp->name, d.name);
+		kstrdup(&lp->name, d.name);
 	}
 	if(d.uid != nil && *d.uid)
-		kstrdup(&sp->owner, d.uid);
+		kstrdup(&lp->owner, d.uid);
 	if(d.mode != ~0UL)
-		sp->perm = d.mode & 0777;
+		lp->perm = d.mode & 0777;

+	if(c->qid.type == QTDIR){
+		wunlock(b->parent);
+		poperror();
+	}
+
 	wunlock(b);
 	poperror();

@@ -538,12 +579,12 @@
 	 	 * which is immutable, all is well.
 	 	 */
 		if(waserror())
-			goto Clunk;
+			return;
 		srvremove(c);
 		poperror();
 		return;
 	}
-Clunk:
+
 	b = c->aux;
 	wlock(b);
 	boardclunk(b, expired); //unlock
@@ -644,6 +685,7 @@

 	b = &root;
 	wlock(b);
+	kstrdup(&b->owner, new);
 	for(sp = b->srv; sp != nil; sp = sp->link) {
 		if(sp->owner != nil && strcmp(old, sp->owner) == 0)
 			kstrdup(&sp->owner, new);

Re: [9front] [PATCH] Permissions for child boards in /srv

[cinap_lenrek]

but thats not just what this patch does.

it changes all kinds of stuff.

i'd suggest we revert everything at this point.

this clearly needs more time for review
and more testing.

we should not do experiments like this in the
kernel for a device that all the hosts security
depends on and that every user has access to.

please.

--
cinap

Re: [9front] [PATCH] Permissions for child boards in /srv

[Jacob Moody]

On 6/29/22 10:35, cinap_lenrek@felloff.net wrote:
> but thats not just what this patch does.
> 
> it changes all kinds of stuff.
> 
> i'd suggest we revert everything at this point.

It's disheartening you feel this way. I will
begin the process of reverting all of my commits
from the past month or so.

Re: [9front] [PATCH] Permissions for child boards in /srv

[ori]

Quoth Jacob Moody <moody@mail.posixcafe.org>:
> On 6/29/22 10:35, cinap_lenrek@felloff.net wrote:
> > but thats not just what this patch does.
> > 
> > it changes all kinds of stuff.
> > 
> > i'd suggest we revert everything at this point.
> 
> It's disheartening you feel this way. I will
> begin the process of reverting all of my commits
> from the past month or so.

The ideas are good, but we need to get them right
before we push them to other people's computers.

I use whatever's committed to actually write code
for work :)

Re: [9front] [PATCH] Permissions for child boards in /srv

[hiro]

you guys know there are also branches?

On 6/29/22, ori@eigenstate.org <ori@eigenstate.org> wrote:
> Quoth Jacob Moody <moody@mail.posixcafe.org>:
>> On 6/29/22 10:35, cinap_lenrek@felloff.net wrote:
>> > but thats not just what this patch does.
>> >
>> > it changes all kinds of stuff.
>> >
>> > i'd suggest we revert everything at this point.
>>
>> It's disheartening you feel this way. I will
>> begin the process of reverting all of my commits
>> from the past month or so.
>
> The ideas are good, but we need to get them right
> before we push them to other people's computers.
>
> I use whatever's committed to actually write code
> for work :)
>
>

Re: [9front] [PATCH] Permissions for child boards in /srv

[cinap_lenrek]

> It's disheartening you feel this way.

sorry, but this code runs on all our computers.

> I will begin the process of reverting all of my commits
> from the past month or so.

no. its just devsrv.c.

the reason i lost faith is that the patch changes
alot more than you said in the message.

its not just fixing that refcounting bug.

if there are other issues with that code we need
to be transparent about it. 

we need to take some time to make sure we'r not
introducing even more issues with that patch.
reverting, at this point, buys us that time.

--
cinap

Re: [9front] [PATCH] Permissions for child boards in /srv

[Jacob Moody]

[-- Attachment #1: Type: text/plain, Size: 16315 bytes --]

On 6/29/22 11:20, cinap_lenrek@felloff.net wrote:
>> It's disheartening you feel this way.
> 
> sorry, but this code runs on all our computers.
> 

You're correct, I am being disrespectful to others.

>> I will begin the process of reverting all of my commits
>> from the past month or so.
> 
> no. its just devsrv.c.

reverted. I had incorrectly assumed you wanted
all the stuff I'd been working on teared out.

> 
> the reason i lost faith is that the patch changes
> alot more than you said in the message.

My apologies, the message was only intended to cover
the delta from the previous post. This wasn't clear
so I understand your lose of faith. The 'other bits'
of that code are doing what the subject, and first couple
of posts detail. Which is allowing permissions to be set
on to child service directories to restrict access.

Here's the patch in total now against the now reverted version
in tree, happy to provide more context. Since this is more or less
a full rewrite, I've included the full devsrv.c as an attachment if
that is easier to dig through.


Thanks,
moody

diff cac03f355caa29a6bd5cafd7d0d6ca362149cb27 uncommitted
--- a//sys/man/3/srv
+++ b//sys/man/3/srv
@@ -5,6 +5,8 @@
 .nf
 .B bind #s /srv

+.BI #s/ clone
+.BI #s/ n
 .BI #s/ service1
 .BI #s/ service2
  ...
@@ -12,7 +14,7 @@
 .SH DESCRIPTION
 The
 .I srv
-device provides a one-level directory holding
+device provides a tree of directories holding
 already-open channels to services.
 In effect,
 .I srv
@@ -40,6 +42,18 @@
 .PP
 It is an error to write more than one number into a server file,
 or to create a file with a name that is already being used.
+.PP
+Opening the
+.I clone
+file allocates a new service directory. Reading
+.I clone
+returns the id of the new directory. This new service
+directory can then be accessed at
+.BR /srv/id .
+Directories are recursable; each new service directory
+contains its own
+.I clone
+file.
 .SH EXAMPLE
 To drop one end of a pipe into
 .BR /srv ,
--- a//sys/src/9/port/devsrv.c
+++ b//sys/src/9/port/devsrv.c
@@ -5,61 +5,220 @@
 #include	"fns.h"
 #include	"../port/error.h"

+#include	"netif.h"

-typedef struct Srv Srv;
-struct Srv
+typedef struct Link Link;
+struct Link
 {
-	char	*name;
+	void 	*link;
+	char 	*name;
+	ulong 	path;
 	char	*owner;
 	ulong	perm;
+};
+
+typedef struct Srv Srv;
+struct Srv
+{
+	Link;
 	Chan	*chan;
-	Srv	*link;
-	ulong	path;
 };

-static QLock	srvlk;
-static Srv	*srv;
-static int	qidpath;
+typedef struct Board Board;
+struct Board
+{
+	Link;
+	RWlock;
+	Ref;

-static Srv*
-srvlookup(char *name, ulong qidpath)
+	Board 	*parent;
+	Board 	*child;
+	Srv 	*srv;
+	long	id;
+	int	qidpath;
+	int 	closed;	
+};
+
+struct{
+	QLock;
+	long path;
+} boards;
+
+enum{
+	Qroot,
+	Qclone,
+	Qlease,
+
+	Qend,
+};
+
+Board root;
+
+static char Eexpired[] = "expired lease";
+
+static void*
+lookup(Link *l, char *name, ulong qidpath)
 {
-	Srv *sp;
+	Link *lp;

-	for(sp = srv; sp != nil; sp = sp->link) {
-		if(sp->path == qidpath || (name != nil && strcmp(sp->name, name) == 0))
-			return sp;
+	if(qidpath != ~0UL)
+		qidpath = NETTYPE(qidpath);
+	for(lp = l; lp != nil; lp = lp->link){
+		if(qidpath != ~0UL && lp->path == qidpath)
+			return lp;
+		if(name != nil && strcmp(lp->name, name) == 0)
+			return lp;
 	}
 	return nil;
 }

+static void*
+remove(Link **l, char *name, ulong qidpath)
+{
+	Link *lp;
+	Link **last;
+
+	if(qidpath != ~0UL)
+		qidpath = NETTYPE(qidpath);
+	last = l;
+	for(lp = *l; lp != nil; lp = lp->link){
+		if(qidpath != ~0UL && lp->path == qidpath)
+			break;
+		if(name != nil && strcmp(lp->name, name) == 0)
+			break;
+		last = &lp->link;
+	}
+	if(lp == nil)
+		return nil;
+
+	*last = lp->link;
+	lp->link = nil;
+	return lp;
+}
+
+static void
+boardclunk(Board *b, int close)
+{
+	Srv *sp, *prv;
+	Board *ch;
+	long ref;
+
+	/* caller holds a wlock */
+	if(b == &root){
+		wunlock(b);
+		return;
+	}
+
+	if(close){
+		assert(b->closed == 0);
+		b->closed++;
+		for(sp = b->srv; sp != nil; sp = prv){
+			prv = sp->link;
+			free(sp->owner);
+			free(sp->name);
+			if(sp->chan != nil)
+				cclose(sp->chan);
+			free(sp);
+		}
+		b->srv = nil;
+	}
+	ref = decref(b);
+
+	/*
+	 * All boards must be walkable from root. So a board
+	 * is allowed to sit at zero references as long as it
+	 * still has active children. For leaf nodes we then
+	 * have to walk up the tree to clear now empty parents.
+	 */
+	while(b->closed && b->child == nil && ref == 0){
+		//Root should never be closed
+		assert(b->parent != nil);
+		wlock(b->parent);
+		ch = remove((Link**)&b->parent->child, b->name, b->path);
+		assert(ch == b);
+
+		b = ch->parent;
+		free(ch->name);
+		free(ch->owner);
+		wunlock(ch);
+		free(ch);
+	}
+	wunlock(b);
+}
+
 static int
 srvgen(Chan *c, char *name, Dirtab*, int, int s, Dir *dp)
 {
 	Srv *sp;
+	Board *b, *ch;
 	Qid q;

-	if(s == DEVDOTDOT){
-		devdir(c, c->qid, "#s", 0, eve, 0555, dp);
-		return 1;
+	if(name != nil && strlen(name) >= sizeof(up->genbuf))
+		return -1;
+
+	b = c->aux;
+	ch = nil;
+	mkqid(&q, ~0L, 0, QTFILE);
+	rlock(b);
+	if(waserror()){
+		runlock(b);
+		nexterror();
 	}
+	switch(s){
+	case -2: /* dot */
+		ch = b;
+		goto Child;
+	case DEVDOTDOT:
+		ch = b->parent;
+		if(ch == nil)
+			ch = &root;
+		goto Child;
+	}
+	if(name != nil){
+		if(strcmp("clone", name) == 0)
+			goto Clone;

-	qlock(&srvlk);
-	if(name != nil)
-		sp = srvlookup(name, -1);
-	else {
-		for(sp = srv; sp != nil && s > 0; sp = sp->link)
+		sp = lookup(b->srv, name, ~0UL);
+		if(sp == nil)
+			ch = lookup(b->child, name, ~0UL);
+	} else {
+		if(s == 0)
+			goto Clone;
+		s--;
+		for(sp = b->srv; sp != nil && s > 0; sp = sp->link)
 			s--;
+		for(ch = b->child; ch != nil && s > 0; ch = ch->link)
+			s--;
 	}
-	if(sp == nil || (name != nil && (strlen(sp->name) >= sizeof(up->genbuf)))) {
-		qunlock(&srvlk);
+	if(sp != nil){
+		kstrcpy(up->genbuf, sp->name, sizeof up->genbuf);
+		q.vers = NETID(c->qid.path);
+		q.path = NETQID(q.vers, sp->path);
+		devdir(c, q, up->genbuf, 0, sp->owner, sp->perm, dp);
+	} else if(ch != nil){
+Child:
+		if(name != nil || s == DEVDOTDOT){
+			devpermcheck(ch->owner, ch->perm, OEXEC);
+			c->aux = ch;
+		}
+		kstrcpy(up->genbuf, ch->name, sizeof up->genbuf);
+		q.vers = ch->id;
+		q.path = NETQID(q.vers, ch->path);
+		q.type = QTDIR;
+		devdir(c, q, up->genbuf, 0, ch->owner, ch->perm|DMDIR, dp);
+	} else if(0){
+Clone:
+		q.vers = NETID(c->qid.path);
+		q.path = NETQID(q.vers, Qclone);
+		devdir(c, q, "clone", 0, eve, 0444, dp);
+	} else {
+		runlock(b);
+		poperror();
 		return -1;
 	}
-	mkqid(&q, sp->path, 0, QTFILE);
-	/* make sure name string continues to exist after we release lock */
-	kstrcpy(up->genbuf, sp->name, sizeof up->genbuf);
-	devdir(c, q, up->genbuf, 0, sp->owner, sp->perm, dp);
-	qunlock(&srvlk);
+
+	runlock(b);
+	poperror();
 	return 1;
 }

@@ -66,24 +225,54 @@
 static void
 srvinit(void)
 {
-	qidpath = 1;
+	root.qidpath = Qend;
+	root.name = "#s";
+	root.perm = 0777;
+	kstrdup(&root.owner, eve);
 }

 static Chan*
 srvattach(char *spec)
 {
-	return devattach('s', spec);
+	Chan *c;
+
+	c = devattach('s', spec);
+	c->aux = &root;
+	return c;
 }

 static Walkqid*
 srvwalk(Chan *c, Chan *nc, char **name, int nname)
 {
-	return devwalk(c, nc, name, nname, 0, 0, srvgen);
+	Board *b;
+	Walkqid *wq;
+
+	wq = devwalk(c, nc, name, nname, 0, 0, srvgen);
+	if(wq == nil || wq->clone == nil)
+		return wq;
+
+	b = wq->clone->aux;
+	if(b == &root)
+		return wq;
+
+	incref(b);
+	return wq;
 }

 static int
 srvstat(Chan *c, uchar *db, int n)
 {
+	Dir d;
+
+	/* devstat cheats for dir stats, we care about our dir perms */
+	if(c->qid.type == QTDIR){
+		srvgen(c, nil, nil, 0, -2, &d);
+		n = convD2M(&d, db, n);
+		if(n == 0)
+			error(Ebadarg);
+		return n;
+	}
+
 	return devstat(c, db, n, 0, 0, srvgen);
 }

@@ -90,12 +279,14 @@
 char*
 srvname(Chan *c)
 {
+	Board *b;
 	Srv *sp;
 	char *s;

 	s = nil;
-	qlock(&srvlk);
-	for(sp = srv; sp != nil; sp = sp->link) {
+	b = &root;
+	rlock(b);
+	for(sp = b->srv; sp != nil; sp = sp->link) {
 		if(sp->chan == c){
 			s = malloc(3+strlen(sp->name)+1);
 			if(s != nil)
@@ -103,7 +294,7 @@
 			break;
 		}
 	}
-	qunlock(&srvlk);
+	runlock(b);
 	return s;
 }

@@ -110,33 +301,73 @@
 static Chan*
 srvopen(Chan *c, int omode)
 {
+	Board *b, *ch;
 	Srv *sp;
 	Chan *nc;
+	char buf[64];

+	if(omode&OTRUNC)
+		error(Eexist);
+	if(omode&ORCLOSE)
+		error(Eperm);
+
+	b = c->aux;
+	if(NETTYPE(c->qid.path) == Qclone){;
+		wlock(b);
+		if(b->closed){
+			wunlock(b);
+			error(Eexpired);
+		}
+		ch = smalloc(sizeof *ch);
+		ch->qidpath = Qend;
+		ch->ref = 1;
+		ch->perm = 0770;
+		kstrdup(&ch->owner, up->user);
+		do {
+			qlock(&boards);
+			ch->id = ++boards.path;
+			qunlock(&boards);
+			snprint(buf, sizeof buf, "%ld", ch->id);
+		} while(lookup(b->srv, buf, ~0UL) != nil);
+
+		ch->parent = b;
+		ch->path = b->qidpath++;
+		kstrdup(&ch->name, buf);
+
+		ch->link = b->child;
+		b->child = ch;
+		c->aux = ch;
+		c->qid.vers = ch->id;
+		c->qid.path = NETQID(ch->id, Qlease);
+		boardclunk(b, 0); //unlock
+		return c;
+	}
+
+	rlock(b);
+	if(waserror()){
+		runlock(b);
+		nexterror();
+	}
 	if(c->qid.type == QTDIR){
 		if(omode & ORCLOSE)
 			error(Eperm);
 		if(omode != OREAD)
 			error(Eisdir);
+		devpermcheck(b->owner, b->perm, omode);
 		c->mode = omode;
 		c->flag |= COPEN;
 		c->offset = 0;
+		runlock(b);
+		poperror();
 		return c;
 	}
-	qlock(&srvlk);
-	if(waserror()){
-		qunlock(&srvlk);
-		nexterror();
-	}
+	if(b->closed)
+		error(Eexpired);

-	sp = srvlookup(nil, c->qid.path);
+	sp = lookup(b->srv, nil, c->qid.path);
 	if(sp == nil || sp->chan == nil)
 		error(Eshutdown);

-	if(omode&OTRUNC)
-		error(Eexist);
-	if(omode&ORCLOSE)
-		error(Eperm);
 	if(openmode(omode)!=sp->chan->mode && sp->chan->mode!=ORDWR)
 		error(Eperm);
 	devpermcheck(sp->owner, sp->perm, omode);
@@ -144,7 +375,7 @@
 	nc = sp->chan;
 	incref(nc);

-	qunlock(&srvlk);
+	runlock(b);
 	poperror();

 	cclose(c);
@@ -154,6 +385,7 @@
 static Chan*
 srvcreate(Chan *c, char *name, int omode, ulong perm)
 {
+	Board *b;
 	Srv *sp;

 	if(openmode(omode) != OWRITE)
@@ -162,31 +394,41 @@
 	if(strlen(name) >= sizeof(up->genbuf))
 		error(Etoolong);

+	if(strcmp("clone", name) == 0)
+		error("reserved name");
+
 	sp = smalloc(sizeof *sp);
 	kstrdup(&sp->name, name);
 	kstrdup(&sp->owner, up->user);

-	qlock(&srvlk);
+	b = c->aux;
+	wlock(b);
 	if(waserror()){
-		qunlock(&srvlk);
+		wunlock(b);
 		free(sp->owner);
 		free(sp->name);
 		free(sp);
 		nexterror();
 	}
-	if(srvlookup(name, -1) != nil)
+	if(b->closed)
+		error(Eexpired);
+	devpermcheck(b->owner, b->perm, OWRITE);
+	if(lookup(b->srv, name, ~0UL) != nil)
 		error(Eexist);
+	if(lookup(b->child, name, ~0UL) != nil)
+		error(Eexist);

 	sp->perm = perm&0777;
-	sp->path = qidpath++;
+	sp->path = b->qidpath++;

-	c->qid.path = sp->path;
+	c->qid.path = NETQID(b->id, sp->path);
+	c->qid.vers = b->id;
 	c->qid.type = QTFILE;

-	sp->link = srv;
-	srv = sp;
+	sp->link = b->srv;
+	b->srv = sp;

-	qunlock(&srvlk);
+	wunlock(b);
 	poperror();

 	c->flag |= COPEN;
@@ -198,41 +440,33 @@
 static void
 srvremove(Chan *c)
 {
-	Srv *sp, **l;
+	Board *b;
+	Srv *sp;

-	if(c->qid.type == QTDIR)
-		error(Eperm);
-
-	qlock(&srvlk);
+	b = c->aux;
+	wlock(b);
 	if(waserror()){
-		qunlock(&srvlk);
+		boardclunk(b, 0); //unlock
 		nexterror();
 	}
-	l = &srv;
-	for(sp = *l; sp != nil; sp = *l) {
-		if(sp->path == c->qid.path)
-			break;
-		l = &sp->link;
+	if(c->qid.type == QTDIR)
+		error(Eperm);
+	switch(NETTYPE(c->qid.path)){
+	case Qlease:
+	case Qclone:
+		error(Eperm);
 	}
+
+	sp = lookup(b->srv, nil, c->qid.path);
 	if(sp == nil)
 		error(Enonexist);

-	/*
-	 * Only eve can remove system services.
-	 */
-	if(strcmp(sp->owner, eve) == 0 && !iseve())
+	if(strcmp(sp->owner, up->user) != 0 && !iseve())
 		error(Eperm);

-	/*
-	 * No removing personal services.
-	 */
-	if((sp->perm&7) != 7 && strcmp(sp->owner, up->user) && !iseve())
-		error(Eperm);
+	remove((Link**)&b->srv, nil, c->qid.path);

-	*l = sp->link;
-	sp->link = nil;
-
-	qunlock(&srvlk);
+	boardclunk(b, 0); //unlock
 	poperror();

 	if(sp->chan != nil)
@@ -245,12 +479,18 @@
 static int
 srvwstat(Chan *c, uchar *dp, int n)
 {
+	Board *b, *s;
 	char *strs;
-	Srv *sp;
 	Dir d;
+	Link *lp;

-	if(c->qid.type & QTDIR)
+	switch(NETTYPE(c->qid.path)){
+	case Qlease:
+	case Qclone:
 		error(Eperm);
+	}
+	if(c->qid.type == QTDIR && c->aux == &root)
+		error(Eperm);

 	strs = smalloc(n);
 	if(waserror()){
@@ -261,32 +501,60 @@
 	if(n == 0)
 		error(Eshortstat);

-	qlock(&srvlk);
+	b = c->aux;
+	wlock(b);
 	if(waserror()){
-		qunlock(&srvlk);
+		wunlock(b);
 		nexterror();
 	}
+	if(b->closed)
+		error(Eexpired);

-	sp = srvlookup(nil, c->qid.path);
-	if(sp == nil)
+	if(c->qid.type == QTDIR){
+		lp = b;
+		/* we share ownership of our stats with our parent */
+		assert(b->parent != nil);
+		wlock(b->parent);
+		if(waserror()){
+			wunlock(b->parent);
+			nexterror();
+		}
+	} else
+		lp = lookup(b->srv, nil, c->qid.path);
+	if(lp == nil)
 		error(Enonexist);

-	if(strcmp(sp->owner, up->user) != 0 && !iseve())
+	if(strcmp(lp->owner, up->user) != 0 && !iseve())
 		error(Eperm);

-	if(d.name != nil && *d.name && strcmp(sp->name, d.name) != 0) {
+	if(d.name != nil && *d.name && strcmp(lp->name, d.name) != 0) {
 		if(strchr(d.name, '/') != nil)
 			error(Ebadchar);
 		if(strlen(d.name) >= sizeof(up->genbuf))
 			error(Etoolong);
-		kstrdup(&sp->name, d.name);
+
+		//Ensure new name doesn't conflict with old names
+		if(c->qid.type == QTDIR)
+			s = b->parent;
+		else
+			s = b;
+		if(lookup(s->srv, d.name, ~0UL) != nil)
+			error(Eexist);
+		if(lookup(s->child, d.name, ~0UL) != nil)
+			error(Eexist);
+		kstrdup(&lp->name, d.name);
 	}
 	if(d.uid != nil && *d.uid)
-		kstrdup(&sp->owner, d.uid);
+		kstrdup(&lp->owner, d.uid);
 	if(d.mode != ~0UL)
-		sp->perm = d.mode & 0777;
+		lp->perm = d.mode & 0777;

-	qunlock(&srvlk);
+	if(c->qid.type == QTDIR){
+		wunlock(b->parent);
+		poperror();
+	}
+
+	wunlock(b);
 	poperror();

 	free(strs);
@@ -298,22 +566,47 @@
 static void
 srvclose(Chan *c)
 {
-	/*
-	 * in theory we need to override any changes in removability
-	 * since open, but since all that's checked is the owner,
-	 * which is immutable, all is well.
-	 */
-	if(c->flag & CRCLOSE){
+	Board *b;
+	int expired;
+
+	expired = 0;
+	if(NETTYPE(c->qid.path) == Qlease)
+		expired++;
+	else if(c->flag & CRCLOSE){
+		/*
+		 * in theory we need to override any changes in removability
+		 * since open, but since all that's checked is the owner,
+	 	 * which is immutable, all is well.
+	 	 */
 		if(waserror())
 			return;
 		srvremove(c);
 		poperror();
+		return;
 	}
+
+	b = c->aux;
+	wlock(b);
+	boardclunk(b, expired); //unlock
 }

 static long
-srvread(Chan *c, void *va, long n, vlong)
+srvread(Chan *c, void *va, long n, vlong off)
 {
+	Board *b;
+
+	if(NETTYPE(c->qid.path) == Qlease){
+		b = c->aux;
+		rlock(b);
+		if(waserror()){
+			runlock(b);
+			nexterror();
+		}
+		n = readstr((ulong)off, va, n, b->name);
+		runlock(b);
+		poperror();
+		return n;
+	}
 	isdir(c);
 	return devdirread(c, va, n, 0, 0, srvgen);
 }
@@ -321,11 +614,15 @@
 static long
 srvwrite(Chan *c, void *va, long n, vlong)
 {
+	Board *b;
 	Srv *sp;
 	Chan *c1;
 	int fd;
 	char buf[32];

+	if(NETTYPE(c->qid.path) == Qlease)
+		error(Eperm);
+
 	if(n >= sizeof buf)
 		error(Etoobig);
 	memmove(buf, va, n);	/* so we can NUL-terminate */
@@ -334,15 +631,18 @@

 	c1 = fdtochan(fd, -1, 0, 1);	/* error check and inc ref */

-	qlock(&srvlk);
+	b = c->aux;
+	wlock(b);
 	if(waserror()) {
-		qunlock(&srvlk);
+		wunlock(b);
 		cclose(c1);
 		nexterror();
 	}
+	if(b->closed)
+		error(Eexpired);
 	if(c1->qid.type & QTAUTH)
 		error("cannot post auth file in srv");
-	sp = srvlookup(nil, c->qid.path);
+	sp = lookup(b->srv, nil, c->qid.path);
 	if(sp == nil)
 		error(Enonexist);

@@ -351,7 +651,7 @@

 	sp->chan = c1;

-	qunlock(&srvlk);
+	wunlock(b);
 	poperror();
 	return n;
 }
@@ -380,12 +680,15 @@
 void
 srvrenameuser(char *old, char *new)
 {
+	Board *b;
 	Srv *sp;

-	qlock(&srvlk);
-	for(sp = srv; sp != nil; sp = sp->link) {
+	b = &root;
+	wlock(b);
+	kstrdup(&b->owner, new);
+	for(sp = b->srv; sp != nil; sp = sp->link) {
 		if(sp->owner != nil && strcmp(old, sp->owner) == 0)
 			kstrdup(&sp->owner, new);
 	}
-	qunlock(&srvlk);
+	wunlock(b);
 }


[-- Attachment #2.1: Type: text/plain, Size: 342 bytes --]

from postmaster@9front:
The following attachment had content that we can't
prove to be harmless.  To avoid possible automatic
execution, we changed the content headers.
The original header was:

	Content-Type: text/x-csrc; charset=UTF-8; name="devsrv.c"
	Content-Disposition: attachment; filename="devsrv.c"
	Content-Transfer-Encoding: base64

[-- Attachment #2.2: devsrv.c.suspect --]
[-- Type: application/octet-stream, Size: 11288 bytes --]

#include	"u.h"
#include	"../port/lib.h"
#include	"mem.h"
#include	"dat.h"
#include	"fns.h"
#include	"../port/error.h"

#include	"netif.h"

typedef struct Link Link;
struct Link
{
	void 	*link;
	char 	*name;
	ulong 	path;
	char	*owner;
	ulong	perm;
};

typedef struct Srv Srv;
struct Srv
{
	Link;
	Chan	*chan;
};

typedef struct Board Board;
struct Board
{
	Link;
	RWlock;
	Ref;

	Board 	*parent;
	Board 	*child;
	Srv 	*srv;
	long	id;
	int	qidpath;
	int 	closed;	
};

struct{
	QLock;
	long path;
} boards;

enum{
	Qroot,
	Qclone,
	Qlease,

	Qend,
};

Board root;

static char Eexpired[] = "expired lease";

static void*
lookup(Link *l, char *name, ulong qidpath)
{
	Link *lp;

	if(qidpath != ~0UL)
		qidpath = NETTYPE(qidpath);
	for(lp = l; lp != nil; lp = lp->link){
		if(qidpath != ~0UL && lp->path == qidpath)
			return lp;
		if(name != nil && strcmp(lp->name, name) == 0)
			return lp;
	}
	return nil;
}

static void*
remove(Link **l, char *name, ulong qidpath)
{
	Link *lp;
	Link **last;

	if(qidpath != ~0UL)
		qidpath = NETTYPE(qidpath);
	last = l;
	for(lp = *l; lp != nil; lp = lp->link){
		if(qidpath != ~0UL && lp->path == qidpath)
			break;
		if(name != nil && strcmp(lp->name, name) == 0)
			break;
		last = &lp->link;
	}
	if(lp == nil)
		return nil;

	*last = lp->link;
	lp->link = nil;
	return lp;
}

static void
boardclunk(Board *b, int close)
{
	Srv *sp, *prv;
	Board *ch;
	long ref;

	/* caller holds a wlock */
	if(b == &root){
		wunlock(b);
		return;
	}

	if(close){
		assert(b->closed == 0);
		b->closed++;
		for(sp = b->srv; sp != nil; sp = prv){
			prv = sp->link;
			free(sp->owner);
			free(sp->name);
			if(sp->chan != nil)
				cclose(sp->chan);
			free(sp);
		}
		b->srv = nil;
	}
	ref = decref(b);

	/*
	 * All boards must be walkable from root. So a board
	 * is allowed to sit at zero references as long as it
	 * still has active children. For leaf nodes we then
	 * have to walk up the tree to clear now empty parents.
	 */
	while(b->closed && b->child == nil && ref == 0){
		//Root should never be closed
		assert(b->parent != nil);
		wlock(b->parent);
		ch = remove((Link**)&b->parent->child, b->name, b->path);
		assert(ch == b);

		b = ch->parent;
		free(ch->name);
		free(ch->owner);
		wunlock(ch);
		free(ch);
	}
	wunlock(b);
}

static int
srvgen(Chan *c, char *name, Dirtab*, int, int s, Dir *dp)
{
	Srv *sp;
	Board *b, *ch;
	Qid q;

	if(name != nil && strlen(name) >= sizeof(up->genbuf))
		return -1;

	b = c->aux;
	ch = nil;
	mkqid(&q, ~0L, 0, QTFILE);
	rlock(b);
	if(waserror()){
		runlock(b);
		nexterror();
	}
	switch(s){
	case -2: /* dot */
		ch = b;
		goto Child;
	case DEVDOTDOT:
		ch = b->parent;
		if(ch == nil)
			ch = &root;
		goto Child;
	}
	if(name != nil){
		if(strcmp("clone", name) == 0)
			goto Clone;

		sp = lookup(b->srv, name, ~0UL);
		if(sp == nil)
			ch = lookup(b->child, name, ~0UL);
	} else {
		if(s == 0)
			goto Clone;
		s--;
		for(sp = b->srv; sp != nil && s > 0; sp = sp->link)
			s--;
		for(ch = b->child; ch != nil && s > 0; ch = ch->link)
			s--;
	}
	if(sp != nil){
		kstrcpy(up->genbuf, sp->name, sizeof up->genbuf);
		q.vers = NETID(c->qid.path);
		q.path = NETQID(q.vers, sp->path);
		devdir(c, q, up->genbuf, 0, sp->owner, sp->perm, dp);
	} else if(ch != nil){
Child:
		if(name != nil || s == DEVDOTDOT){
			devpermcheck(ch->owner, ch->perm, OEXEC);
			c->aux = ch;
		}
		kstrcpy(up->genbuf, ch->name, sizeof up->genbuf);
		q.vers = ch->id;
		q.path = NETQID(q.vers, ch->path);
		q.type = QTDIR;
		devdir(c, q, up->genbuf, 0, ch->owner, ch->perm|DMDIR, dp);
	} else if(0){
Clone:
		q.vers = NETID(c->qid.path);
		q.path = NETQID(q.vers, Qclone);
		devdir(c, q, "clone", 0, eve, 0444, dp);
	} else {
		runlock(b);
		poperror();
		return -1;
	}

	runlock(b);
	poperror();
	return 1;
}

static void
srvinit(void)
{
	root.qidpath = Qend;
	root.name = "#s";
	root.perm = 0777;
	kstrdup(&root.owner, eve);
}

static Chan*
srvattach(char *spec)
{
	Chan *c;

	c = devattach('s', spec);
	c->aux = &root;
	return c;
}

static Walkqid*
srvwalk(Chan *c, Chan *nc, char **name, int nname)
{
	Board *b;
	Walkqid *wq;

	wq = devwalk(c, nc, name, nname, 0, 0, srvgen);
	if(wq == nil || wq->clone == nil)
		return wq;

	b = wq->clone->aux;
	if(b == &root)
		return wq;

	incref(b);
	return wq;
}

static int
srvstat(Chan *c, uchar *db, int n)
{
	Dir d;

	/* devstat cheats for dir stats, we care about our dir perms */
	if(c->qid.type == QTDIR){
		srvgen(c, nil, nil, 0, -2, &d);
		n = convD2M(&d, db, n);
		if(n == 0)
			error(Ebadarg);
		return n;
	}

	return devstat(c, db, n, 0, 0, srvgen);
}

char*
srvname(Chan *c)
{
	Board *b;
	Srv *sp;
	char *s;

	s = nil;
	b = &root;
	rlock(b);
	for(sp = b->srv; sp != nil; sp = sp->link) {
		if(sp->chan == c){
			s = malloc(3+strlen(sp->name)+1);
			if(s != nil)
				sprint(s, "#s/%s", sp->name);
			break;
		}
	}
	runlock(b);
	return s;
}

static Chan*
srvopen(Chan *c, int omode)
{
	Board *b, *ch;
	Srv *sp;
	Chan *nc;
	char buf[64];

	if(omode&OTRUNC)
		error(Eexist);
	if(omode&ORCLOSE)
		error(Eperm);

	b = c->aux;
	if(NETTYPE(c->qid.path) == Qclone){;
		wlock(b);
		if(b->closed){
			wunlock(b);
			error(Eexpired);
		}
		ch = smalloc(sizeof *ch);
		ch->qidpath = Qend;
		ch->ref = 1;
		ch->perm = 0770;
		kstrdup(&ch->owner, up->user);
		do {
			qlock(&boards);
			ch->id = ++boards.path;
			qunlock(&boards);
			snprint(buf, sizeof buf, "%ld", ch->id);
		} while(lookup(b->srv, buf, ~0UL) != nil);

		ch->parent = b;
		ch->path = b->qidpath++;
		kstrdup(&ch->name, buf);

		ch->link = b->child;
		b->child = ch;
		c->aux = ch;
		c->qid.vers = ch->id;
		c->qid.path = NETQID(ch->id, Qlease);
		boardclunk(b, 0); //unlock
		return c;
	}

	rlock(b);
	if(waserror()){
		runlock(b);
		nexterror();
	}
	if(c->qid.type == QTDIR){
		if(omode & ORCLOSE)
			error(Eperm);
		if(omode != OREAD)
			error(Eisdir);
		devpermcheck(b->owner, b->perm, omode);
		c->mode = omode;
		c->flag |= COPEN;
		c->offset = 0;
		runlock(b);
		poperror();
		return c;
	}
	if(b->closed)
		error(Eexpired);

	sp = lookup(b->srv, nil, c->qid.path);
	if(sp == nil || sp->chan == nil)
		error(Eshutdown);

	if(openmode(omode)!=sp->chan->mode && sp->chan->mode!=ORDWR)
		error(Eperm);
	devpermcheck(sp->owner, sp->perm, omode);

	nc = sp->chan;
	incref(nc);

	runlock(b);
	poperror();

	cclose(c);
	return nc;
}

static Chan*
srvcreate(Chan *c, char *name, int omode, ulong perm)
{
	Board *b;
	Srv *sp;

	if(openmode(omode) != OWRITE)
		error(Eperm);

	if(strlen(name) >= sizeof(up->genbuf))
		error(Etoolong);

	if(strcmp("clone", name) == 0)
		error("reserved name");

	sp = smalloc(sizeof *sp);
	kstrdup(&sp->name, name);
	kstrdup(&sp->owner, up->user);

	b = c->aux;
	wlock(b);
	if(waserror()){
		wunlock(b);
		free(sp->owner);
		free(sp->name);
		free(sp);
		nexterror();
	}
	if(b->closed)
		error(Eexpired);
	devpermcheck(b->owner, b->perm, OWRITE);
	if(lookup(b->srv, name, ~0UL) != nil)
		error(Eexist);
	if(lookup(b->child, name, ~0UL) != nil)
		error(Eexist);

	sp->perm = perm&0777;
	sp->path = b->qidpath++;

	c->qid.path = NETQID(b->id, sp->path);
	c->qid.vers = b->id;
	c->qid.type = QTFILE;

	sp->link = b->srv;
	b->srv = sp;

	wunlock(b);
	poperror();

	c->flag |= COPEN;
	c->mode = OWRITE;

	return c;
}

static void
srvremove(Chan *c)
{
	Board *b;
	Srv *sp;

	b = c->aux;
	wlock(b);
	if(waserror()){
		boardclunk(b, 0); //unlock
		nexterror();
	}
	if(c->qid.type == QTDIR)
		error(Eperm);
	switch(NETTYPE(c->qid.path)){
	case Qlease:
	case Qclone:
		error(Eperm);
	}

	sp = lookup(b->srv, nil, c->qid.path);
	if(sp == nil)
		error(Enonexist);

	if(strcmp(sp->owner, up->user) != 0 && !iseve())
		error(Eperm);

	remove((Link**)&b->srv, nil, c->qid.path);

	boardclunk(b, 0); //unlock
	poperror();

	if(sp->chan != nil)
		cclose(sp->chan);
	free(sp->owner);
	free(sp->name);
	free(sp);
}

static int
srvwstat(Chan *c, uchar *dp, int n)
{
	Board *b, *s;
	char *strs;
	Dir d;
	Link *lp;

	switch(NETTYPE(c->qid.path)){
	case Qlease:
	case Qclone:
		error(Eperm);
	}
	if(c->qid.type == QTDIR && c->aux == &root)
		error(Eperm);

	strs = smalloc(n);
	if(waserror()){
		free(strs);
		nexterror();
	}
	n = convM2D(dp, n, &d, strs);
	if(n == 0)
		error(Eshortstat);

	b = c->aux;
	wlock(b);
	if(waserror()){
		wunlock(b);
		nexterror();
	}
	if(b->closed)
		error(Eexpired);

	if(c->qid.type == QTDIR){
		lp = b;
		/* we share ownership of our stats with our parent */
		assert(b->parent != nil);
		wlock(b->parent);
		if(waserror()){
			wunlock(b->parent);
			nexterror();
		}
	} else
		lp = lookup(b->srv, nil, c->qid.path);
	if(lp == nil)
		error(Enonexist);

	if(strcmp(lp->owner, up->user) != 0 && !iseve())
		error(Eperm);

	if(d.name != nil && *d.name && strcmp(lp->name, d.name) != 0) {
		if(strchr(d.name, '/') != nil)
			error(Ebadchar);
		if(strlen(d.name) >= sizeof(up->genbuf))
			error(Etoolong);

		//Ensure new name doesn't conflict with old names
		if(c->qid.type == QTDIR)
			s = b->parent;
		else
			s = b;
		if(lookup(s->srv, d.name, ~0UL) != nil)
			error(Eexist);
		if(lookup(s->child, d.name, ~0UL) != nil)
			error(Eexist);
		kstrdup(&lp->name, d.name);
	}
	if(d.uid != nil && *d.uid)
		kstrdup(&lp->owner, d.uid);
	if(d.mode != ~0UL)
		lp->perm = d.mode & 0777;

	if(c->qid.type == QTDIR){
		wunlock(b->parent);
		poperror();
	}

	wunlock(b);
	poperror();

	free(strs);
	poperror();

	return n;
}

static void
srvclose(Chan *c)
{
	Board *b;
	int expired;

	expired = 0;
	if(NETTYPE(c->qid.path) == Qlease)
		expired++;
	else if(c->flag & CRCLOSE){
		/*
		 * in theory we need to override any changes in removability
		 * since open, but since all that's checked is the owner,
	 	 * which is immutable, all is well.
	 	 */
		if(waserror())
			return;
		srvremove(c);
		poperror();
		return;
	}

	b = c->aux;
	wlock(b);
	boardclunk(b, expired); //unlock
}

static long
srvread(Chan *c, void *va, long n, vlong off)
{
	Board *b;

	if(NETTYPE(c->qid.path) == Qlease){
		b = c->aux;
		rlock(b);
		if(waserror()){
			runlock(b);
			nexterror();
		}
		n = readstr((ulong)off, va, n, b->name);
		runlock(b);
		poperror();
		return n;
	}
	isdir(c);
	return devdirread(c, va, n, 0, 0, srvgen);
}

static long
srvwrite(Chan *c, void *va, long n, vlong)
{
	Board *b;
	Srv *sp;
	Chan *c1;
	int fd;
	char buf[32];

	if(NETTYPE(c->qid.path) == Qlease)
		error(Eperm);

	if(n >= sizeof buf)
		error(Etoobig);
	memmove(buf, va, n);	/* so we can NUL-terminate */
	buf[n] = 0;
	fd = strtoul(buf, 0, 0);

	c1 = fdtochan(fd, -1, 0, 1);	/* error check and inc ref */

	b = c->aux;
	wlock(b);
	if(waserror()) {
		wunlock(b);
		cclose(c1);
		nexterror();
	}
	if(b->closed)
		error(Eexpired);
	if(c1->qid.type & QTAUTH)
		error("cannot post auth file in srv");
	sp = lookup(b->srv, nil, c->qid.path);
	if(sp == nil)
		error(Enonexist);

	if(sp->chan != nil)
		error(Ebadusefd);

	sp->chan = c1;

	wunlock(b);
	poperror();
	return n;
}

Dev srvdevtab = {
	's',
	"srv",

	devreset,
	srvinit,	
	devshutdown,
	srvattach,
	srvwalk,
	srvstat,
	srvopen,
	srvcreate,
	srvclose,
	srvread,
	devbread,
	srvwrite,
	devbwrite,
	srvremove,
	srvwstat,
};

void
srvrenameuser(char *old, char *new)
{
	Board *b;
	Srv *sp;

	b = &root;
	wlock(b);
	kstrdup(&b->owner, new);
	for(sp = b->srv; sp != nil; sp = sp->link) {
		if(sp->owner != nil && strcmp(old, sp->owner) == 0)
			kstrdup(&sp->owner, new);
	}
	wunlock(b);
}

Re: [9front] [PATCH] Permissions for child boards in /srv

[ori]

Quoth Jacob Moody <moody@mail.posixcafe.org>:
> 
> Here's the patch in total now against the now reverted version
> in tree, happy to provide more context. Since this is more or less
> a full rewrite, I've included the full devsrv.c as an attachment if
> that is easier to dig through.
> 

Applied this on shithub, let's see if the front falls off.

I'll also be putting together some code to make more use
of it. Maybe we also want auth/box to grow a way of
running with a private /srv? Maybe even unconditionally?

Something like:

diff d457233c70ef6aa28861dd2978e92968ffba0920 uncommitted
--- a/sys/src/cmd/auth/box.c
+++ b/sys/src/cmd/auth/box.c
@@ -145,11 +145,11 @@
 {
 	char *b;
 	Dir *d;
-	char devs[1024];
-	int dfd;
+	char devs[1024], srvname[24], srvpath[32];
+	int dfd, sfd;
 	char *parts[256];
 	int mflags[256];
-	int nparts;
+	int n, nparts;
 
 	nparts = 0;
 	memset(devs, 0, sizeof devs);
@@ -192,6 +192,14 @@
 
 	rfork(RFNAMEG|RFFDG);
 	skelfs();
+	sfd = open("/srv/clone", OREAD);
+	if(sfd < 0)
+		sysfatal("could not open /srv/clone: %r");
+	if((n = read(sfd, srvname, sizeof(srvname))) == -1)
+		sysfatal("could not read srv id: %r");
+	snprint(srvpath, sizeof(srvpath), "/srv/%.*s", n, srvname);
+	if(bind(srvpath, "/srv", MREPL) < 0)
+		sysfatal("could not bind %s: %r", srvpath);
 	dfd = open("/dev/drivers", OWRITE|OCEXEC);
 	if(dfd < 0)
 		sysfatal("could not /dev/drivers: %r");