From 6de350ce9d263c97e6fb5df88cb862fc2d0e00af Mon Sep 17 00:00:00 2001 From: Bananymous Date: Thu, 1 Aug 2024 17:01:18 +0300 Subject: [PATCH] Kernel/LibC: Cleanup, fix and implement a lot of signal code This patch implements sigsets and some of their usages --- kernel/include/kernel/Process.h | 31 +++--- kernel/kernel/IDT.cpp | 4 + kernel/kernel/Process.cpp | 99 ++++++++++++++++--- kernel/kernel/Thread.cpp | 30 ++++-- userspace/libraries/LibC/include/signal.h | 2 +- .../libraries/LibC/include/sys/syscall.h | 4 +- userspace/libraries/LibC/signal.cpp | 55 ++++++++++- 7 files changed, 180 insertions(+), 45 deletions(-) diff --git a/kernel/include/kernel/Process.h b/kernel/include/kernel/Process.h index 55d85426..d843a99a 100644 --- a/kernel/include/kernel/Process.h +++ b/kernel/include/kernel/Process.h @@ -174,8 +174,10 @@ namespace Kernel BAN::ErrorOr sys_tty_ctrl(int fildes, int command, int flags); - BAN::ErrorOr sys_signal(int, void (*)(int)); BAN::ErrorOr sys_kill(pid_t pid, int signal); + BAN::ErrorOr sys_sigaction(int signal, const struct sigaction* act, struct sigaction* oact); + BAN::ErrorOr sys_sigpending(sigset_t* set); + BAN::ErrorOr sys_sigprocmask(int how, const sigset_t* set, sigset_t* oset); BAN::ErrorOr sys_tcsetpgrp(int fd, pid_t pgid); @@ -220,7 +222,8 @@ namespace Kernel uint64_t signal_pending_mask() const { - return ((uint64_t)m_signal_pending_mask[1].load() << 32) | m_signal_pending_mask[0].load(); + SpinLockGuard _(m_signal_lock); + return m_signal_pending_mask; } void add_pending_signal(uint8_t signal) @@ -228,15 +231,13 @@ namespace Kernel ASSERT(signal >= _SIGMIN); ASSERT(signal <= _SIGMAX); ASSERT(signal < 64); - vaddr_t handler = m_signal_handlers[signal]; - if (handler == (vaddr_t)SIG_IGN) + SpinLockGuard _(m_signal_lock); + auto handler = m_signal_handlers[signal].sa_handler; + if (handler == SIG_IGN) return; - if (handler == (vaddr_t)SIG_DFL && (signal == SIGCHLD || signal == SIGURG)) + if (handler == SIG_DFL && (signal == SIGCHLD || signal == SIGURG)) return; - if (signal < 32) - m_signal_pending_mask[0] |= (uint32_t)1 << signal; - else - m_signal_pending_mask[1] |= (uint32_t)1 << (signal - 32); + m_signal_pending_mask |= 1ull << signal; } void remove_pending_signal(uint8_t signal) @@ -244,10 +245,8 @@ namespace Kernel ASSERT(signal >= _SIGMIN); ASSERT(signal <= _SIGMAX); ASSERT(signal < 64); - if (signal < 32) - m_signal_pending_mask[0] &= ~((uint32_t)1 << signal); - else - m_signal_pending_mask[1] &= ~((uint32_t)1 << (signal - 32)); + SpinLockGuard _(m_signal_lock); + m_signal_pending_mask &= ~(1ull << signal); } private: @@ -276,9 +275,9 @@ namespace Kernel BAN::String m_working_directory; BAN::Vector m_threads; - BAN::Atomic m_signal_handlers[_SIGMAX + 1] { }; - // This is 2 32 bit values to allow atomicity on 32 targets - BAN::Atomic m_signal_pending_mask[2] { 0, 0 }; + mutable SpinLock m_signal_lock; + struct sigaction m_signal_handlers[_SIGMAX + 1] { }; + uint64_t m_signal_pending_mask { 0 }; BAN::Vector m_cmdline; BAN::Vector m_environ; diff --git a/kernel/kernel/IDT.cpp b/kernel/kernel/IDT.cpp index c4155d22..0cf2630c 100644 --- a/kernel/kernel/IDT.cpp +++ b/kernel/kernel/IDT.cpp @@ -338,6 +338,10 @@ done: ASSERT(InterruptController::get().is_in_service(IRQ_TIMER)); InterruptController::get().eoi(IRQ_TIMER); Processor::scheduler().timer_interrupt(); + + auto& current_thread = Thread::current(); + if (current_thread.can_add_signal_to_execute()) + current_thread.handle_signal(); } extern "C" void cpp_irq_handler(uint32_t irq) diff --git a/kernel/kernel/Process.cpp b/kernel/kernel/Process.cpp index 5f7742fe..31f6a4bd 100644 --- a/kernel/kernel/Process.cpp +++ b/kernel/kernel/Process.cpp @@ -173,7 +173,10 @@ namespace Kernel , m_parent(parent) { for (size_t i = 0; i < sizeof(m_signal_handlers) / sizeof(*m_signal_handlers); i++) - m_signal_handlers[i] = (vaddr_t)SIG_DFL; + { + m_signal_handlers[i].sa_handler = SIG_DFL; + m_signal_handlers[i].sa_flags = 0; + } } Process::~Process() @@ -472,7 +475,10 @@ namespace Kernel m_userspace_info.entry = m_loadable_elf->entry_point(); for (size_t i = 0; i < sizeof(m_signal_handlers) / sizeof(*m_signal_handlers); i++) - m_signal_handlers[i] = (vaddr_t)SIG_DFL; + { + m_signal_handlers[i].sa_handler = SIG_DFL; + m_signal_handlers[i].sa_flags = 0; + } ASSERT(m_threads.size() == 1); ASSERT(&Process::current() == this); @@ -1640,20 +1646,6 @@ namespace Kernel return 0; } - BAN::ErrorOr Process::sys_signal(int signal, void (*handler)(int)) - { - if (signal < _SIGMIN || signal > _SIGMAX) - return BAN::Error::from_errno(EINVAL); - - { - LockGuard _(m_process_lock); - TRY(validate_pointer_access((void*)handler, sizeof(handler))); - } - - m_signal_handlers[signal] = (vaddr_t)handler; - return 0; - } - BAN::ErrorOr Process::sys_kill(pid_t pid, int signal) { if (pid == 0 || pid == -1) @@ -1690,6 +1682,81 @@ namespace Kernel return BAN::Error::from_errno(ESRCH); } + BAN::ErrorOr Process::sys_sigaction(int signal, const struct sigaction* act, struct sigaction* oact) + { + if (signal < _SIGMIN || signal > _SIGMAX) + return BAN::Error::from_errno(EINVAL); + + LockGuard _(m_process_lock); + if (act) + TRY(validate_pointer_access(act, sizeof(struct sigaction))); + if (oact) + TRY(validate_pointer_access(oact, sizeof(struct sigaction))); + + SpinLockGuard signal_lock_guard(m_signal_lock); + + if (oact) + *oact = m_signal_handlers[signal]; + + if (act) + { + if (act->sa_flags) + return BAN::Error::from_errno(ENOTSUP); + m_signal_handlers[signal] = *act; + } + + return 0; + } + + BAN::ErrorOr Process::sys_sigpending(sigset_t* set) + { + LockGuard _(m_process_lock); + TRY(validate_pointer_access(set, sizeof(sigset_t))); + *set = (signal_pending_mask() | Thread::current().m_signal_pending_mask) & Thread::current().m_signal_block_mask; + return 0; + } + + BAN::ErrorOr Process::sys_sigprocmask(int how, const sigset_t* set, sigset_t* oset) + { + switch (how) + { + case SIG_BLOCK: + case SIG_SETMASK: + case SIG_UNBLOCK: + break; + default: + return BAN::Error::from_errno(EINVAL); + } + + LockGuard _(m_process_lock); + if (set) + TRY(validate_pointer_access(set, sizeof(sigset_t))); + if (oset) + TRY(validate_pointer_access(oset, sizeof(sigset_t))); + + if (oset) + *oset = Thread::current().m_signal_block_mask; + + if (set) + { + const sigset_t mask = *set & ~(SIGKILL | SIGSTOP); + switch (how) + { + case SIG_BLOCK: + Thread::current().m_signal_block_mask |= mask; + break; + case SIG_SETMASK: + Thread::current().m_signal_block_mask = mask; + break; + case SIG_UNBLOCK: + Thread::current().m_signal_block_mask &= mask; + break; + } + } + + return 0; + } + BAN::ErrorOr Process::sys_tcsetpgrp(int fd, pid_t pgrp) { LockGuard _(m_process_lock); diff --git a/kernel/kernel/Thread.cpp b/kernel/kernel/Thread.cpp index dac15fae..a9492c6d 100644 --- a/kernel/kernel/Thread.cpp +++ b/kernel/kernel/Thread.cpp @@ -266,10 +266,16 @@ namespace Kernel { if (!(signals & ((uint64_t)1 << i))) continue; - vaddr_t handler = m_process->m_signal_handlers[i]; - if (handler == (vaddr_t)SIG_IGN) + + vaddr_t signal_handler; + { + SpinLockGuard _(m_process->m_signal_lock); + ASSERT(!(m_process->m_signal_handlers[i].sa_flags & SA_SIGINFO)); + signal_handler = (vaddr_t)m_process->m_signal_handlers[i].sa_handler; + } + if (signal_handler == (vaddr_t)SIG_IGN) continue; - if (handler == (vaddr_t)SIG_DFL && (i == SIGCHLD || i == SIGURG)) + if (signal_handler == (vaddr_t)SIG_DFL && (i == SIGCHLD || i == SIGURG)) continue; return true; } @@ -316,7 +322,12 @@ namespace Kernel ASSERT(signal <= _SIGMAX); } - vaddr_t signal_handler = process().m_signal_handlers[signal]; + vaddr_t signal_handler; + { + SpinLockGuard _(m_process->m_signal_lock); + ASSERT(!(m_process->m_signal_handlers[signal].sa_flags & SA_SIGINFO)); + signal_handler = (vaddr_t)m_process->m_signal_handlers[signal].sa_handler; + } m_signal_pending_mask &= ~(1ull << signal); process().remove_pending_signal(signal); @@ -390,10 +401,15 @@ namespace Kernel SpinLockGuard _(m_signal_lock); if (m_process) { - vaddr_t handler = m_process->m_signal_handlers[signal]; - if (handler == (vaddr_t)SIG_IGN) + vaddr_t signal_handler; + { + SpinLockGuard _(m_process->m_signal_lock); + ASSERT(!(m_process->m_signal_handlers[signal].sa_flags & SA_SIGINFO)); + signal_handler = (vaddr_t)m_process->m_signal_handlers[signal].sa_handler; + } + if (signal_handler == (vaddr_t)SIG_IGN) return false; - if (handler == (vaddr_t)SIG_DFL && (signal == SIGCHLD || signal == SIGURG)) + if (signal_handler == (vaddr_t)SIG_DFL && (signal == SIGCHLD || signal == SIGURG)) return false; } uint64_t mask = 1ull << signal; diff --git a/userspace/libraries/LibC/include/signal.h b/userspace/libraries/LibC/include/signal.h index f555c742..ebab97d1 100644 --- a/userspace/libraries/LibC/include/signal.h +++ b/userspace/libraries/LibC/include/signal.h @@ -22,7 +22,7 @@ __BEGIN_DECLS #include typedef int sig_atomic_t; -typedef void* sigset_t; +typedef unsigned long long sigset_t; union sigval { diff --git a/userspace/libraries/LibC/include/sys/syscall.h b/userspace/libraries/LibC/include/sys/syscall.h index f03e7d5f..dbcacd9f 100644 --- a/userspace/libraries/LibC/include/sys/syscall.h +++ b/userspace/libraries/LibC/include/sys/syscall.h @@ -40,7 +40,6 @@ __BEGIN_DECLS O(SYS_DUP, dup) \ O(SYS_DUP2, dup2) \ O(SYS_KILL, kill) \ - O(SYS_SIGNAL, signal) \ O(SYS_TCSETPGRP, tcsetpgrp) \ O(SYS_GET_PID, getpid) \ O(SYS_GET_PGID, getpgid) \ @@ -84,6 +83,9 @@ __BEGIN_DECLS O(SYS_REALPATH, realpath) \ O(SYS_TTYNAME, ttyname) \ O(SYS_ACCESS, access) \ + O(SYS_SIGACTION, sigaction) \ + O(SYS_SIGPENDING, sigpending) \ + O(SYS_SIGPROCMASK, sigprocmask) \ enum Syscall { diff --git a/userspace/libraries/LibC/signal.cpp b/userspace/libraries/LibC/signal.cpp index 397822cb..489a1606 100644 --- a/userspace/libraries/LibC/signal.cpp +++ b/userspace/libraries/LibC/signal.cpp @@ -1,20 +1,67 @@ +#include #include #include #include +static_assert(sizeof(sigset_t) * 8 >= _SIGMAX); + +int kill(pid_t pid, int sig) +{ + return syscall(SYS_KILL, pid, sig); +} + int raise(int sig) { // FIXME: won't work after multithreaded return kill(getpid(), sig); } -int kill(pid_t pid, int sig) +int sigaction(int sig, const struct sigaction* __restrict act, struct sigaction* __restrict oact) { - return syscall(SYS_KILL, pid, sig); + return syscall(SYS_SIGACTION, sig, act, oact); +} + +int sigaddset(sigset_t* set, int signo) +{ + *set |= 1ull << signo; + return 0; +} + +int sigemptyset(sigset_t* set) +{ + *set = 0; + return 0; +} + +int sigfillset(sigset_t* set) +{ + *set = (1ull << _SIGMAX) - 1; + return 0; +} + +int sigismember(const sigset_t* set, int signo) +{ + return (*set >> signo) & 1; } void (*signal(int sig, void (*func)(int)))(int) { - long ret = syscall(SYS_SIGNAL, sig, func); - return (void (*)(int))ret; + struct sigaction act; + act.sa_handler = func; + act.sa_flags = 0; + + int ret = sigaction(sig, &act, nullptr); + if (ret == -1) + return SIG_ERR; + return func; +} + +int sigpending(sigset_t* set) +{ + return syscall(SYS_SIGPENDING, set); +} + +int sigprocmask(int how, const sigset_t* __restrict set, sigset_t* __restrict oset) +{ + return syscall(SYS_SIGPROCMASK, how, set, oset); }