* [9front] drawterm: kern/devfs-posix: fscreate does not respect parent dir's perm
@ 2023-01-03 22:29 Xiao-Yong Jin
2023-01-06 14:21 ` cinap_lenrek
` (3 more replies)
0 siblings, 4 replies; 6+ messages in thread
From: Xiao-Yong Jin @ 2023-01-03 22:29 UTC (permalink / raw)
To: 9front
Under posix drawterm, creating new file or dir under /mnt/term
always ends up having perm bits 0666 or 0777. The patch below
includes the parent dir's perm in fscreate. It also fix the
uid/gid the same as the parent dir.
diff --git a/kern/devfs-posix.c b/kern/devfs-posix.c
index 31eca3c..c715072 100644
--- a/kern/devfs-posix.c
+++ b/kern/devfs-posix.c
@@ -244,8 +244,9 @@ fscreate(Chan *c, char *name, int mode, ulong perm)
fd = open(path, 0);
if(fd >= 0) {
+ perm &= uif->mode & 0777;
chmod(path, perm & 0777);
- chown(path, uif->uid, uif->uid);
+ chown(path, uif->uid, uif->gid);
}
close(fd);
@@ -260,7 +261,8 @@ fscreate(Chan *c, char *name, int mode, ulong perm)
close(fd);
fd = open(path, m);
}
- chmod(path, perm & 0777);
+ perm &= uif->mode & 0666;
+ chmod(path, perm & 0666);
chown(path, uif->uid, uif->gid);
}
if(fd < 0)
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [9front] drawterm: kern/devfs-posix: fscreate does not respect parent dir's perm
2023-01-03 22:29 [9front] drawterm: kern/devfs-posix: fscreate does not respect parent dir's perm Xiao-Yong Jin
@ 2023-01-06 14:21 ` cinap_lenrek
2023-01-06 14:44 ` cinap_lenrek
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: cinap_lenrek @ 2023-01-06 14:21 UTC (permalink / raw)
To: 9front
it looks ok, but it is still wrong no?
The create request asks the file server to create a new file
with the name supplied, in the directory (dir) represented
by fid, and requires write permission in the directory. The
owner of the file is the implied user id of the request, the
group of the file is the same as dir, and the permissions
are the value of
perm & (~0666 | (dir.perm & 0666))
if a regular file is being created and
perm & (~0777 | (dir.perm & 0777))
if a directory is being created. This means, for example,
that if the create allows read permission to others, but the
containing directory does not, then the created file will
not allow others to read the file.
we'r changing the uid to the directory owner, but it should the the uid
of the drawterm process.
--
cinap
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [9front] drawterm: kern/devfs-posix: fscreate does not respect parent dir's perm
2023-01-03 22:29 [9front] drawterm: kern/devfs-posix: fscreate does not respect parent dir's perm 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
3 siblings, 1 reply; 6+ messages in thread
From: cinap_lenrek @ 2023-01-06 14:44 UTC (permalink / raw)
To: 9front
how about this?
--- a/./kern/devfs-posix.c
+++ b/./kern/devfs-posix.c
@@ -244,8 +244,8 @@
fd = open(path, 0);
if(fd >= 0) {
- chmod(path, perm & 0777);
- chown(path, uif->uid, uif->uid);
+ chmod(path, uif->mode & perm & 0777);
+ chown(path, -1, uif->gid);
}
close(fd);
@@ -260,8 +260,8 @@
close(fd);
fd = open(path, m);
}
- chmod(path, perm & 0777);
- chown(path, uif->uid, uif->gid);
+ chmod(path, uif->mode & perm & 0666);
+ chown(path, -1, uif->gid);
}
if(fd < 0)
error(strerror(errno));
--
cinap
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [9front] drawterm: kern/devfs-posix: fscreate does not respect parent dir's perm
2023-01-06 14:44 ` cinap_lenrek
@ 2023-01-06 15:30 ` Xiao-Yong Jin
0 siblings, 0 replies; 6+ messages in thread
From: Xiao-Yong Jin @ 2023-01-06 15:30 UTC (permalink / raw)
To: 9front
better.
> On Jan 6, 2023, at 8:44 AM, cinap_lenrek@felloff.net wrote:
>
> how about this?
>
> --- a/./kern/devfs-posix.c
> +++ b/./kern/devfs-posix.c
> @@ -244,8 +244,8 @@
>
> fd = open(path, 0);
> if(fd >= 0) {
> - chmod(path, perm & 0777);
> - chown(path, uif->uid, uif->uid);
> + chmod(path, uif->mode & perm & 0777);
> + chown(path, -1, uif->gid);
> }
> close(fd);
>
> @@ -260,8 +260,8 @@
> close(fd);
> fd = open(path, m);
> }
> - chmod(path, perm & 0777);
> - chown(path, uif->uid, uif->gid);
> + chmod(path, uif->mode & perm & 0666);
> + chown(path, -1, uif->gid);
> }
> if(fd < 0)
> error(strerror(errno));
>
> --
> cinap
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [9front] drawterm: kern/devfs-posix: fscreate does not respect parent dir's perm
2023-01-03 22:29 [9front] drawterm: kern/devfs-posix: fscreate does not respect parent dir's perm Xiao-Yong Jin
2023-01-06 14:21 ` cinap_lenrek
2023-01-06 14:44 ` cinap_lenrek
@ 2023-01-06 15:40 ` cinap_lenrek
2023-01-06 18:20 ` cinap_lenrek
3 siblings, 0 replies; 6+ messages in thread
From: cinap_lenrek @ 2023-01-06 15:40 UTC (permalink / raw)
To: 9front
hm... the next question is, why do we even do the chmod(),
when we can just pass the desired permissions (masked by
the parent directory) to open?
who the hell wrote this crazy code?
--
cinap
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [9front] drawterm: kern/devfs-posix: fscreate does not respect parent dir's perm
2023-01-03 22:29 [9front] drawterm: kern/devfs-posix: fscreate does not respect parent dir's perm Xiao-Yong Jin
` (2 preceding siblings ...)
2023-01-06 15:40 ` cinap_lenrek
@ 2023-01-06 18:20 ` cinap_lenrek
3 siblings, 0 replies; 6+ messages in thread
From: cinap_lenrek @ 2023-01-06 18:20 UTC (permalink / raw)
To: 9front
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;
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-01-06 18:24 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-03 22:29 [9front] drawterm: kern/devfs-posix: fscreate does not respect parent dir's perm 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 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).