From 2dce0a04153b1d28144e2c548de1b64a2eab5166 Mon Sep 17 00:00:00 2001 From: Bananymous Date: Sun, 23 Jul 2023 13:09:04 +0300 Subject: [PATCH] Kernel: Userspace signal handlers are now called one at a time I added a syscall for telling the kernel when signal execution has finished. We should send a random hash or id to the signal trampoline that we would include in the syscall, so validity of signal exit can be confirmed. --- kernel/CMakeLists.txt | 1 + kernel/arch/x86_64/Signal.S | 3 +++ kernel/include/kernel/Thread.h | 1 + kernel/kernel/Signal.cpp | 8 ++++++++ kernel/kernel/Syscall.cpp | 9 +++++++++ kernel/kernel/Thread.cpp | 24 ++++++++++++++++++------ libc/include/sys/syscall.h | 1 + libc/unistd.cpp | 3 +++ userspace/Shell/main.cpp | 33 +++++++++++++++++++++++++++++++++ 9 files changed, 77 insertions(+), 6 deletions(-) create mode 100644 kernel/kernel/Signal.cpp diff --git a/kernel/CMakeLists.txt b/kernel/CMakeLists.txt index a527569285..3e744aaf26 100644 --- a/kernel/CMakeLists.txt +++ b/kernel/CMakeLists.txt @@ -46,6 +46,7 @@ set(KERNEL_SOURCES kernel/Scheduler.cpp kernel/Semaphore.cpp kernel/Serial.cpp + kernel/Signal.cpp kernel/SpinLock.cpp kernel/SSP.cpp kernel/Storage/ATABus.cpp diff --git a/kernel/arch/x86_64/Signal.S b/kernel/arch/x86_64/Signal.S index 8651010856..60546390fa 100644 --- a/kernel/arch/x86_64/Signal.S +++ b/kernel/arch/x86_64/Signal.S @@ -24,6 +24,9 @@ signal_trampoline: movq 120(%rsp), %rax call *%rax + movq 128(%rsp), %rdi + call signal_done + popq %r15 popq %r14 popq %r13 diff --git a/kernel/include/kernel/Thread.h b/kernel/include/kernel/Thread.h index 4bf01aa951..35b5ae8728 100644 --- a/kernel/include/kernel/Thread.h +++ b/kernel/include/kernel/Thread.h @@ -38,6 +38,7 @@ namespace Kernel void setup_exec(); bool has_signal_to_execute() const; + void set_signal_done(int signal); void handle_next_signal(); void handle_signal(int signal, uintptr_t& return_rsp, uintptr_t& return_rip); diff --git a/kernel/kernel/Signal.cpp b/kernel/kernel/Signal.cpp new file mode 100644 index 0000000000..84eff01d41 --- /dev/null +++ b/kernel/kernel/Signal.cpp @@ -0,0 +1,8 @@ +#include +#include + +extern "C" __attribute__((section(".userspace"))) +void signal_done(int signal) +{ + Kernel::syscall(SYS_SIGNAL_DONE, signal); +} diff --git a/kernel/kernel/Syscall.cpp b/kernel/kernel/Syscall.cpp index 3c7ccdc235..913986c6b0 100644 --- a/kernel/kernel/Syscall.cpp +++ b/kernel/kernel/Syscall.cpp @@ -22,6 +22,12 @@ namespace Kernel { Thread::current().set_in_syscall(true); + if (syscall == SYS_SIGNAL_DONE) + { + Thread::current().set_signal_done((int)arg1); + return 0; + } + asm volatile("sti"); (void)arg1; @@ -149,6 +155,9 @@ namespace Kernel case SYS_SIGNAL: ret = Process::current().sys_signal((int)arg1, (void (*)(int))arg2); break; + case SYS_SIGNAL_DONE: + // Handled above + ASSERT_NOT_REACHED(); default: dwarnln("Unknown syscall {}", syscall); break; diff --git a/kernel/kernel/Thread.cpp b/kernel/kernel/Thread.cpp index b1a5091ee6..47c1430bf9 100644 --- a/kernel/kernel/Thread.cpp +++ b/kernel/kernel/Thread.cpp @@ -150,6 +150,19 @@ namespace Kernel return !m_signal_queue.empty() && !m_handling_signal; } + void Thread::set_signal_done(int signal) + { + ASSERT(!interrupts_enabled()); + if (!m_handling_signal) + derrorln("set_signal_done called while not handling singal"); + else if (m_signal_queue.empty()) + derrorln("set_signal_done called and there are no signals in queue"); + else if (m_signal_queue.front() != signal) + derrorln("set_signal_done called with wrong signal"); + else + m_signal_queue.pop(); + } + void Thread::handle_next_signal() { ASSERT(!interrupts_enabled()); @@ -167,8 +180,6 @@ namespace Kernel vaddr_t signal_handler = process().m_signal_handlers[signal]; - m_handling_signal = true; - // Skip masked and ignored signals if (m_signal_mask & (1ull << signal)) ; @@ -176,13 +187,15 @@ namespace Kernel ; else if (signal_handler != (vaddr_t)SIG_DFL) { + // call userspace signal handlers + // FIXME: signal trampoline should take a hash etc + // to only allow marking signals done from it + m_handling_signal = true; write_to_stack(return_rsp, return_rip); write_to_stack(return_rsp, signal); write_to_stack(return_rsp, signal_handler); return_rip = (uintptr_t)signal_trampoline; - - // FIXME: we should only mark this signal as done when - // handler returns + return; } else { @@ -241,7 +254,6 @@ namespace Kernel } m_signal_queue.pop(); - m_handling_signal = false; } void Thread::validate_stack() const diff --git a/libc/include/sys/syscall.h b/libc/include/sys/syscall.h index ddba8e8921..c4ba6dc90a 100644 --- a/libc/include/sys/syscall.h +++ b/libc/include/sys/syscall.h @@ -44,6 +44,7 @@ __BEGIN_DECLS #define SYS_RAISE 37 #define SYS_KILL 38 #define SYS_SIGNAL 39 +#define SYS_SIGNAL_DONE 40 __END_DECLS diff --git a/libc/unistd.cpp b/libc/unistd.cpp index 2b9b7eed8c..a116a89289 100644 --- a/libc/unistd.cpp +++ b/libc/unistd.cpp @@ -286,6 +286,9 @@ long syscall(long syscall, ...) ret = Kernel::syscall(SYS_SIGNAL, signal, (uintptr_t)handler); break; } + case SYS_SIGNAL_DONE: + // Should not be called by an user + // fall through default: puts("LibC: Unhandeled syscall"); ret = -ENOSYS; diff --git a/userspace/Shell/main.cpp b/userspace/Shell/main.cpp index 5309c29123..d40fe5f44c 100644 --- a/userspace/Shell/main.cpp +++ b/userspace/Shell/main.cpp @@ -281,6 +281,39 @@ int execute_command(BAN::Vector& args) return 1; } } + else if (args.front() == "signal-test"sv) + { + pid_t pid = fork(); + if (pid == 0) + { + if (signal(SIGSEGV, [](int) { printf("SIGSEGV\n"); }) == SIG_ERR) + { + perror("signal"); + exit(1); + } + printf("child\n"); + for (;;); + } + if (pid == -1) + { + perror("fork"); + return 1; + } + + sleep(1); + if (kill(pid, SIGSEGV) == -1) + { + perror("kill"); + return 1; + } + + sleep(1); + if (kill(pid, SIGTERM) == -1) + { + perror("kill"); + return 1; + } + } else if (args.front() == "cd"sv) { if (args.size() > 2)