From a3d15c48c811c089154f24488b0930d777dd2883 Mon Sep 17 00:00:00 2001 From: Aleksander Kaminski Date: Fri, 4 Oct 2024 17:56:30 +0200 Subject: [PATCH] syscalls: Sanitize all pointers DONE: RTOS-939 --- syscalls.c | 134 ++++++++++++++++++++++++++++++++++++++++++++--------- vm/map.c | 30 +++++++++++- vm/map.h | 3 ++ 3 files changed, 145 insertions(+), 22 deletions(-) diff --git a/syscalls.c b/syscalls.c index fc991c893..1a7703eac 100644 --- a/syscalls.c +++ b/syscalls.c @@ -36,12 +36,14 @@ void syscalls_debug(void *ustack) { + process_t *proc = proc_current()->process; const char *s; - /* FIXME: pass strlen(s) from userspace */ - GETFROMSTACK(ustack, const char *, s, 0); - hal_consolePrint(ATTR_USER, s); + + if (vm_mapStringBelongs(proc, s) == 0) { + hal_consolePrint(ATTR_USER, s); + } } @@ -173,52 +175,119 @@ int syscalls_release(void *ustack) } +static int syscalls_sanitizeVector(process_t *proc, char **v) +{ + size_t i; + + if (v == NULL) { + return 0; + } + + if (vm_mapBelongs(proc, v, sizeof(v)) < 0) { + return -1; + } + + for (i = 0;; ++i) { + if (vm_mapBelongs(proc, &v[i], sizeof(v)) < 0) { + return -1; + } + + if (v[i] == NULL) { + break; + } + + if (vm_mapStringBelongs(proc, v[i]) < 0) { + return -1; + } + } + + return 0; +} + + int syscalls_sys_spawn(void *ustack) { + process_t *proc = proc_current()->process; char *path; char **argv; char **envp; - /* FIXME pass fields lengths from userspace */ - GETFROMSTACK(ustack, char *, path, 0); GETFROMSTACK(ustack, char **, argv, 1); GETFROMSTACK(ustack, char **, envp, 2); + if (vm_mapStringBelongs(proc, path) < 0) { + return -EFAULT; + } + + if (syscalls_sanitizeVector(proc, argv) < 0) { + return -EFAULT; + } + + if (syscalls_sanitizeVector(proc, envp) < 0) { + return -EFAULT; + } + return proc_fileSpawn(path, argv, envp); } int syscalls_exec(void *ustack) { + process_t *proc = proc_current()->process; char *path; char **argv; char **envp; - /* FIXME pass fields lengths from userspace */ - GETFROMSTACK(ustack, char *, path, 0); GETFROMSTACK(ustack, char **, argv, 1); GETFROMSTACK(ustack, char **, envp, 2); + if (vm_mapStringBelongs(proc, path) < 0) { + return -EFAULT; + } + + if (syscalls_sanitizeVector(proc, argv) < 0) { + return -EFAULT; + } + + if (syscalls_sanitizeVector(proc, envp) < 0) { + return -EFAULT; + } + return proc_execve(path, argv, envp); } int syscalls_spawnSyspage(void *ustack) { + process_t *proc = proc_current()->process; char *imap; char *dmap; char *name; char **argv; - /* FIXME pass fields lengths from userspace */ - GETFROMSTACK(ustack, char *, imap, 0); GETFROMSTACK(ustack, char *, dmap, 1); GETFROMSTACK(ustack, char *, name, 2); GETFROMSTACK(ustack, char **, argv, 3); + if (vm_mapStringBelongs(proc, imap) < 0) { + return -EFAULT; + } + + if (vm_mapStringBelongs(proc, dmap) < 0) { + return -EFAULT; + } + + if (vm_mapStringBelongs(proc, name) < 0) { + return -EFAULT; + } + + if (syscalls_sanitizeVector(proc, argv) < 0) { + return -EFAULT; + } + return proc_syspageSpawnName(imap, dmap, name, argv); } @@ -729,7 +798,9 @@ u32 syscalls_portRegister(void *ustack) GETFROMSTACK(ustack, char *, name, 1); GETFROMSTACK(ustack, oid_t *, oid, 2); - /* FIXME: Pass strlen(name) from userspace */ + if (vm_mapStringBelongs(proc, name) < 0) { + return -EFAULT; + } if (vm_mapBelongs(proc, oid, sizeof(*oid)) < 0) { return -EFAULT; @@ -828,7 +899,9 @@ int syscalls_lookup(void *ustack) GETFROMSTACK(ustack, oid_t *, file, 1); GETFROMSTACK(ustack, oid_t *, dev, 2); - /* FIXME: Pass strlen(name) from userspace */ + if (vm_mapStringBelongs(proc, name) < 0) { + return -EFAULT; + } if ((file != NULL) && (vm_mapBelongs(proc, file, sizeof(*file)) < 0)) { return -EFAULT; @@ -1066,14 +1139,17 @@ void syscalls_sigreturn(void *ustack) int syscalls_sys_open(char *ustack) { + process_t *proc = proc_current()->process; const char *filename; int oflag; - /* FIXME: pass strlen(filename) from userspace */ - GETFROMSTACK(ustack, const char *, filename, 0); GETFROMSTACK(ustack, int, oflag, 1); + if (vm_mapStringBelongs(proc, filename) < 0) { + return -EFAULT; + } + return posix_open(filename, oflag, ustack); } @@ -1158,26 +1234,36 @@ int syscalls_sys_dup2(char *ustack) int syscalls_sys_link(char *ustack) { + process_t *proc = proc_current()->process; const char *path1; const char *path2; - /* FIXME pass strlen(path1) and strlen(path2) from userspace */ - GETFROMSTACK(ustack, const char *, path1, 0); GETFROMSTACK(ustack, const char *, path2, 1); + if (vm_mapStringBelongs(proc, path1) < 0) { + return -EFAULT; + } + + if (vm_mapStringBelongs(proc, path2) < 0) { + return -EFAULT; + } + return posix_link(path1, path2); } int syscalls_sys_unlink(char *ustack) { + process_t *proc = proc_current()->process; const char *pathname; - /* FIXME: pass strlen(pathname) from userspace */ - GETFROMSTACK(ustack, const char *, pathname, 0); + if (vm_mapStringBelongs(proc, pathname) < 0) { + return -EFAULT; + } + return posix_unlink(pathname); } @@ -1242,14 +1328,17 @@ int syscalls_sys_pipe(char *ustack) int syscalls_sys_mkfifo(char *ustack) { + process_t *proc = proc_current()->process; const char *path; mode_t mode; - /* FIXME: pass strlen(path) from userspace */ - GETFROMSTACK(ustack, const char *, path, 0); GETFROMSTACK(ustack, mode_t, mode, 1); + if (vm_mapStringBelongs(proc, path) < 0) { + return -EFAULT; + } + return posix_mkfifo(path, mode); } @@ -1283,14 +1372,17 @@ int syscalls_sys_fsync(char *ustack) int syscalls_sys_chmod(char *ustack) { + process_t *proc = proc_current()->process; const char *path; mode_t mode; - /* FIXME: pass strlen(path) from userspace */ - GETFROMSTACK(ustack, const char *, path, 0); GETFROMSTACK(ustack, mode_t, mode, 1); + if (vm_mapStringBelongs(proc, path) < 0) { + return -EFAULT; + } + return posix_chmod(path, mode); } diff --git a/vm/map.c b/vm/map.c index 11fb3b338..9ce520db1 100644 --- a/vm/map.c +++ b/vm/map.c @@ -1105,7 +1105,9 @@ static int _vm_mapBelongs(const struct _process_t *proc, const void *ptr, size_t return 0; } - +/* FIXME checked pages should be marked somehow. + * Right now they might be unmapped by some other + * thread in the user process */ int vm_mapBelongs(const struct _process_t *proc, const void *ptr, size_t size) { int ret; @@ -1120,6 +1122,32 @@ int vm_mapBelongs(const struct _process_t *proc, const void *ptr, size_t size) } +int vm_mapStringBelongs(const struct _process_t *proc, const char *str) +{ + const char *page = (const char *)((ptr_t)str & (SIZE_PAGE - 1)); + size_t offs = str - page; + + proc_lockSet(&proc->mapp->lock); + + while (_vm_mapBelongs(proc, page, SIZE_PAGE) == 0) { + while (offs < SIZE_PAGE) { + if (page[offs] == '\0') { + proc_lockClear(&proc->mapp->lock); + return 0; + } + + ++offs; + } + + page += SIZE_PAGE; + offs = 0; + } + proc_lockClear(&proc->mapp->lock); + + return -1; +} + + void vm_mapinfo(meminfo_t *info) { rbnode_t *n; diff --git a/vm/map.h b/vm/map.h index 6bd011ea4..9d9e36cfe 100644 --- a/vm/map.h +++ b/vm/map.h @@ -130,6 +130,9 @@ extern vm_map_t *vm_getSharedMap(int map); extern int vm_mapBelongs(const struct _process_t *proc, const void *ptr, size_t size); +extern int vm_mapStringBelongs(const struct _process_t *proc, const char *str); + + extern int _map_init(vm_map_t *kmap, struct _vm_object_t *kernel, void **start, void **end);