9front - general discussion about 9front
 help / color / mirror / Atom feed
From: cinap_lenrek@felloff.net
To: 9front@9front.org
Subject: Re: [9front] drawterm: kern/devfs-posix: fscreate does not respect parent dir's perm
Date: Fri, 06 Jan 2023 19:20:23 +0100	[thread overview]
Message-ID: <F2C2B306802018CC563F7A4E0CB26D80@felloff.net> (raw)
In-Reply-To: <22E420E1-6595-48D5-ABE5-7800430C142B@gmail.com>

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 <cinap_lenrek@felloff.net>
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;

      parent reply	other threads:[~2023-01-06 18:24 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-01-03 22:29 Xiao-Yong Jin
2023-01-06 14:21 ` cinap_lenrek
2023-01-06 14:44 ` cinap_lenrek
2023-01-06 15:30   ` Xiao-Yong Jin
2023-01-06 15:40 ` cinap_lenrek
2023-01-06 18:20 ` cinap_lenrek [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=F2C2B306802018CC563F7A4E0CB26D80@felloff.net \
    --to=cinap_lenrek@felloff.net \
    --cc=9front@9front.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).