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.0 required=5.0 tests=RCVD_IN_DNSWL_NONE autolearn=ham autolearn_force=no version=3.4.4 Received: (qmail 10433 invoked from network); 9 Dec 2020 17:05:57 -0000 Received: from ewsd.inri.net (107.191.116.128) by inbox.vuxu.org with ESMTPUTF8; 9 Dec 2020 17:05:57 -0000 Received: from duke.felloff.net ([216.126.196.34]) by ewsd; Wed Dec 9 12:02:36 -0500 2020 Message-ID: <2CB362642212E7D888A1D4EFC5EF8846@felloff.net> Date: Wed, 09 Dec 2020 18:02:23 +0100 From: cinap_lenrek@felloff.net To: 9front@9front.org MIME-Version: 1.0 Content-Type: text/plain; charset="US-ASCII" Content-Transfer-Encoding: 7bit List-ID: <9front.9front.org> List-Help: X-Glyph: ➈ X-Bullshit: stateless shader-aware base-based element browser Subject: [9front] OCEXEC and ORCLOSE issues and solutions Reply-To: 9front@9front.org Precedence: bulk There are some tricky corner cases with ORCLOSE and OCEXEC open(2) flags with regard to the channel passing drivers like devsrv (/srv), devshr (/shr) and devdup (/fd). There are two issues: 1) Because OCEXEC flag is *shared* between all file descriptors pointing to same chan, it is possible to set the close-on-exec flag on a file posted to /srv. It is also possible to set close-on-exec on the file that you inherited from a parent process. So it would be closed in the parent as well if it would do a exec. 2) It was possible to retroactively set the ORCLOSE flag on a file descriptor or file posted to /srv and /fd. I'v currently running a kernel that is modified in the following way: Problem 1) The OCEXEC flag semantics have been changed to always refer to the *file descriptor* instead of the channel. (basically the same as posix FD_CLOEXEC). So when opening a file with OCEXEC (even /fd/N), the close-on-exec applies only to the file descriptor that open returned, not anyone else. When the file descriptors are copied with rfork(RFFDG), the per file descriptor flags are also copied. The dup(2) syscall always resets the OCEXEC flag for newfd. Note that if you want to have it close on exec, you can just reopen it with /fd. Problem 2) The remove-on-close flag makes sense to be shared by all the file descriptors. We just need to make sure to not set it retroactively in the relevant drivers. The solution is to have devfd, devsrv and devshr error out of open with Eperm[] when ORCLOSE flag was given. A patch of these modification follows. Please make some noise if there are any objections to these changes or bugs spotted. diff -r dcd57f411618 sys/src/9/port/auth.c --- a/sys/src/9/port/auth.c Wed Dec 09 01:05:14 2020 +0100 +++ b/sys/src/9/port/auth.c Wed Dec 09 17:37:26 2020 +0100 @@ -97,13 +97,12 @@ nexterror(); } - fd = newfd(ac); + /* always mark it close on exec */ + fd = newfd(ac, OCEXEC); if(fd < 0) error(Enofd); poperror(); /* ac */ - /* always mark it close on exec */ - ac->flag |= CCEXEC; return (uintptr)fd; } diff -r dcd57f411618 sys/src/9/port/chan.c --- a/sys/src/9/port/chan.c Wed Dec 09 01:05:14 2020 +0100 +++ b/sys/src/9/port/chan.c Wed Dec 09 17:37:26 2020 +0100 @@ -1468,9 +1468,6 @@ saveregisters(); c = devtab[c->type]->open(c, omode&~OCEXEC); - - if(omode & OCEXEC) - c->flag |= CCEXEC; if(omode & ORCLOSE) c->flag |= CRCLOSE; break; @@ -1571,11 +1568,9 @@ incref(cnew->path); cnew = devtab[cnew->type]->create(cnew, e.elems[e.nelems-1], omode&~(OEXCL|OCEXEC), perm); - poperror(); - if(omode & OCEXEC) - cnew->flag |= CCEXEC; if(omode & ORCLOSE) cnew->flag |= CRCLOSE; + poperror(); putmhead(m); cclose(c); c = cnew; diff -r dcd57f411618 sys/src/9/port/devdup.c --- a/sys/src/9/port/devdup.c Wed Dec 09 01:05:14 2020 +0100 +++ b/sys/src/9/port/devdup.c Wed Dec 09 17:37:26 2020 +0100 @@ -63,6 +63,8 @@ Chan *f; int fd, twicefd; + if(omode & ORCLOSE) + error(Eperm); if(c->qid.type & QTDIR){ if(omode != 0) error(Eisdir); diff -r dcd57f411618 sys/src/9/port/devshr.c --- a/sys/src/9/port/devshr.c Wed Dec 09 01:05:14 2020 +0100 +++ b/sys/src/9/port/devshr.c Wed Dec 09 17:37:26 2020 +0100 @@ -396,6 +396,8 @@ case Qcmpt: if(omode&OTRUNC) error(Eexist); + if(omode&ORCLOSE) + error(Eperm); shr = sch->shr; mpt = sch->mpt; devpermcheck(mpt->owner, mpt->perm, mode); diff -r dcd57f411618 sys/src/9/port/devsrv.c --- a/sys/src/9/port/devsrv.c Wed Dec 09 01:05:14 2020 +0100 +++ b/sys/src/9/port/devsrv.c Wed Dec 09 17:37:26 2020 +0100 @@ -135,6 +135,8 @@ 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); @@ -338,8 +340,6 @@ cclose(c1); nexterror(); } - if(c1->flag & (CCEXEC|CRCLOSE)) - error("posted fd has remove-on-close or close-on-exec"); if(c1->qid.type & QTAUTH) error("cannot post auth file in srv"); sp = srvlookup(nil, c->qid.path); diff -r dcd57f411618 sys/src/9/port/lib.h --- a/sys/src/9/port/lib.h Wed Dec 09 01:05:14 2020 +0100 +++ b/sys/src/9/port/lib.h Wed Dec 09 17:37:26 2020 +0100 @@ -176,7 +176,7 @@ #define ORDWR 2 /* read and write */ #define OEXEC 3 /* execute, == read but check execute permission */ #define OTRUNC 16 /* or'ed in (except for exec), truncate file first */ -#define OCEXEC 32 /* or'ed in, close on exec */ +#define OCEXEC 32 /* or'ed in (per file descriptor), close on exec */ #define ORCLOSE 64 /* or'ed in, remove on close */ #define OEXCL 0x1000 /* or'ed in, exclusive create */ diff -r dcd57f411618 sys/src/9/port/pgrp.c --- a/sys/src/9/port/pgrp.c Wed Dec 09 01:05:14 2020 +0100 +++ b/sys/src/9/port/pgrp.c Wed Dec 09 17:37:26 2020 +0100 @@ -140,7 +140,8 @@ new = smalloc(sizeof(Fgrp)); if(f == nil){ - new->fd = smalloc(DELTAFD*sizeof(Chan*)); + new->flag = smalloc(DELTAFD*sizeof(new->flag[0])); + new->fd = smalloc(DELTAFD*sizeof(new->fd[0])); new->nfd = DELTAFD; new->ref = 1; return new; @@ -152,12 +153,19 @@ i = new->nfd%DELTAFD; if(i != 0) new->nfd += DELTAFD - i; - new->fd = malloc(new->nfd*sizeof(Chan*)); + new->fd = malloc(new->nfd*sizeof(new->fd[0])); if(new->fd == nil){ unlock(f); free(new); error("no memory for fgrp"); } + new->flag = malloc(new->nfd*sizeof(new->flag[0])); + if(new->flag == nil){ + unlock(f); + free(new->fd); + free(new); + error("no memory for fgrp"); + } new->ref = 1; new->maxfd = f->maxfd; @@ -165,6 +173,7 @@ if((c = f->fd[i]) != nil){ incref(c); new->fd[i] = c; + new->flag[i] = f->flag[i]; } } unlock(f); @@ -194,6 +203,7 @@ up->closingfgrp = nil; free(f->fd); + free(f->flag); free(f); } diff -r dcd57f411618 sys/src/9/port/portdat.h --- a/sys/src/9/port/portdat.h Wed Dec 09 01:05:14 2020 +0100 +++ b/sys/src/9/port/portdat.h Wed Dec 09 17:37:26 2020 +0100 @@ -125,7 +125,7 @@ COPEN = 0x0001, /* for i/o */ CMSG = 0x0002, /* the message channel for a mount */ /*rsc CCREATE = 0x0004, /* permits creation if c->mnt */ - CCEXEC = 0x0008, /* close on exec */ + CCEXEC = 0x0008, /* close on exec (per file descriptor) */ CFREE = 0x0010, /* not in use */ CRCLOSE = 0x0020, /* remove on close */ CCACHE = 0x0080, /* client cache */ @@ -509,6 +509,7 @@ Ref; Lock; Chan **fd; + uchar *flag; /* per file-descriptor flags (CCEXEC) */ int nfd; /* number allocated */ int maxfd; /* highest fd in use */ int exceed; /* debugging */ diff -r dcd57f411618 sys/src/9/port/portfns.h --- a/sys/src/9/port/portfns.h Wed Dec 09 01:05:14 2020 +0100 +++ b/sys/src/9/port/portfns.h Wed Dec 09 17:37:26 2020 +0100 @@ -201,7 +201,7 @@ void nameerror(char*, char*); int needpages(void*); Chan* newchan(void); -int newfd(Chan*); +int newfd(Chan*, int); Mhead* newmhead(Chan*); Mount* newmount(Chan*, int, char*); Page* newpage(int, Segment **, uintptr); diff -r dcd57f411618 sys/src/9/port/sysfile.c --- a/sys/src/9/port/sysfile.c Wed Dec 09 01:05:14 2020 +0100 +++ b/sys/src/9/port/sysfile.c Wed Dec 09 17:37:26 2020 +0100 @@ -25,33 +25,45 @@ growfd(Fgrp *f, int fd) /* fd is always >= 0 */ { Chan **newfd, **oldfd; + uchar *newflag, *oldflag; + int nfd; - if(fd < f->nfd) + nfd = f->nfd; + if(fd < nfd) return 0; - if(fd >= f->nfd+DELTAFD) + if(fd >= nfd+DELTAFD) return -1; /* out of range */ /* * Unbounded allocation is unwise; besides, there are only 16 bits * of fid in 9P */ - if(f->nfd >= 5000){ + if(nfd >= 5000){ Exhausted: print("no free file descriptors\n"); return -1; } - newfd = malloc((f->nfd+DELTAFD)*sizeof(Chan*)); + oldfd = f->fd; + oldflag = f->flag; + newfd = malloc((nfd+DELTAFD)*sizeof(newfd[0])); if(newfd == nil) goto Exhausted; - oldfd = f->fd; - memmove(newfd, oldfd, f->nfd*sizeof(Chan*)); + memmove(newfd, oldfd, nfd*sizeof(newfd[0])); + newflag = malloc((nfd+DELTAFD)*sizeof(newflag[0])); + if(newfd == nil){ + free(newfd); + goto Exhausted; + } + memmove(newflag, oldflag, nfd*sizeof(newflag[0])); f->fd = newfd; - free(oldfd); - f->nfd += DELTAFD; + f->flag = newflag; + f->nfd = nfd+DELTAFD; if(fd > f->maxfd){ if(fd/100 > f->maxfd/100) f->exceed = (fd/100)*100; f->maxfd = fd; } + free(oldfd); + free(oldflag); return 1; } @@ -72,9 +84,9 @@ } int -newfd(Chan *c) +newfd(Chan *c, int mode) { - int fd; + int fd, flag; Fgrp *f; f = up->fgrp; @@ -87,6 +99,13 @@ if(fd > f->maxfd) f->maxfd = fd; f->fd[fd] = c; + + /* per file-descriptor flags */ + flag = 0; + if(mode & OCEXEC) + flag |= CCEXEC; + f->flag[fd] = flag; + unlockfgrp(f); return fd; } @@ -112,6 +131,8 @@ f->maxfd = fd[1]; f->fd[fd[0]] = c[0]; f->fd[fd[1]] = c[1]; + f->flag[fd[0]] = 0; + f->flag[fd[1]] = 0; unlockfgrp(f); return 0; } @@ -247,6 +268,7 @@ oc = f->fd[fd]; f->fd[fd] = c; + f->flag[fd] = 0; unlockfgrp(f); if(oc != nil) cclose(oc); @@ -255,7 +277,7 @@ cclose(c); nexterror(); } - fd = newfd(c); + fd = newfd(c, 0); if(fd < 0) error(Enofd); poperror(); @@ -280,7 +302,7 @@ cclose(c); nexterror(); } - fd = newfd(c); + fd = newfd(c, mode); if(fd < 0) error(Enofd); poperror(); @@ -295,7 +317,7 @@ lock(f); c = fd <= f->maxfd ? f->fd[fd] : nil; - if(c == nil || (flag != 0 && (c->flag&flag) == 0)){ + if(c == nil || (flag != 0 && ((f->flag[fd]|c->flag)&flag) == 0)){ unlock(f); return; } @@ -1166,7 +1188,7 @@ cclose(c); nexterror(); } - fd = newfd(c); + fd = newfd(c, mode); if(fd < 0) error(Enofd); poperror();