From 8a7ddfca80850b5261a5552e0f4263d4997637cd Mon Sep 17 00:00:00 2001 From: apio Date: Wed, 12 Oct 2022 18:43:48 +0200 Subject: [PATCH] exec: Use check_elf_image() This allows exec to recover if an error should occur when loading the executable. Thus, the calling process will be notified instead of killed. --- kernel/src/sys/exec.cpp | 40 ++++++++++++++++++---------------------- 1 file changed, 18 insertions(+), 22 deletions(-) diff --git a/kernel/src/sys/exec.cpp b/kernel/src/sys/exec.cpp index b7c1313f..c9663df5 100644 --- a/kernel/src/sys/exec.cpp +++ b/kernel/src/sys/exec.cpp @@ -26,6 +26,18 @@ void sys_exec(Context* context, const char* pathname) return; } + if (program->type == VFS_DIRECTORY) + { + context->rax = -EISDIR; + return; + } + + if (ELFLoader::check_elf_image(program) < 0) + { + context->rax = -EINVAL; // FIXME: Should be ENOEXEC. + return; + } + uint64_t allocated_stack = (uint64_t)MemoryManager::get_pages(TASK_PAGES_IN_STACK, MAP_READ_WRITE | MAP_USER); if (!allocated_stack) { @@ -33,28 +45,6 @@ void sys_exec(Context* context, const char* pathname) return; } - char* kpathname = strdup( - pathname); // Since we are going to free the original task's memory, we cannot use anything coming from it. - - // FIXME: This should be done later, but since there is a very high chance that loading the executed program's - // ELF image will overwrite ours, we have to do it here. - ELFLoader::release_elf_image(Scheduler::current_task()->image); - - // FIXME: Check the ELF image is valid before loading it into memory. This will allow us to first check, then free - // the previous image, then load, which should reduce the chances of loading failing to almost zero. - ELFImage* image = ELFLoader::load_elf_from_filesystem(kpathname); - if (!image) - { - MemoryManager::release_pages((void*)allocated_stack, TASK_PAGES_IN_STACK); - Scheduler::current_task()->image = nullptr; - kfree(kpathname); - kerrorln("exec(): ERROR: Failed to load program. Previous program has already been freed, thus cannot " - "return to it."); - return Scheduler::task_exit(context, -255); - } - - kfree(kpathname); - Interrupts::disable(); ASSERT(!Interrupts::are_enabled()); // This part is pretty sensitive. @@ -63,6 +53,12 @@ void sys_exec(Context* context, const char* pathname) // At this point, pretty much nothing can fail. + ELFLoader::release_elf_image(Scheduler::current_task()->image); + + ELFImage* image = ELFLoader::load_elf_from_vfs(program); + ASSERT(image); // If check_elf_image succeeded, load_elf_from_vfs MUST succeed, unless something has gone terribly + // wrong. + MemoryManager::release_pages((void*)task->allocated_stack, TASK_PAGES_IN_STACK); task->allocated_stack = allocated_stack;