shithub: drawterm

Download patch

ref: 94f8d9a60fabb07dd7e37c806ca39eadbd883339
parent: eeeb7632dd77d90d1439bb31ca198c5061950301
author: cinap_lenrek <[email protected]>
date: Fri Jan 6 13:17:58 EST 2023

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.

--- a/kern/devfs-posix.c
+++ b/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;
--- a/kern/devfs-win32.c
+++ b/kern/devfs-win32.c
@@ -47,7 +47,6 @@
 
 static	wchar_t *catpath(wchar_t *, char *, wchar_t *);
 static	ulong	fsdirread(Chan*, uchar*, int, vlong);
-static	int	fsomode(int);
 static	ulong	fsaccess(int);
 static	ulong	pathtype(wchar_t *);
 static	int	checkvolume(wchar_t *);
@@ -331,40 +330,24 @@
 static Chan*
 fsopen(Chan *c, int mode)
 {
-	ulong t;
-	int m, isdir;
 	wchar_t *p;
 	Ufsinfo *uif;
+	int omode;
 
-	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;
-	uif->offset = 0;
-	t = pathtype(uif->path);
-	if(isdir){
+	if(c->qid.type & QTDIR){
 		DIR *d;
+
+		if(omode != OREAD)
+			error(Eperm);
 		d = malloc(sizeof(*d));
-		switch(t){
+		if(waserror()){
+			free(d);
+			nexterror();
+		}
+		switch(pathtype(uif->path)){
 		case TPATH_ROOT:
 			d->drivebuf = malloc(sizeof(wchar_t)*MAX_PATH);
 			if(GetLogicalDriveStrings(MAX_PATH-1, d->drivebuf) == 0){
@@ -377,21 +360,21 @@
 		case TPATH_VOLUME:
 		case TPATH_FILE:
 			p = catpath(uif->path, "*.*", nil);
-			d->handle = FindFirstFile(p, &d->wfd);
-			free(p);
-			if(d->handle == INVALID_HANDLE_VALUE){
-				free(d);
+			if((d->handle = FindFirstFile(p, &d->wfd)) == INVALID_HANDLE_VALUE){
+				free(p);
 				oserror();
 			}
+			free(p);
 			break;
 		}
 		d->keep = 1;
 		uif->dir = d;
+		poperror();
 	} else {
-		uif->dir = nil;
+		uif->dir = NULL;
 		if((uif->fh = CreateFile(
 			uif->path,
-			fsaccess(mode),
+			fsaccess(omode & 3),
 			FILE_SHARE_READ | FILE_SHARE_WRITE,
 			NULL,
 			(mode & OTRUNC) ? TRUNCATE_EXISTING : OPEN_EXISTING,
@@ -399,7 +382,10 @@
 			0)) == INVALID_HANDLE_VALUE)
 			oserror();
 	}
+	uif->offset = 0;
+
 	c->offset = 0;
+	c->mode = omode;
 	c->flag |= COPEN;
 	return c;
 }
@@ -407,55 +393,72 @@
 static Chan*
 fscreate(Chan *c, char *name, int mode, ulong perm)
 {
-	int m;
-	ulong t;
 	wchar_t *newpath;
 	Ufsinfo *uif;
+	DIR *d;
+	HANDLE h;
+	int omode;
 
-	m = fsomode(mode&3);
+	omode = openmode(mode & ~OEXCL);
+
 	uif = c->aux;
-	t = pathtype(uif->path);
 	newpath = catpath(uif->path, name, nil);
 	if(waserror()){
 		free(newpath);
 		nexterror();
 	}
+	d = NULL;
+	h = INVALID_HANDLE_VALUE;
 	if(perm & DMDIR) {
 		wchar_t *p;
-		DIR *d;
-		if(m || t==TPATH_ROOT)
+
+		if(omode != OREAD || pathtype(uif->path) == TPATH_ROOT)
 			error(Eperm);
-		if(!CreateDirectory(newpath, NULL))
-			oserror();
+		if(!CreateDirectory(newpath, NULL)){
+			if(GetLastError() != ERROR_ALREADY_EXISTS || mode & OEXCL)
+				oserror();
+		}
 		d = malloc(sizeof(*d));
 		p = catpath(newpath, "*.*", nil);
-		d->handle = FindFirstFile(p, &d->wfd);
-		free(p);
-		if(d->handle == INVALID_HANDLE_VALUE){
+		if((d->handle = FindFirstFile(p, &d->wfd)) == INVALID_HANDLE_VALUE){
+			free(p);
 			free(d);
 			oserror();
 		}
+		free(p);
+		if(waserror()){
+			FindClose(d->handle);
+			free(d);
+			nexterror();
+		}
 		d->keep = 1;
-		uif->dir = d;
 	} else {
-		uif->dir = nil;
-		if((uif->fh = CreateFile(
+		if((h = CreateFile(
 			newpath,
-			fsaccess(mode),
+			fsaccess(omode & 3),
 			FILE_SHARE_READ | FILE_SHARE_WRITE,
 			NULL,
-			CREATE_NEW,
+			(mode & OEXCL) ? CREATE_NEW : CREATE_ALWAYS,
 			FILE_ATTRIBUTE_NORMAL,
 			0)) == INVALID_HANDLE_VALUE)
 			oserror();
+		if(waserror()){
+			CloseHandle(h);
+			nexterror();
+		}
 	}
+	c->qid = wfdtoqid(newpath, nil);
+	uif->dir = d;
+	uif->fh = h;
+	poperror();
 	free(uif->path);
 	uif->path = newpath;
+	uif->offset = 0;
 	poperror();
-	c->qid = wfdtoqid(newpath, nil);
+
 	c->offset = 0;
+	c->mode = omode;
 	c->flag |= COPEN;
-	c->mode = openmode(mode);
 	return c;
 }
 
@@ -770,22 +773,6 @@
 out:
 	uif->offset += i;
 	return i;
-}
-
-static int
-fsomode(int m)
-{
-	switch(m) {
-	case 0:			/* OREAD */
-	case 3:			/* OEXEC */
-		return 0;
-	case 1:			/* OWRITE */
-		return 1;
-	case 2:			/* ORDWR */
-		return 2;
-	}
-	error(Ebadarg);
-	return 0;
 }
 
 static ulong
--- a/kern/screen.h
+++ b/kern/screen.h
@@ -1,6 +1,7 @@
 typedef struct Mouseinfo Mouseinfo;
 typedef struct Mousestate Mousestate;
 typedef struct Cursorinfo Cursorinfo;
+typedef struct Screeninfo Screeninfo;
 
 struct Mousestate {
 	Point	xy;
@@ -28,10 +29,17 @@
 	uchar	set[2*16];
 };
 
+struct Screeninfo {
+	Lock	lk;
+	int	depth;
+	int	dibtype;
+};
+
 extern	Memimage *gscreen;
 extern	Mouseinfo mouse;
 extern	Cursorinfo cursor;
 extern	Cursorinfo arrow;
+extern	Screeninfo screen;
 
 void	screeninit(void);
 void	screenload(Rectangle, int, uchar *, Point, int);