From af46b8d9ac84f0bc0699af37ca9573616ce5c097 Mon Sep 17 00:00:00 2001 From: apio Date: Tue, 25 Oct 2022 18:35:17 +0200 Subject: [PATCH] Kernel: Cleanup file descriptor validation --- kernel/include/thread/Task.h | 2 + kernel/src/sys/stdio.cpp | 100 +++++++++++++++-------------------- kernel/src/thread/Task.cpp | 16 ++++++ 3 files changed, 60 insertions(+), 58 deletions(-) diff --git a/kernel/include/thread/Task.h b/kernel/include/thread/Task.h index ae2f109e..c0655535 100644 --- a/kernel/include/thread/Task.h +++ b/kernel/include/thread/Task.h @@ -79,4 +79,6 @@ struct Task void resume_read(); bool is_still_blocking(); + + Descriptor* descriptor_from_fd(int fd, int& error); }; \ No newline at end of file diff --git a/kernel/src/sys/stdio.cpp b/kernel/src/sys/stdio.cpp index 40727a0a..7b615988 100644 --- a/kernel/src/sys/stdio.cpp +++ b/kernel/src/sys/stdio.cpp @@ -27,18 +27,14 @@ void sys_fcntl(Context* context, int fd, int command, uintptr_t arg) { - if (fd >= TASK_MAX_FDS || fd < 0) - { - context->rax = -EBADF; - return; - } Task* current_task = Scheduler::current_task(); - if (!current_task->files[fd].is_open()) + int err; + Descriptor* file = current_task->descriptor_from_fd(fd, err); + if (!file) { - context->rax = -EBADF; + context->rax = -err; return; } - Descriptor& file = current_task->files[fd]; if (command == FCNTL_DUPFD) { if ((int)arg < 0 || (int)arg >= TASK_MAX_FDS) @@ -52,14 +48,14 @@ void sys_fcntl(Context* context, int fd, int command, uintptr_t arg) context->rax = -EMFILE; return; } - current_task->files[dupfd] = file; + current_task->files[dupfd] = *file; context->rax = dupfd; kdbgln("fcntl(F_DUPFD): duplicated fd %d, result is %d", fd, dupfd); return; } else if (command == FCNTL_ISTTY) { - VFS::Node* node = file.node(); + VFS::Node* node = file->node(); if (node->tty) { context->rax = 1; } else context->rax = -ENOTTY; @@ -79,24 +75,19 @@ void sys_seek(Context* context, int fd, long offset, int whence) context->rax = -EINVAL; return; } - if (fd >= TASK_MAX_FDS || fd < 0) + int err; + Descriptor* file = Scheduler::current_task()->descriptor_from_fd(fd, err); + if (!file) { - context->rax = -EBADF; + context->rax = -err; return; } - Task* current_task = Scheduler::current_task(); - if (!current_task->files[fd].is_open()) - { - context->rax = -EBADF; - return; - } - Descriptor& file = current_task->files[fd]; long new_offset; if (whence == SEEK_SET) new_offset = offset; else if (whence == SEEK_CUR) - new_offset = offset + file.offset(); + new_offset = offset + file->offset(); else if (whence == SEEK_END) - new_offset = file.length() + offset; + new_offset = file->length() + offset; else __builtin_unreachable(); if (new_offset < 0) @@ -104,12 +95,12 @@ void sys_seek(Context* context, int fd, long offset, int whence) context->rax = -EINVAL; // FIXME: Is this the right error? return; } - if (new_offset == file.offset()) + if (new_offset == file->offset()) { context->rax = new_offset; return; } - int result = file.seek(new_offset); + int result = file->seek(new_offset); if (result < 0) { context->rax = result; @@ -126,24 +117,19 @@ void sys_write(Context* context, int fd, size_t size, const char* addr) context->rax = -EFAULT; return; } - if (fd >= TASK_MAX_FDS || fd < 0) + int err; + Descriptor* file = Scheduler::current_task()->descriptor_from_fd(fd, err); + if (!file) + { + context->rax = -err; + return; + } + if (!file->can_write()) { context->rax = -EBADF; return; } - Task* current_task = Scheduler::current_task(); - if (!current_task->files[fd].is_open()) - { - context->rax = -EBADF; - return; - } - Descriptor& file = current_task->files[fd]; - if (!file.can_write()) - { - context->rax = -EBADF; - return; - } - ssize_t result = file.write(size, (const char*)VMM::get_physical((uint64_t)addr)); + ssize_t result = file->write(size, (const char*)VMM::get_physical((uint64_t)addr)); context->rax = (size_t)result; return; } @@ -213,52 +199,50 @@ void sys_read(Context* context, int fd, size_t size, char* buffer) context->rax = -EFAULT; return; } - if (fd >= TASK_MAX_FDS || fd < 0) + int err; + Descriptor* file = Scheduler::current_task()->descriptor_from_fd(fd, err); + if (!err) + { + context->rax = -err; + return; + } + if (!file->is_open() || !file->can_read()) { context->rax = -EBADF; return; } - Task* current_task = Scheduler::current_task(); - if (!current_task->files[fd].is_open() || !current_task->files[fd].can_read()) + if (VFS::would_block(file->node())) { - context->rax = -EBADF; - return; - } - if (VFS::would_block(current_task->files[fd].node())) - { - if (!current_task->files[fd].able_to_block()) + if (!file->able_to_block()) { context->rax = -EAGAIN; return; } + Task* current_task = Scheduler::current_task(); current_task->state = current_task->Blocking; current_task->blocking_read_info.fd = fd; current_task->blocking_read_info.buf = (char*)VMM::get_physical((uint64_t)buffer); // FIXME: Handle errors. current_task->blocking_read_info.size = size; return Scheduler::task_yield(context); } - ssize_t result = current_task->files[fd].read( - size, (char*)VMM::get_physical((uint64_t)buffer)); // FIXME: Handle errors, and big buffers which may not be - // across continuous physical pages. + ssize_t result = + file->read(size, (char*)VMM::get_physical((uint64_t)buffer)); // FIXME: Handle errors, and big buffers which may + // not be across continuous physical pages. context->rax = (size_t)result; return; } void sys_close(Context* context, int fd) { - if (fd >= TASK_MAX_FDS || fd < 0) + int err; + Descriptor* file = Scheduler::current_task()->descriptor_from_fd(fd, err); + if (!err) { - context->rax = -EBADF; - return; - } - Task* current_task = Scheduler::current_task(); - if (!current_task->files[fd].is_open()) - { - context->rax = -EBADF; + context->rax = -err; return; } kdbgln("close(): releasing file descriptor %d", fd); - current_task->files[fd].close(); + file->close(); context->rax = 0; return; } diff --git a/kernel/src/thread/Task.cpp b/kernel/src/thread/Task.cpp index ec404f6f..392f352c 100644 --- a/kernel/src/thread/Task.cpp +++ b/kernel/src/thread/Task.cpp @@ -3,6 +3,7 @@ #include "thread/Task.h" #include "log/Log.h" #include "memory/VMM.h" +#include "std/errno.h" #include "std/string.h" void Task::restore_context(Context* context) @@ -82,4 +83,19 @@ void Task::resume_read() bool Task::is_still_blocking() { return VFS::would_block(files[blocking_read_info.fd].node()); +} + +Descriptor* Task::descriptor_from_fd(int fd, int& error) +{ + if (fd < 0 || fd >= TASK_MAX_FDS) + { + error = EBADF; + return nullptr; + } + if (!files[fd].is_open()) + { + error = EBADF; + return nullptr; + } + return &files[fd]; } \ No newline at end of file