From 710e9b2b5c16f74f66420c66d12455ad518d42c7 Mon Sep 17 00:00:00 2001 From: dzwdz Date: Sun, 2 Oct 2022 19:25:17 +0200 Subject: syscall/open: add the full suite of READ/WRITE flags --- src/kernel/handle.h | 3 ++- src/kernel/syscalls.c | 34 ++++++++++++++++++++------------- src/kernel/vfs/request.c | 11 ++++++++--- src/kernel/vfs/request.h | 2 ++ src/shared/include/camellia/flags.h | 19 +++++++++++++++--- src/user/app/drawmouse/drawmouse.c | 3 ++- src/user/app/httpd/httpd.c | 4 ++-- src/user/app/iochk/iochk.c | 3 ++- src/user/app/login/login.c | 4 +++- src/user/app/netdog/nd.c | 3 ++- src/user/app/netstack/fs.c | 3 ++- src/user/app/netstack/netstack.c | 3 ++- src/user/app/shell/builtins.c | 3 ++- src/user/app/tests/kernel/misc.c | 2 +- src/user/app/tests/kernel/miscsyscall.c | 6 ++++++ src/user/lib/draw/draw.c | 3 ++- src/user/lib/fs/misc.c | 2 +- src/user/lib/fs/whitelist.c | 9 +++++++-- src/user/lib/stdio/file.c | 12 ++++++++++-- src/user/lib/stdlib.c | 2 +- src/user/lib/unistd.c | 9 ++++++--- 21 files changed, 100 insertions(+), 40 deletions(-) (limited to 'src') diff --git a/src/kernel/handle.h b/src/kernel/handle.h index 6b095e5..3c16061 100644 --- a/src/kernel/handle.h +++ b/src/kernel/handle.h @@ -19,7 +19,8 @@ struct handle { enum handle_type type; struct vfs_backend *backend; // HANDLE_FILE | HANDLE_FS_FRONT void __user *file_id; // only applicable to HANDLE_FILE - bool ro; /* currently only for HANDLE_FILE */ + // TODO readable/writeable could be reused for pipes + bool readable, writeable; /* currently only for HANDLE_FILE */ struct vfs_request *req; /* HANDLE_FS_REQ */ struct { struct process *queued; diff --git a/src/kernel/syscalls.c b/src/kernel/syscalls.c index aa3c28f..dc8f9df 100644 --- a/src/kernel/syscalls.c +++ b/src/kernel/syscalls.c @@ -83,7 +83,7 @@ handle_t _syscall_open(const char __user *path, long len, int flags) { struct vfs_mount *mount; char *path_buf = NULL; - if (flags & ~(OPEN_CREATE | OPEN_RO)) SYSCALL_RETURN(-ENOSYS); + if (flags & ~(OPEN_RW | OPEN_CREATE)) SYSCALL_RETURN(-ENOSYS); if (PATH_MAX < len) SYSCALL_RETURN(-1); @@ -184,11 +184,16 @@ static long simple_vfsop( enum vfs_operation vfsop, handle_t hid, void __user *buf, size_t len, long offset, int flags) { - assert(vfsop != VFSOP_OPEN && vfsop != VFSOP_CLOSE); + assert(vfsop == VFSOP_READ + || vfsop == VFSOP_WRITE + || vfsop == VFSOP_GETSIZE); struct handle *h = process_handle_get(process_current, hid); if (!h) SYSCALL_RETURN(-1); if (h->type == HANDLE_FILE) { - if (h->ro && !(vfsop == VFSOP_READ || vfsop == VFSOP_GETSIZE)) + // TODO those checks really need some comprehensive tests + if (!h->readable && vfsop == VFSOP_READ) + SYSCALL_RETURN(-EACCES); + if (!h->writeable && vfsop == VFSOP_WRITE) SYSCALL_RETURN(-EACCES); struct vfs_request req = (struct vfs_request){ .type = vfsop, @@ -235,19 +240,22 @@ long _syscall_getsize(handle_t hid) { long _syscall_remove(handle_t hid) { struct handle *h = process_handle_get(process_current, hid); if (!h) SYSCALL_RETURN(-EBADF); - if (!h->ro && h->type == HANDLE_FILE) { - vfsreq_create((struct vfs_request) { - .type = VFSOP_REMOVE, - .id = h->file_id, - .caller = process_current, - .backend = h->backend, - }); + if (h->type != HANDLE_FILE) { process_handle_close(process_current, hid); - return -1; // dummy - } else { + SYSCALL_RETURN(-ENOSYS); + } + if (!h->writeable) { process_handle_close(process_current, hid); - SYSCALL_RETURN(h->ro ? -EACCES : -ENOSYS); + SYSCALL_RETURN(-EACCES); } + vfsreq_create((struct vfs_request) { + .type = VFSOP_REMOVE, + .id = h->file_id, + .caller = process_current, + .backend = h->backend, + }); + process_handle_close(process_current, hid); + return -1; // dummy } long _syscall_close(handle_t hid) { diff --git a/src/kernel/vfs/request.c b/src/kernel/vfs/request.c index 24d5dac..0a377d3 100644 --- a/src/kernel/vfs/request.c +++ b/src/kernel/vfs/request.c @@ -23,8 +23,12 @@ void vfsreq_create(struct vfs_request req_) { memcpy(req, &req_, sizeof *req); if (req->backend) req->backend->refcount++; - if (req->type == VFSOP_OPEN && (req->flags & OPEN_RO)) - req->flags &= ~OPEN_CREATE; + if (req->type == VFSOP_OPEN && !(req->flags & OPEN_WRITE) && (req->flags & OPEN_CREATE)) { + vfsreq_finish_short(req, -EINVAL); + return; + } + + // TODO if i add a handle field to vfs_request, check ->readable ->writeable here if (req->backend && req->backend->potential_handlers) { struct vfs_request **iter = &req->backend->queue; @@ -47,7 +51,8 @@ void vfsreq_finish(struct vfs_request *req, char __user *stored, long ret, h = handle_init(HANDLE_FILE); h->backend = req->backend; req->backend->refcount++; h->file_id = stored; - h->ro = req->flags & OPEN_RO; + h->readable = OPEN_READABLE(req->flags); + h->writeable = OPEN_WRITEABLE(req->flags); } else { /* delegating - moving a handle to the caller */ assert(handler); diff --git a/src/kernel/vfs/request.h b/src/kernel/vfs/request.h index 4e309b0..db707d0 100644 --- a/src/kernel/vfs/request.h +++ b/src/kernel/vfs/request.h @@ -42,6 +42,8 @@ struct vfs_request { size_t len; } output; + // TODO why doesn't this just have a reference to the handle? + void __user *id; // handle.file.id long offset; int flags; diff --git a/src/shared/include/camellia/flags.h b/src/shared/include/camellia/flags.h index 0eaa02e..b7cc42d 100644 --- a/src/shared/include/camellia/flags.h +++ b/src/shared/include/camellia/flags.h @@ -10,7 +10,20 @@ #define WRITE_TRUNCATE 1 -#define OPEN_CREATE 1 -#define OPEN_RO 2 - #define FSR_DELEGATE 1 + + +#define OPEN_READ 1 +#define OPEN_WRITE 2 +#define OPEN_RW 3 +/* not setting OPEN_READ nor OPEN_WRITE works as if OPEN_READ was set, but it also checks the execute bit. + * same as in plan9. */ +#define OPEN_EXEC 0 + +#define OPEN_READABLE(flags) ((flags & 3) != OPEN_WRITE) +#define OPEN_WRITEABLE(flags) (flags & OPEN_WRITE) + +/* Requires OPEN_WRITE to be set, enforced by the kernel. + * The idea is that if all flags which allow modifying the filesystem state require + * OPEN_WRITE to be set, filesystem handlers could just check for the OPEN_WRITE flag. */ +#define OPEN_CREATE 4 diff --git a/src/user/app/drawmouse/drawmouse.c b/src/user/app/drawmouse/drawmouse.c index 47e8e8a..6ddfc79 100644 --- a/src/user/app/drawmouse/drawmouse.c +++ b/src/user/app/drawmouse/drawmouse.c @@ -1,3 +1,4 @@ +#include #include #include #include @@ -49,7 +50,7 @@ struct packet { int main(void) { char buf[64]; - handle_t fd = _syscall_open("/kdev/ps2/mouse", 15, 0); + handle_t fd = _syscall_open("/kdev/ps2/mouse", 15, OPEN_READ); if (fd < 0) { eprintf("couldn't open mouse"); return 1; diff --git a/src/user/app/httpd/httpd.c b/src/user/app/httpd/httpd.c index 9141a39..2ffc2ba 100644 --- a/src/user/app/httpd/httpd.c +++ b/src/user/app/httpd/httpd.c @@ -21,7 +21,7 @@ static void handle(FILE *c) { char *end = strchr(path, ' '); if (end) *end = '\0'; - handle_t h = _syscall_open(path, strlen(path), OPEN_RO); + handle_t h = _syscall_open(path, strlen(path), OPEN_READ); if (h < 0) { fprintf(c, "HTTP/1.1 404 Not Found\r\n\r\n"); return; @@ -68,7 +68,7 @@ int main(int argc, char **argv) { const char *path = (argc > 1) ? argv[1] : "/net/listen/0.0.0.0/tcp/80"; handle_t conn; for (;;) { - conn = _syscall_open(path, strlen(path), 0); + conn = _syscall_open(path, strlen(path), OPEN_RW); if (conn < 0) { eprintf("open('%s') failed, %d", path, conn); return 1; diff --git a/src/user/app/iochk/iochk.c b/src/user/app/iochk/iochk.c index 38e0473..92975be 100644 --- a/src/user/app/iochk/iochk.c +++ b/src/user/app/iochk/iochk.c @@ -1,4 +1,5 @@ #include +#include #include #include #include @@ -84,7 +85,7 @@ int main(int argc, char **argv) { for (; optind < argc; optind++) { const char *path = argv[optind]; verbosef("checking %s...\n", path); - handle_t h = _syscall_open(path, strlen(path), 0); + handle_t h = _syscall_open(path, strlen(path), OPEN_READ); if (h < 0) { eprintf("couldn't open %s", path); continue; diff --git a/src/user/app/login/login.c b/src/user/app/login/login.c index 5d9eb36..af6e19c 100644 --- a/src/user/app/login/login.c +++ b/src/user/app/login/login.c @@ -44,8 +44,10 @@ static void drv(const char *user) { forward_open(reqh, buf, req.len, req.flags); } else if (segcmp(buf, 1, "Users") && segcmp(buf, 3, "private")) { // /Users/*/private/** _syscall_fs_respond(reqh, NULL, -EACCES, 0); + } else if (!OPEN_WRITEABLE(req.flags)) { + forward_open(reqh, buf, req.len, req.flags); } else { - forward_open(reqh, buf, req.len, req.flags | OPEN_RO); + _syscall_fs_respond(reqh, NULL, -EACCES, 0); } break; diff --git a/src/user/app/netdog/nd.c b/src/user/app/netdog/nd.c index 363c6c6..8c16bb0 100644 --- a/src/user/app/netdog/nd.c +++ b/src/user/app/netdog/nd.c @@ -1,3 +1,4 @@ +#include #include #include #include @@ -34,7 +35,7 @@ int main(int argc, char **argv) { return 1; } - conn = _syscall_open(argv[1], strlen(argv[1]), 0); + conn = _syscall_open(argv[1], strlen(argv[1]), OPEN_RW); if (conn < 0) { eprintf("couldn't open '%s', err %u", argv[1], -conn); return -conn; diff --git a/src/user/app/netstack/fs.c b/src/user/app/netstack/fs.c index d8faadf..ad6c23c 100644 --- a/src/user/app/netstack/fs.c +++ b/src/user/app/netstack/fs.c @@ -144,7 +144,8 @@ static void fs_open(handle_t reqh, char *path, int flags) { } /* everything below ends up sending packets */ - if (flags & OPEN_RO) respond(NULL, -EACCES); + if (!OPEN_WRITEABLE(flags)) + respond(NULL, -EACCES); char *save; const char *verb, *proto, *port_s; diff --git a/src/user/app/netstack/netstack.c b/src/user/app/netstack/netstack.c index cc5247b..c0934fd 100644 --- a/src/user/app/netstack/netstack.c +++ b/src/user/app/netstack/netstack.c @@ -1,5 +1,6 @@ #include "proto.h" #include "util.h" +#include #include #include #include @@ -33,7 +34,7 @@ int main(int argc, char **argv) { eprintf("usage: netstack iface ip gateway"); return 1; } - state.raw_h = _syscall_open(argv[1], strlen(argv[1]), 0); + state.raw_h = _syscall_open(argv[1], strlen(argv[1]), OPEN_RW); if (state.raw_h < 0) { eprintf("couldn't open %s", argv[1]); return 1; diff --git a/src/user/app/shell/builtins.c b/src/user/app/shell/builtins.c index bc7731c..a23e501 100644 --- a/src/user/app/shell/builtins.c +++ b/src/user/app/shell/builtins.c @@ -1,5 +1,6 @@ #include "builtins.h" #include "shell.h" +#include #include #include #include @@ -60,7 +61,7 @@ void cmd_getsize(int argc, char **argv) { } for (int i = 1; i < argc; i++) { - handle_t h = _syscall_open(argv[i], strlen(argv[i]), 0); + handle_t h = _syscall_open(argv[i], strlen(argv[i]), OPEN_READ); if (h < 0) { eprintf("error opening %s", argv[i]); continue; diff --git a/src/user/app/tests/kernel/misc.c b/src/user/app/tests/kernel/misc.c index c0bb6b3..9ee88dd 100644 --- a/src/user/app/tests/kernel/misc.c +++ b/src/user/app/tests/kernel/misc.c @@ -28,7 +28,7 @@ static void test_efault(void) { memcpy(str2, str, 16); - test((h = _syscall_open(TMPFILEPATH, strlen(TMPFILEPATH), OPEN_CREATE))); + test((h = _syscall_open(TMPFILEPATH, strlen(TMPFILEPATH), OPEN_CREATE | OPEN_WRITE)) >= 0); test(_syscall_write(h, str, 16, 0, 0) == 16); test(_syscall_write(h, str2, 16, 0, 0) == 16); diff --git a/src/user/app/tests/kernel/miscsyscall.c b/src/user/app/tests/kernel/miscsyscall.c index a2eed24..66899b1 100644 --- a/src/user/app/tests/kernel/miscsyscall.c +++ b/src/user/app/tests/kernel/miscsyscall.c @@ -270,6 +270,11 @@ static void test_sleep(void) { } } +static void test_badopen(void) { + test(_syscall_open(TMPFILEPATH, strlen(TMPFILEPATH), OPEN_CREATE | OPEN_WRITE) >= 0); + test(_syscall_open(TMPFILEPATH, strlen(TMPFILEPATH), OPEN_CREATE) == -EINVAL); +} + void r_k_miscsyscall(void) { run_test(test_await); run_test(test_pipe); @@ -277,4 +282,5 @@ void r_k_miscsyscall(void) { run_test(test_dup); run_test(test_execbuf); run_test(test_sleep); + run_test(test_badopen); } diff --git a/src/user/lib/draw/draw.c b/src/user/lib/draw/draw.c index 27c1d33..943b8ba 100644 --- a/src/user/lib/draw/draw.c +++ b/src/user/lib/draw/draw.c @@ -1,3 +1,4 @@ +#include #include #include #include @@ -32,7 +33,7 @@ int fb_setup(struct framebuf *fb, const char *base) { /* assumes the read went correctly */ fclose(f); - fb->fd = _syscall_open(path, strlen(path), 0); + fb->fd = _syscall_open(path, strlen(path), OPEN_RW); if (fb->fd < 0) return fb->fd; fb->width = strtol(spec, &spec, 0); diff --git a/src/user/lib/fs/misc.c b/src/user/lib/fs/misc.c index f660f6f..860b312 100644 --- a/src/user/lib/fs/misc.c +++ b/src/user/lib/fs/misc.c @@ -135,7 +135,7 @@ void fs_union(const char **list) { size_t prefixlen = strlen(prefix); // TODO only open the directories once // TODO ensure trailing slash - handle_t h = _syscall_open(prefix, prefixlen, 0); + handle_t h = _syscall_open(prefix, prefixlen, OPEN_READ); if (h < 0) continue; end = end || dir_append_from(&db, h); _syscall_close(h); diff --git a/src/user/lib/fs/whitelist.c b/src/user/lib/fs/whitelist.c index 676b36f..571ebfb 100644 --- a/src/user/lib/fs/whitelist.c +++ b/src/user/lib/fs/whitelist.c @@ -1,5 +1,6 @@ #include #include +#include #include #include #include @@ -49,6 +50,7 @@ void fs_whitelist(const char **whitelist) { switch (res.op) { case VFSOP_OPEN: { + bool error = false; bool passthru = false; bool inject = false; @@ -57,8 +59,9 @@ void fs_whitelist(const char **whitelist) { size_t entry_len = suffix_parse(*entry, strlen(*entry), &ro); /* If *entry is a prefix of the opened path, pass the open() through. */ if (prefix_match(*entry, entry_len, buf, res.len)) { - if (ro) res.flags |= OPEN_RO; passthru = true; + if (ro && OPEN_WRITEABLE(res.flags)) + error = true; break; } /* If the path is a prefix of *entry, we might need to inject a directory. */ @@ -66,7 +69,9 @@ void fs_whitelist(const char **whitelist) { inject = true; } } - if (passthru) { + if (error) { + _syscall_fs_respond(reqh, NULL, -EACCES, 0); + } else if (passthru) { forward_open(reqh, buf, res.len, res.flags); } else if (inject) { // TODO all the inject points could be precomputed diff --git a/src/user/lib/stdio/file.c b/src/user/lib/stdio/file.c index 8c0fc57..f3120d7 100644 --- a/src/user/lib/stdio/file.c +++ b/src/user/lib/stdio/file.c @@ -42,8 +42,16 @@ FILE *fopen(const char *path, const char *mode) { path = tmppath; } - if (mode[0] == 'w' || mode[0] == 'a') - flags |= OPEN_CREATE; + if (strchr(mode, 'e')) { + /* camellia extension: open as executable */ + flags |= OPEN_EXEC; + } else if (strchr(mode, 'r')) { + flags |= OPEN_READ; + if (strchr(mode, '+')) + flags |= OPEN_WRITE; + } else { + flags |= OPEN_WRITE | OPEN_CREATE; + } h = _syscall_open(path, strlen(path), flags); if (tmppath) free(tmppath); diff --git a/src/user/lib/stdlib.c b/src/user/lib/stdlib.c index 38e87ad..c1ee217 100644 --- a/src/user/lib/stdlib.c +++ b/src/user/lib/stdlib.c @@ -10,7 +10,7 @@ _Noreturn void abort(void) { int mkstemp(char *template) { // TODO randomize template - handle_t h = _syscall_open(template, strlen(template), OPEN_CREATE); + handle_t h = _syscall_open(template, strlen(template), OPEN_CREATE | OPEN_RW); if (h < 0) { errno = -h; return -1; diff --git a/src/user/lib/unistd.c b/src/user/lib/unistd.c index ed8d77f..04a060d 100644 --- a/src/user/lib/unistd.c +++ b/src/user/lib/unistd.c @@ -1,3 +1,4 @@ +#include #include #include #include @@ -30,7 +31,8 @@ int unlink(const char *path) { size_t abslen = absolutepath(abspath, path, len); if (abslen == 0) { errno = EINVAL; goto err; } - handle_t h = _syscall_open(abspath, abslen - 1, 0); + // TODO take cwd into account + handle_t h = _syscall_open(abspath, abslen - 1, OPEN_WRITE); if (h < 0) { errno = -h; goto err; } long ret = _syscall_remove(h); @@ -49,7 +51,7 @@ int isatty(int fd) { int execv(const char *path, char *const argv[]) { - FILE *file = fopen(path, "r"); + FILE *file = fopen(path, "e"); char hdr[4] = {0}; if (!file) return -1; @@ -114,7 +116,8 @@ int chdir(const char *path) { cwd2[len + 1] = '\0'; } - h = _syscall_open(cwd2, strlen(cwd2), 0); + /* check if exists */ + h = _syscall_open(cwd2, strlen(cwd2), OPEN_READ); if (h < 0) { errno = ENOENT; return -1; -- cgit v1.2.3