From d996f88bfda890df5d2b76e7c06cae329e04ab00 Mon Sep 17 00:00:00 2001 From: dzwdz Date: Sun, 1 May 2022 20:09:52 +0200 Subject: kernel/proc: make handles separate refcounted objects --- src/kernel/handle.c | 27 +++++++++++++++++++++++++++ src/kernel/handle.h | 7 ++++++- src/kernel/proc.c | 20 +++++++++++++++----- src/kernel/proc.h | 4 ++-- src/kernel/syscalls.c | 38 ++++++++++++++------------------------ src/kernel/vfs/request.c | 14 ++++++-------- 6 files changed, 70 insertions(+), 40 deletions(-) create mode 100644 src/kernel/handle.c (limited to 'src') diff --git a/src/kernel/handle.c b/src/kernel/handle.c new file mode 100644 index 0000000..d7f19b3 --- /dev/null +++ b/src/kernel/handle.c @@ -0,0 +1,27 @@ +#include +#include +#include +#include + +struct handle *handle_init(enum handle_type type) { + struct handle *h = kmalloc(sizeof *h); + h->type = type; + h->refcount = 1; + return h; +} + +void handle_close(struct handle *h) { + if (!h) return; + assert(h->refcount > 0); + if (--(h->refcount) == 0) { + // TODO call close() in handler + // TODO count allocations and frees + + // TODO sanity check to check if refcount is true. handle_sanity? + + // TODO tests which would catch premature frees + // by that i mean duplicating a handle and killing the original process + h->type = HANDLE_INVALID; + kfree(h); + } +} diff --git a/src/kernel/handle.h b/src/kernel/handle.h index 774e633..86c1dbe 100644 --- a/src/kernel/handle.h +++ b/src/kernel/handle.h @@ -6,7 +6,7 @@ #define HANDLE_MAX 16 enum handle_type { - HANDLE_EMPTY = 0, // by design - handle structs start out NULLed out + HANDLE_INVALID = 0, HANDLE_FILE, HANDLE_FS_FRONT, @@ -23,4 +23,9 @@ struct handle { struct vfs_backend *backend; } fs; }; + + size_t refcount; }; + +struct handle *handle_init(enum handle_type); +void handle_close(struct handle *); diff --git a/src/kernel/proc.c b/src/kernel/proc.c index 57b9669..79c4ed7 100644 --- a/src/kernel/proc.c +++ b/src/kernel/proc.c @@ -64,7 +64,13 @@ struct process *process_fork(struct process *parent) { if (child->controlled) child->controlled->potential_handlers++; - child->id = next_pid++; + for (handle_t h = 0; h < HANDLE_MAX; h++) { + if (child->handles[h]) + child->handles[h]->refcount++; + // no overflow check - if you manage to get 2^32 references to a handle you have bigger problems + } + + child->id = next_pid++; return child; } @@ -191,13 +197,14 @@ size_t process_find_multiple(enum process_state target, struct process **buf, si return i; } -handle_t process_find_handle(struct process *proc) { +handle_t process_find_handle(struct process *proc, handle_t start_at) { + // TODO start_at is a bit of a hack handle_t handle; - for (handle = 0; handle < HANDLE_MAX; handle++) { - if (proc->handles[handle].type == HANDLE_EMPTY) + for (handle = start_at; handle < HANDLE_MAX; handle++) { + if (proc->handles[handle] == NULL) break; } - if (handle == HANDLE_MAX) handle = -1; + if (handle >= HANDLE_MAX) handle = -1; return handle; } @@ -293,6 +300,9 @@ void process_kill(struct process *p, int ret) { kprintf("process_kill unexpected state 0x%x\n", p->state); panic_invalid_state(); } + + for (handle_t h = 0; h < HANDLE_MAX; h++) + handle_close(p->handles[h]); process_transition(p, PS_DEAD); p->death_msg = ret; process_try2collect(p); diff --git a/src/kernel/proc.h b/src/kernel/proc.h index 4fabe0e..7e3cdd3 100644 --- a/src/kernel/proc.h +++ b/src/kernel/proc.h @@ -55,7 +55,7 @@ struct process { struct vfs_mount *mount; - struct handle handles[HANDLE_MAX]; + struct handle *handles[HANDLE_MAX]; }; extern struct process *process_first; @@ -75,7 +75,7 @@ struct process *process_next(struct process *); struct process *process_find(enum process_state); size_t process_find_multiple(enum process_state, struct process **buf, size_t max); -handle_t process_find_handle(struct process *proc); // finds the first free handle +handle_t process_find_handle(struct process *proc, handle_t start_at); // finds the first free handle void process_transition(struct process *, enum process_state); diff --git a/src/kernel/syscalls.c b/src/kernel/syscalls.c index b21eb5b..b6d1007 100644 --- a/src/kernel/syscalls.c +++ b/src/kernel/syscalls.c @@ -47,7 +47,7 @@ handle_t _syscall_open(const char __user *path, int len) { if (PATH_MAX < len) return -1; - if (process_find_handle(process_current) < 0) + if (process_find_handle(process_current, 0) < 0) return -1; path_buf = virt_cpy2kmalloc(process_current->pages, path, len); @@ -103,11 +103,14 @@ int _syscall_mount(handle_t handle, const char __user *path, int len) { } if (handle >= 0) { // mounting a real backend? + // TODO macro/function for validating handles, this is ridiculous if (handle >= HANDLE_MAX) goto fail; - if (process_current->handles[handle].type != HANDLE_FS_FRONT) + if (!process_current->handles[handle]) goto fail; - backend = process_current->handles[handle].fs.backend; + if (process_current->handles[handle]->type != HANDLE_FS_FRONT) + goto fail; + backend = process_current->handles[handle]->fs.backend; } // otherwise backend == NULL // append to mount list @@ -126,9 +129,9 @@ fail: } int _syscall_read(handle_t handle_num, void __user *buf, size_t len, int offset) { - struct handle *handle = &process_current->handles[handle_num]; if (handle_num < 0 || handle_num >= HANDLE_MAX) return -1; - if (handle->type != HANDLE_FILE) return -1; + struct handle *handle = process_current->handles[handle_num]; + if (handle == NULL || handle->type != HANDLE_FILE) return -1; return vfs_request_create((struct vfs_request) { .type = VFSOP_READ, .output = { @@ -143,9 +146,9 @@ int _syscall_read(handle_t handle_num, void __user *buf, size_t len, int offset) } int _syscall_write(handle_t handle_num, const void __user *buf, size_t len, int offset) { - struct handle *handle = &process_current->handles[handle_num]; if (handle_num < 0 || handle_num >= HANDLE_MAX) return -1; - if (handle->type != HANDLE_FILE) return -1; + struct handle *handle = process_current->handles[handle_num]; + if (handle == NULL || handle->type != HANDLE_FILE) return -1; return vfs_request_create((struct vfs_request) { .type = VFSOP_WRITE, .input = { @@ -169,23 +172,10 @@ handle_t _syscall_fs_fork2(void) { struct process *child; handle_t front; - front = process_find_handle(process_current); + front = process_find_handle(process_current, 1); if (front < 0) return -1; - process_current->handles[front].type = HANDLE_FS_FRONT; - - /* so this can't return 0 in the parent or it will make it think that it's - * the child. i could make fork()s return -1 in the child but that's weird. - * - * also there's this whole thing with handling errors here properly and - * errno - * TODO figure this out */ - if (front == 0) { - // dumb hack - front = process_find_handle(process_current); - if (front < 0) return -1; - process_current->handles[front].type = HANDLE_FS_FRONT; - process_current->handles[0].type = HANDLE_EMPTY; - } + process_current->handles[front] = handle_init(HANDLE_FS_FRONT); + process_current->handles[front]->type = HANDLE_FS_FRONT; backend = kmalloc(sizeof *backend); // TODO never freed backend->type = VFS_BACK_USER; @@ -197,7 +187,7 @@ handle_t _syscall_fs_fork2(void) { child->controlled = backend; regs_savereturn(&child->regs, 0); - process_current->handles[front].fs.backend = backend; + process_current->handles[front]->fs.backend = backend; return front; } diff --git a/src/kernel/vfs/request.c b/src/kernel/vfs/request.c index b3769cf..5ee1678 100644 --- a/src/kernel/vfs/request.c +++ b/src/kernel/vfs/request.c @@ -75,16 +75,14 @@ int vfs_request_finish(struct vfs_request *req, int ret) { // open() calls need special handling // we need to wrap the id returned by the VFS in a handle passed to // the client - handle_t handle = process_find_handle(req->caller); + handle_t handle = process_find_handle(req->caller, 0); if (handle < 0) panic_invalid_state(); // we check for free handles before the open() call - req->caller->handles[handle] = (struct handle){ - .type = HANDLE_FILE, - .file = { - .backend = req->backend, - .id = ret, - }, - }; + + struct handle *backing = handle_init(HANDLE_FILE); + backing->file.backend = req->backend; + backing->file.id = ret; + req->caller->handles[handle] = backing; ret = handle; } -- cgit v1.2.3