Bananymous/banan-os
1
1
Fork 1
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Kernel: Clean up signal handling

Browse Source 
We now appreciate sa_mask and SA_NODEFER and change the signal mask for
the duration of signal handler. This is done by making a sigprocmask
syscall at the end of the signal handler. Back-to-back signals will
still grow stack as original registers are popped AFTER the block mask
is updated. I guess this is why linux has sigreturn(?).
This commit is contained in:
Bananymous
2026-04-05 02:25:59 +03:00
parent df257755f7
commit 8ca3c5d778
4 changed files with 83 additions and 78 deletions
Download Patch File Download Diff File Expand all files Collapse all files

24
kernel/arch/i686/Signal.S
View File

@@ -1,12 +1,13 @@
.section .userspace, "ax" .section .userspace, "ax"
// stack contains // stack contains
// return address // (4 bytes) return address
// return stack // (4 bytes) return stack
// return rflags // (4 bytes) return rflags
// siginfo_t // (8 bytes) restore sigmask
// signal number // (36 bytes) siginfo_t
// signal handler // (4 bytes) signal number
// (4 bytes) signal handler
.global signal_trampoline .global signal_trampoline
signal_trampoline: signal_trampoline:
@@ -53,6 +54,13 @@ signal_trampoline:
movl %ebp, %esp movl %ebp, %esp
addl $24, %esp addl $24, %esp
// restore sigmask
movl $83, %eax // SYS_SIGPROCMASK
movl $3, %ebx // SIG_SETMASK
leal 72(%esp), %ecx // set
xorl %edx, %edx // oset
int $0xF0
// restore registers // restore registers
popl %ebp popl %ebp
popl %eax popl %eax
@@ -62,8 +70,8 @@ signal_trampoline:
popl %edi popl %edi
popl %esi popl %esi
// skip handler, number, siginfo_t // skip handler, number, siginfo_t, sigmask
addl $44, %esp addl $52, %esp
// restore flags // restore flags
popf popf

26
kernel/arch/x86_64/Signal.S
View File

@@ -1,12 +1,13 @@
.section .userspace, "ax" .section .userspace, "ax"
// stack contains // stack contains
// return address // (8 bytes) return address
// return stack // (8 bytes) return stack
// return rflags // (8 bytes) return rflags
// siginfo_t // (8 bytes) restore sigmask
// signal number // (56 bytes) siginfo_t
// signal handler // (8 bytes) signal number
// (8 bytes) signal handler
.global signal_trampoline .global signal_trampoline
signal_trampoline: signal_trampoline:
@@ -55,6 +56,13 @@ signal_trampoline:
movq %rbp, %rsp movq %rbp, %rsp
addq $40, %rsp addq $40, %rsp
// restore sigmask
movq $83, %rdi // SYS_SIGPROCMASK
movq $3, %rsi // SIG_SETMASK
leaq 192(%rsp), %rdx // set
xorq %r10, %r10 // oset
syscall
// restore registers // restore registers
popq %rbp popq %rbp
popq %rax popq %rax
@@ -72,13 +80,13 @@ signal_trampoline:
popq %r14 popq %r14
popq %r15 popq %r15
// skip handler, number, siginfo_t // skip handler, number, siginfo_t, sigmask
addq $72, %rsp addq $80, %rsp
// restore flags // restore flags
popfq popfq
movq (%rsp), %rsp movq (%rsp), %rsp
// return over red-zone and siginfo_t // return over red-zone
ret $128 ret $128

7
kernel/include/kernel/Thread.h
View File

@@ -157,12 +157,13 @@ namespace Kernel
struct signal_handle_info_t struct signal_handle_info_t
{ {
vaddr_t signal_handler; vaddr_t handler;
vaddr_t signal_stack_top; vaddr_t stack_top;
uint64_t restore_sigmask;
bool has_sa_restart; bool has_sa_restart;
}; };
signal_handle_info_t remove_signal_and_get_info(int signal); signal_handle_info_t remove_signal_and_get_info(int signal);
void handle_signal_impl(int signal, const siginfo_t&, vaddr_t signal_handler, vaddr_t signal_stack_top); void handle_signal_impl(int signal, const siginfo_t&, const signal_handle_info_t&);
private: private:
// NOTE: this is the first member to force it being last destructed // NOTE: this is the first member to force it being last destructed

