From mboxrd@z Thu Jan 1 00:00:00 1970 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on inbox.vuxu.org X-Spam-Level: X-Spam-Status: No, score=0.2 required=5.0 tests=DKIM_INVALID,DKIM_SIGNED, NICE_REPLY_A,T_SCC_BODY_TEXT_LINE autolearn=no autolearn_force=no version=3.4.4 Received: (qmail 20527 invoked from network); 29 Jun 2022 14:55:22 -0000 Received: from 9front.inri.net (168.235.81.73) by inbox.vuxu.org with ESMTPUTF8; 29 Jun 2022 14:55:22 -0000 Received: from mail.posixcafe.org ([45.76.19.58]) by 9front; Wed Jun 29 10:53:59 -0400 2022 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=posixcafe.org; s=20200506; t=1656514435; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=7iOrylmQtSI1+/KVb8Uf7DCkxlsh7oIB7zTiaTfvTlE=; b=GzYeluoTNrSDhj9furUqAPVs3AWNxM1Y/XO5PWux/pNqnsQ8s9x6k1vmhnz1zeOEXNNQO4 QJhefdOX/MGGcthfECMKYL4TNyUlqOCHP8orBaoaA0tmn4VBiKe7RxAniVl+jUUHg0m1yl NS4IC5nWXvWbehCisr7Sj+pawljXtIg= Received: from [192.168.168.200] (161-97-228-135.lpcnextlight.net [161.97.228.135]) by mail.posixcafe.org (OpenSMTPD) with ESMTPSA id f93bdcb8 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO) for <9front@9front.org>; Wed, 29 Jun 2022 09:53:55 -0500 (CDT) Message-ID: <8b9f9937-6e59-a07e-aed1-3ebd94fa119a@posixcafe.org> Date: Wed, 29 Jun 2022 08:53:45 -0600 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.10.0 Content-Language: en-US To: 9front@9front.org References: <59eb60df-a92a-9d40-a97e-d179b5f80ea0@posixcafe.org> <16473a2f-dc03-d6f2-4060-bb1b5bef3f9f@posixcafe.org> From: Jacob Moody In-Reply-To: <16473a2f-dc03-d6f2-4060-bb1b5bef3f9f@posixcafe.org> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit List-ID: <9front.9front.org> List-Help: X-Glyph: ➈ X-Bullshit: social hypervisor CSS cache Subject: Re: [9front] [PATCH] Permissions for child boards in /srv Reply-To: 9front@9front.org Precedence: bulk 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);