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=none autolearn=ham autolearn_force=no version=3.4.4 Received: (qmail 19267 invoked from network); 6 Jan 2023 18:24:26 -0000 Received: from 9front.inri.net (168.235.81.73) by inbox.vuxu.org with ESMTPUTF8; 6 Jan 2023 18:24:26 -0000 Received: from duke.felloff.net ([216.126.196.34]) by 9front; Fri Jan 6 13:20:33 -0500 2023 Message-ID: Date: Fri, 06 Jan 2023 19:20:23 +0100 From: cinap_lenrek@felloff.net To: 9front@9front.org In-Reply-To: <22E420E1-6595-48D5-ABE5-7800430C142B@gmail.com> 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: extended non-blocking AJAX over SQL pipelining GPU controller Subject: Re: [9front] drawterm: kern/devfs-posix: fscreate does not respect parent dir's perm Reply-To: 9front@9front.org Precedence: bulk ok, i found some other issues with the code and committed the following: 94f8d9a60fabb07dd7e37c806ca39eadbd883339 devfs: create handle OEXCL, mask permissions with parent directory, fix leaks Hash: 94f8d9a60fabb07dd7e37c806ca39eadbd883339 Date: Fri Jan 6 19:17:58 CET 2023 Author: cinap_lenrek Message: devfs: create handle OEXCL, mask permissions with parent directory, fix leaks OEXCL has not been handled correctly. We have to return an error if the file already exists. The file-permissions should only be applied when we create a new file. When creating a new file, we mask te permissions with the permissions of the parent directory (see open(5)). There where a few potential leaks. After creating file file, we do a stat to get the qid information, but this can potentially fail, eigther leaving the Chan in the wrong state or leaking memory. Fix it. diff -u .git/fs/object/eeeb7632dd77d90d1439bb31ca198c5061950301/tree/kern/devfs-posix.c .git/fs/object/94f8d9a60fabb07dd7e37c806ca39eadbd883339/tree/kern/devfs-posix.c --- .git/fs/object/eeeb7632dd77d90d1439bb31ca198c5061950301/tree/kern/devfs-posix.c +++ .git/fs/object/94f8d9a60fabb07dd7e37c806ca39eadbd883339/tree/kern/devfs-posix.c @@ -170,51 +170,29 @@ static Chan* fsopen(Chan *c, int mode) { - int m, isdir; Ufsinfo *uif; + int omode; -/*print("fsopen %s\n", chanpath(c));*/ - m = mode & (OTRUNC|3); - switch(m) { - case 0: - break; - case 1: - case 1|16: - break; - case 2: - case 0|16: - case 2|16: - break; - case 3: - break; - default: - error(Ebadarg); - } + omode = openmode(mode); - isdir = c->qid.type & QTDIR; - - if(isdir && mode != OREAD) - error(Eperm); - - m = fsomode(m & 3); - c->mode = openmode(mode); - uif = c->aux; - if(isdir) { - uif->dir = opendir(uif->path); - if(uif->dir == 0) + if(c->qid.type & QTDIR) { + if(omode != OREAD) + error(Eperm); + if((uif->dir = opendir(uif->path)) == NULL) error(strerror(errno)); } else { + int m = fsomode(omode & 3); if(mode & OTRUNC) m |= O_TRUNC; - uif->fd = open(uif->path, m, 0666); - if(uif->fd < 0) + if((uif->fd = open(uif->path, m, 0666)) < 0) error(strerror(errno)); } uif->offset = 0; c->offset = 0; + c->mode = omode; c->flag |= COPEN; return c; } @@ -222,12 +200,13 @@ static Chan* fscreate(Chan *c, char *name, int mode, ulong perm) { - int fd, m; - char *path; struct stat stbuf; Ufsinfo *uif; + char *path; + DIR *dir; + int fd, omode; - m = fsomode(mode&3); + omode = openmode(mode & ~OEXCL); uif = c->aux; path = catpath(uif->path, name); @@ -235,49 +214,48 @@ free(path); nexterror(); } + fd = -1; + dir = NULL; if(perm & DMDIR) { - if(m) + if(omode != OREAD) error(Eperm); - - if(mkdir(path, perm & 0777) < 0) - error(strerror(errno)); - - fd = open(path, 0); - if(fd >= 0) { - chmod(path, perm & 0777); - chown(path, uif->uid, uif->uid); + if(mkdir(path, uif->mode & perm & 0777) < 0){ + if(errno != EEXIST || mode & OEXCL) + error(strerror(errno)); } - close(fd); - - uif->dir = opendir(path); - if(uif->dir == 0) + if((dir = opendir(path)) == NULL) error(strerror(errno)); + if(waserror()){ + closedir(dir); + nexterror(); + } } else { - fd = open(path, O_WRONLY|O_CREAT|O_TRUNC, 0666); - if(fd >= 0) { - if(m != 1) { - close(fd); - fd = open(path, m); - } - chmod(path, perm & 0777); - chown(path, uif->uid, uif->gid); - } - if(fd < 0) + int m = fsomode(omode & 3) | O_CREAT | O_TRUNC; + if(mode & OEXCL) + m |= O_EXCL; + if((fd = open(path, m, uif->mode & perm & 0666)) < 0) error(strerror(errno)); - uif->fd = fd; + if(waserror()){ + close(fd); + nexterror(); + } } if(stat(path, &stbuf) < 0) error(strerror(errno)); + c->qid = fsqid(&stbuf); + uif->fd = fd; + uif->dir = dir; + poperror(); free(uif->path); uif->path = path; + uif->offset = 0; poperror(); - c->qid = fsqid(&stbuf); c->offset = 0; + c->mode = omode; c->flag |= COPEN; - c->mode = openmode(mode); return c; } @@ -604,13 +582,13 @@ fsomode(int m) { switch(m) { - case 0: /* OREAD */ - case 3: /* OEXEC */ - return 0; - case 1: /* OWRITE */ - return 1; - case 2: /* ORDWR */ - return 2; + case OREAD: + case OEXEC: + return O_RDONLY; + case OWRITE: + return O_WRONLY; + case ORDWR: + return O_RDWR; } error(Ebadarg); return 0;