104
kernel/kernel/Thread.cpp
View File

@@ -632,12 +632,7 @@ namespace Kernel
handle_info = remove_signal_and_get_info(signal); handle_info = remove_signal_and_get_info(signal);
} }
handle_signal_impl( handle_signal_impl(signal, signal_info, handle_info);
signal,
signal_info,
handle_info.signal_handler,
handle_info.signal_stack_top
);
return handle_info.has_sa_restart; return handle_info.has_sa_restart;
} }
@@ -652,18 +647,29 @@ namespace Kernel
signal_handle_info_t handle_info; signal_handle_info_t handle_info;
// If this signal is blocked or ignored, terminate the process
bool terminate_process = false;
{ {
SpinLockGuard _1(m_signal_lock); SpinLockGuard _1(m_signal_lock);
SpinLockGuard _2(m_process->m_signal_lock); SpinLockGuard _2(m_process->m_signal_lock);
if (m_signal_block_mask & (1ull << signal))
terminate_process = true;
handle_info = remove_signal_and_get_info(signal); handle_info = remove_signal_and_get_info(signal);
if (handle_info.handler == reinterpret_cast<vaddr_t>(SIG_IGN))
terminate_process = true;
} }
handle_signal_impl( if (terminate_process)
signal, {
signal_info, process().exit(128 + signal, signal | 0x80);
handle_info.signal_handler, ASSERT_NOT_REACHED();
handle_info.signal_stack_top }
);
handle_signal_impl(signal, signal_info, handle_info);
return handle_info.has_sa_restart; return handle_info.has_sa_restart;
} }
@@ -676,6 +682,8 @@ namespace Kernel
ASSERT(signal >= _SIGMIN); ASSERT(signal >= _SIGMIN);
ASSERT(signal <= _SIGMAX); ASSERT(signal <= _SIGMAX);
const uint64_t restore_sigmask = m_signal_block_mask;
auto& handler = m_process->m_signal_handlers[signal]; auto& handler = m_process->m_signal_handlers[signal];
const vaddr_t signal_handler = (handler.sa_flags & SA_SIGINFO) const vaddr_t signal_handler = (handler.sa_flags & SA_SIGINFO)
@@ -687,6 +695,10 @@ namespace Kernel
if (m_signal_alt_stack.ss_flags != SS_DISABLE && (handler.sa_flags & SA_ONSTACK) && !currently_on_alternate_stack()) if (m_signal_alt_stack.ss_flags != SS_DISABLE && (handler.sa_flags & SA_ONSTACK) && !currently_on_alternate_stack())
signal_stack_top = reinterpret_cast<vaddr_t>(m_signal_alt_stack.ss_sp) + m_signal_alt_stack.ss_size; signal_stack_top = reinterpret_cast<vaddr_t>(m_signal_alt_stack.ss_sp) + m_signal_alt_stack.ss_size;
m_signal_block_mask |= handler.sa_mask;
if (!(handler.sa_flags & SA_NODEFER))
m_signal_block_mask |= 1ull << signal;
if (handler.sa_flags & SA_RESETHAND) if (handler.sa_flags & SA_RESETHAND)
handler = { .sa_handler = SIG_DFL, .sa_mask = 0, .sa_flags = 0 }; handler = { .sa_handler = SIG_DFL, .sa_mask = 0, .sa_flags = 0 };
@@ -700,13 +712,14 @@ namespace Kernel
} }
return { return {
.signal_handler = signal_handler, .handler = signal_handler,
.signal_stack_top = signal_stack_top, .stack_top = signal_stack_top,
.restore_sigmask = restore_sigmask,
.has_sa_restart = has_sa_restart, .has_sa_restart = has_sa_restart,
}; };
} }
void Thread::handle_signal_impl(int signal, const siginfo_t& signal_info, vaddr_t signal_handler, vaddr_t signal_stack_top) void Thread::handle_signal_impl(int signal, const siginfo_t& signal_info, const signal_handle_info_t& handle_info)
{ {
ASSERT(this == &Thread::current()); ASSERT(this == &Thread::current());
ASSERT(is_userspace()); ASSERT(is_userspace());
@@ -715,64 +728,39 @@ namespace Kernel
auto& interrupt_stack = *reinterpret_cast<InterruptStack*>(kernel_stack_top() - sizeof(InterruptStack)); auto& interrupt_stack = *reinterpret_cast<InterruptStack*>(kernel_stack_top() - sizeof(InterruptStack));
ASSERT(GDT::is_user_segment(interrupt_stack.cs)); ASSERT(GDT::is_user_segment(interrupt_stack.cs));
if (signal_handler == (vaddr_t)SIG_IGN) if (handle_info.handler == reinterpret_cast<vaddr_t>(SIG_IGN))
; ;
else if (signal_handler != (vaddr_t)SIG_DFL) else if (handle_info.handler != reinterpret_cast<vaddr_t>(SIG_DFL))
{ {
// call userspace signal handlers // call userspace signal handlers
#if ARCH(x86_64) #if ARCH(x86_64)
interrupt_stack.sp -= 128; // skip possible red-zone interrupt_stack.sp -= 128; // skip possible red-zone
#endif #endif
{ const auto write_to_stack =
// Make sure stack is allocated [&]<typename T>(uintptr_t& sp, const T& value)
vaddr_t pages[3] {};
size_t page_count { 0 };
if (signal_stack_top == 0)
{ {
pages[0] = (interrupt_stack.sp - 1 * sizeof(uintptr_t) ) & PAGE_ADDR_MASK; static_assert(sizeof(T) >= sizeof(uintptr_t));
pages[1] = (interrupt_stack.sp - 5 * sizeof(uintptr_t) - sizeof(siginfo_t)) & PAGE_ADDR_MASK; sp -= sizeof(T);
page_count = 2; if (m_process->write_to_user(reinterpret_cast<void*>(sp), &value, sizeof(T)).is_error())
} m_process->exit(128 + SIGSEGV, SIGSEGV | 0x80);
else };
{
pages[0] = (interrupt_stack.sp - 1 * sizeof(uintptr_t) ) & PAGE_ADDR_MASK;
pages[2] = (signal_stack_top - 1 * sizeof(uintptr_t) ) & PAGE_ADDR_MASK;
pages[1] = (signal_stack_top - 4 * sizeof(uintptr_t) - sizeof(siginfo_t)) & PAGE_ADDR_MASK;
page_count = 3;
}
for (size_t i = 0; i < page_count; i++)
{
if (m_process->page_table().get_page_flags(pages[i]) & PageTable::Flags::Present)
continue;
if (auto ret = m_process->allocate_page_for_demand_paging(pages[i], true, false); ret.is_error() || !ret.value())
m_process->exit(128 + SIGSEGV, SIGSEGV);
}
}
write_to_stack(interrupt_stack.sp, interrupt_stack.ip); write_to_stack(interrupt_stack.sp, interrupt_stack.ip);
const vaddr_t old_stack = interrupt_stack.sp; const vaddr_t old_stack = interrupt_stack.sp;
if (signal_stack_top) if (handle_info.stack_top)
interrupt_stack.sp = signal_stack_top; interrupt_stack.sp = handle_info.stack_top;
write_to_stack(interrupt_stack.sp, old_stack); write_to_stack(interrupt_stack.sp, old_stack);
write_to_stack(interrupt_stack.sp, interrupt_stack.flags); write_to_stack(interrupt_stack.sp, interrupt_stack.flags);
write_to_stack(interrupt_stack.sp, handle_info.restore_sigmask);
{ siginfo_t copy = signal_info;
interrupt_stack.sp -= sizeof(siginfo_t); copy.si_signo = signal;
copy.si_addr = reinterpret_cast<void*>(interrupt_stack.ip);
write_to_stack(interrupt_stack.sp, copy);
auto& stack_signal_info = *reinterpret_cast<siginfo_t*>(interrupt_stack.sp); write_to_stack(interrupt_stack.sp, static_cast<uintptr_t>(signal));
stack_signal_info = signal_info; write_to_stack(interrupt_stack.sp, handle_info.handler);
stack_signal_info.si_signo = signal;
stack_signal_info.si_addr = reinterpret_cast<void*>(interrupt_stack.ip);
static_assert(sizeof(siginfo_t) % sizeof(uintptr_t) == 0);
}
write_to_stack(interrupt_stack.sp, signal);
write_to_stack(interrupt_stack.sp, signal_handler);
interrupt_stack.ip = (uintptr_t)signal_trampoline; interrupt_stack.ip = (uintptr_t)signal_trampoline;
} }
else else