From 0fd31698b25e08bc9b50afe51df8e5e07e8e8484 Mon Sep 17 00:00:00 2001 From: apio Date: Thu, 3 Nov 2022 16:52:21 +0100 Subject: [PATCH] Kernel: Accept not opened file descriptors in dup2() This involves renaming the descriptor_from_fd function to the more appropriately named open_descriptor_from_fd (since we check if the descriptor was opened and error out otherwise), and creating a new function that does not verify that the file descriptor was opened. --- kernel/include/thread/Task.h | 1 + kernel/src/sys/mem.cpp | 34 +++++++++++++++++----------------- kernel/src/sys/stdio.cpp | 21 +++++++++++---------- kernel/src/thread/Task.cpp | 13 ++++++++++--- 4 files changed, 39 insertions(+), 30 deletions(-) diff --git a/kernel/include/thread/Task.h b/kernel/include/thread/Task.h index 8faf4a8d..a7a4d743 100644 --- a/kernel/include/thread/Task.h +++ b/kernel/include/thread/Task.h @@ -103,6 +103,7 @@ struct Task bool is_still_blocking(); + Descriptor* open_descriptor_from_fd(int fd, int& error); Descriptor* descriptor_from_fd(int fd, int& error); bool is_superuser(); diff --git a/kernel/src/sys/mem.cpp b/kernel/src/sys/mem.cpp index b30261d4..fe9cc7c3 100644 --- a/kernel/src/sys/mem.cpp +++ b/kernel/src/sys/mem.cpp @@ -32,12 +32,8 @@ static int mman_flags_from_prot(int prot) prot &= 0b111; int flags = MAP_USER | MAP_AS_OWNED_BY_TASK; if (prot == PROT_NONE) return MAP_AS_OWNED_BY_TASK; - if ((prot & PROT_WRITE) > 0) { - flags |= MAP_READ_WRITE; - } - if ((prot & PROT_EXEC) > 0) { - flags |= MAP_EXEC; - } + if ((prot & PROT_WRITE) > 0) { flags |= MAP_READ_WRITE; } + if ((prot & PROT_EXEC) > 0) { flags |= MAP_EXEC; } return flags; } @@ -72,11 +68,11 @@ void sys_mmap(Context* context, void* address, size_t size, int prot, int fd, of return; } uint64_t addr_offset = (uint64_t)address % PAGE_SIZE; - if(fd >= 0) + if (fd >= 0) { int err; - Descriptor* file = Scheduler::current_task()->descriptor_from_fd(fd, err); - if(!file) + Descriptor* file = Scheduler::current_task()->open_descriptor_from_fd(fd, err); + if (!file) { context->rax = MAP_FAIL(err); return; @@ -99,7 +95,8 @@ void sys_mmap(Context* context, void* address, size_t size, int prot, int fd, of return; } } - kdbgln("mmap(): %ld pages at any address, %s, fd %d", Utilities::get_blocks_from_size(PAGE_SIZE, size), format_prot(prot), fd); + kdbgln("mmap(): %ld pages at any address, %s, fd %d", Utilities::get_blocks_from_size(PAGE_SIZE, size), + format_prot(prot), fd); uint64_t ptr = Scheduler::current_task()->allocator.request_virtual_pages(Utilities::get_blocks_from_size(PAGE_SIZE, size)); if (!ptr) @@ -108,11 +105,11 @@ void sys_mmap(Context* context, void* address, size_t size, int prot, int fd, of context->rax = MAP_FAIL(ENOMEM); return; } - if(fd >= 0) + if (fd >= 0) { int err; - Descriptor* file = Scheduler::current_task()->descriptor_from_fd(fd, err); - if(!file) + Descriptor* file = Scheduler::current_task()->open_descriptor_from_fd(fd, err); + if (!file) { context->rax = MAP_FAIL(err); return; @@ -172,10 +169,12 @@ void sys_munmap(Context* context, void* address, size_t size) uint64_t offset = (uint64_t)address % PAGE_SIZE; Scheduler::current_task()->allocator.free_virtual_pages(((uint64_t)address - offset), Utilities::get_blocks_from_size(PAGE_SIZE, size)); - if(flags & MAP_AS_OWNED_BY_TASK) - MemoryManager::release_pages((void*)((uint64_t)address - offset), Utilities::get_blocks_from_size(PAGE_SIZE, size)); + if (flags & MAP_AS_OWNED_BY_TASK) + MemoryManager::release_pages((void*)((uint64_t)address - offset), + Utilities::get_blocks_from_size(PAGE_SIZE, size)); else - MemoryManager::release_unaligned_mappings((void*)((uint64_t)address - offset), Utilities::get_blocks_from_size(PAGE_SIZE, size)); + MemoryManager::release_unaligned_mappings((void*)((uint64_t)address - offset), + Utilities::get_blocks_from_size(PAGE_SIZE, size)); kdbgln("munmap() succeeded"); context->rax = 0; return; @@ -219,7 +218,8 @@ void sys_mprotect(Context* context, void* address, size_t size, int prot) uint64_t offset = (uint64_t)address % PAGE_SIZE; MemoryManager::protect((void*)((uint64_t)address - offset), Utilities::get_blocks_from_size(PAGE_SIZE, size), - flags & MAP_AS_OWNED_BY_TASK ? mman_flags_from_prot(prot) : mman_flags_from_prot(prot) & ~(MAP_AS_OWNED_BY_TASK)); + flags & MAP_AS_OWNED_BY_TASK ? mman_flags_from_prot(prot) + : mman_flags_from_prot(prot) & ~(MAP_AS_OWNED_BY_TASK)); kdbgln("mprotect() succeeded"); context->rax = 0; return; diff --git a/kernel/src/sys/stdio.cpp b/kernel/src/sys/stdio.cpp index af95e628..8c1a735a 100644 --- a/kernel/src/sys/stdio.cpp +++ b/kernel/src/sys/stdio.cpp @@ -38,7 +38,7 @@ void sys_fcntl(Context* context, int fd, int command, uintptr_t arg) { Task* current_task = Scheduler::current_task(); int err; - Descriptor* file = current_task->descriptor_from_fd(fd, err); + Descriptor* file = current_task->open_descriptor_from_fd(fd, err); if (!file) { context->rax = -err; @@ -102,7 +102,7 @@ void sys_seek(Context* context, int fd, long offset, int whence) return; } int err; - Descriptor* file = Scheduler::current_task()->descriptor_from_fd(fd, err); + Descriptor* file = Scheduler::current_task()->open_descriptor_from_fd(fd, err); if (!file) { context->rax = -err; @@ -144,7 +144,7 @@ void sys_write(Context* context, int fd, size_t size, const char* addr) return; } int err; - Descriptor* file = Scheduler::current_task()->descriptor_from_fd(fd, err); + Descriptor* file = Scheduler::current_task()->open_descriptor_from_fd(fd, err); if (!file) { context->rax = -err; @@ -155,7 +155,8 @@ void sys_write(Context* context, int fd, size_t size, const char* addr) 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)); // FIXME: Handle big buffers and invalid addresses. context->rax = (size_t)result; return; } @@ -274,13 +275,13 @@ void sys_read(Context* context, int fd, size_t size, char* buffer) return; } int err; - Descriptor* file = Scheduler::current_task()->descriptor_from_fd(fd, err); + Descriptor* file = Scheduler::current_task()->open_descriptor_from_fd(fd, err); if (!file) { context->rax = -err; return; } - if (!file->is_open() || !file->can_read()) + if (!file->can_read()) { context->rax = -EBADF; return; @@ -310,7 +311,7 @@ void sys_read(Context* context, int fd, size_t size, char* buffer) void sys_close(Context* context, int fd) { int err; - Descriptor* file = Scheduler::current_task()->descriptor_from_fd(fd, err); + Descriptor* file = Scheduler::current_task()->open_descriptor_from_fd(fd, err); if (!file) { context->rax = -err; @@ -352,13 +353,13 @@ void sys_access(Context* context, const char* path, int) // FIXME: Use the amode void sys_dup2(Context* context, int fd, int fd2) { int err; - Descriptor* file1 = Scheduler::current_task()->descriptor_from_fd(fd, err); + Descriptor* file1 = Scheduler::current_task()->open_descriptor_from_fd(fd, err); if (!file1) { context->rax = -err; return; } - Descriptor* file2 = Scheduler::current_task()->descriptor_from_fd(fd2, err); // FIXME: This will return EBADF if fd2 is not open, but we want to replace it with fd1 if it is not open. + Descriptor* file2 = Scheduler::current_task()->descriptor_from_fd(fd2, err); if (!file2) { context->rax = -err; @@ -380,7 +381,7 @@ void sys_umask(Context* context, mode_t cmask) void sys_ioctl(Context* context, int fd, int cmd, uintptr_t arg) { int err; - Descriptor* file = Scheduler::current_task()->descriptor_from_fd(fd, err); + Descriptor* file = Scheduler::current_task()->open_descriptor_from_fd(fd, err); if (!file) { context->rax = -err; diff --git a/kernel/src/thread/Task.cpp b/kernel/src/thread/Task.cpp index f91dde1b..29b11ced 100644 --- a/kernel/src/thread/Task.cpp +++ b/kernel/src/thread/Task.cpp @@ -112,14 +112,21 @@ bool Task::is_still_blocking() } } -Descriptor* Task::descriptor_from_fd(int fd, int& error) +Descriptor* Task::open_descriptor_from_fd(int fd, int& error) { - if (fd < 0 || fd >= TASK_MAX_FDS) + Descriptor* file = descriptor_from_fd(fd, error); + if (!file) return nullptr; + if (!file->is_open()) { error = EBADF; return nullptr; } - if (!files[fd].is_open()) + return file; +} + +Descriptor* Task::descriptor_from_fd(int fd, int& error) +{ + if (fd < 0 || fd >= TASK_MAX_FDS) { error = EBADF; return nullptr;