diff --git a/Kernel/Syscalls/fork.cpp b/Kernel/Syscalls/fork.cpp index 5ccb2e0795..e6d933f601 100644 --- a/Kernel/Syscalls/fork.cpp +++ b/Kernel/Syscalls/fork.cpp @@ -117,7 +117,6 @@ ErrorOr Process::sys$fork(RegisterState& regs) // A child process created via fork(2) inherits a copy of its parent's alternate signal stack settings. child_first_thread->m_alternative_signal_stack = Thread::current()->m_alternative_signal_stack; - child_first_thread->m_alternative_signal_stack_size = Thread::current()->m_alternative_signal_stack_size; auto& child_regs = child_first_thread->m_regs; #if ARCH(X86_64) diff --git a/Kernel/Syscalls/sigaction.cpp b/Kernel/Syscalls/sigaction.cpp index a72d3a0472..23fd8b3bf0 100644 --- a/Kernel/Syscalls/sigaction.cpp +++ b/Kernel/Syscalls/sigaction.cpp @@ -117,17 +117,20 @@ ErrorOr Process::sys$sigreturn(RegisterState& registers) return saved_ax; } -ErrorOr Process::remap_range_as_stack(FlatPtr address, size_t size) +ErrorOr Process::remap_range_as_stack(FlatPtr address, size_t size) { // FIXME: This duplicates a lot of logic from sys$mprotect, this should be abstracted out somehow - auto range_to_remap = TRY(Memory::expand_range_to_page_boundaries(address, size)); + // NOTE: We shrink the given range to page boundaries (instead of expanding it), as sigaltstack's manpage suggests + // using malloc() to allocate the stack region, and many heap implementations (including ours) store heap chunk + // metadata in memory just before the vended pointer, which we would end up zeroing. + auto range_to_remap = TRY(Memory::shrink_range_to_page_boundaries(address, size)); if (!range_to_remap.size()) return EINVAL; if (!is_user_range(range_to_remap)) return EFAULT; - return address_space().with([&](auto& space) -> ErrorOr { + return address_space().with([&](auto& space) -> ErrorOr { if (auto* whole_region = space->find_region_from_range(range_to_remap)) { if (!whole_region->is_mmap()) return EPERM; @@ -141,7 +144,7 @@ ErrorOr Process::remap_range_as_stack(FlatPtr address, size_t size) whole_region->clear_to_zero(); whole_region->remap(); - return {}; + return range_to_remap; } if (auto* old_region = space->find_region_containing(range_to_remap)) { @@ -174,7 +177,7 @@ ErrorOr Process::remap_range_as_stack(FlatPtr address, size_t size) } TRY(new_region->map(space->page_directory())); - return {}; + return range_to_remap; } if (auto const& regions = TRY(space->find_regions_intersecting(range_to_remap)); regions.size()) { @@ -235,7 +238,7 @@ ErrorOr Process::remap_range_as_stack(FlatPtr address, size_t size) TRY(new_region->map(space->page_directory())); } - return {}; + return range_to_remap; } return EINVAL; @@ -249,13 +252,16 @@ ErrorOr Process::sys$sigaltstack(Userspace user_ss, Use if (user_old_ss) { stack_t old_ss_value {}; - old_ss_value.ss_sp = (void*)Thread::current()->m_alternative_signal_stack; - old_ss_value.ss_size = Thread::current()->m_alternative_signal_stack_size; - old_ss_value.ss_flags = 0; - if (!Thread::current()->has_alternative_signal_stack()) + if (Thread::current()->m_alternative_signal_stack.has_value()) { + old_ss_value.ss_sp = Thread::current()->m_alternative_signal_stack->base().as_ptr(); + old_ss_value.ss_size = Thread::current()->m_alternative_signal_stack->size(); + if (Thread::current()->is_in_alternative_signal_stack()) + old_ss_value.ss_flags = SS_ONSTACK; + else + old_ss_value.ss_flags = 0; + } else { old_ss_value.ss_flags = SS_DISABLE; - else if (Thread::current()->is_in_alternative_signal_stack()) - old_ss_value.ss_flags = SS_ONSTACK; + } TRY(copy_to_user(user_old_ss, &old_ss_value)); } @@ -266,8 +272,7 @@ ErrorOr Process::sys$sigaltstack(Userspace user_ss, Use return EPERM; if (ss.ss_flags == SS_DISABLE) { - Thread::current()->m_alternative_signal_stack_size = 0; - Thread::current()->m_alternative_signal_stack = 0; + Thread::current()->m_alternative_signal_stack.clear(); } else if (ss.ss_flags == 0) { if (ss.ss_size <= MINSIGSTKSZ) return ENOMEM; @@ -278,10 +283,7 @@ ErrorOr Process::sys$sigaltstack(Userspace user_ss, Use // protections, sigaltstack ranges are carved out of their regions, zeroed, and // turned into read/writable MAP_STACK-enabled regions. // This is inspired by OpenBSD's solution: https://man.openbsd.org/sigaltstack.2 - TRY(remap_range_as_stack((FlatPtr)ss.ss_sp, ss.ss_size)); - - Thread::current()->m_alternative_signal_stack = (FlatPtr)ss.ss_sp; - Thread::current()->m_alternative_signal_stack_size = ss.ss_size; + Thread::current()->m_alternative_signal_stack = TRY(remap_range_as_stack((FlatPtr)ss.ss_sp, ss.ss_size)); } else { return EINVAL; } diff --git a/Kernel/Tasks/Process.h b/Kernel/Tasks/Process.h index e9ec6cd53d..df63c85bf6 100644 --- a/Kernel/Tasks/Process.h +++ b/Kernel/Tasks/Process.h @@ -688,7 +688,7 @@ private: ErrorOr get_futex_key(FlatPtr user_address, bool shared); - ErrorOr remap_range_as_stack(FlatPtr address, size_t size); + ErrorOr remap_range_as_stack(FlatPtr address, size_t size); ErrorOr open_impl(Userspace); ErrorOr close_impl(int fd); diff --git a/Kernel/Tasks/Thread.cpp b/Kernel/Tasks/Thread.cpp index 62685a18f2..ca5c726234 100644 --- a/Kernel/Tasks/Thread.cpp +++ b/Kernel/Tasks/Thread.cpp @@ -752,8 +752,7 @@ void Thread::reset_signals_for_exec() m_have_any_unmasked_pending_signals.store(false, AK::memory_order_release); m_signal_action_masks.fill({}); // A successful call to execve(2) removes any existing alternate signal stack - m_alternative_signal_stack = 0; - m_alternative_signal_stack_size = 0; + m_alternative_signal_stack.clear(); } // Certain exceptions, such as SIGSEGV and SIGILL, put a @@ -881,15 +880,12 @@ bool Thread::is_signal_masked(u8 signal) const return (1 << (signal - 1)) & m_signal_mask; } -bool Thread::has_alternative_signal_stack() const -{ - return m_alternative_signal_stack_size != 0; -} - bool Thread::is_in_alternative_signal_stack() const { auto sp = get_register_dump_from_stack().userspace_sp(); - return sp >= m_alternative_signal_stack && sp < m_alternative_signal_stack + m_alternative_signal_stack_size; + if (!m_alternative_signal_stack.has_value()) + return false; + return m_alternative_signal_stack->contains(VirtualAddress(sp)); } static ErrorOr push_value_on_user_stack(FlatPtr& stack, FlatPtr data) @@ -1025,12 +1021,12 @@ DispatchSignalResult Thread::dispatch_signal(u8 signal) m_signal_mask |= new_signal_mask; m_have_any_unmasked_pending_signals.store((m_pending_signals & ~m_signal_mask) != 0, AK::memory_order_release); - bool use_alternative_stack = ((action.flags & SA_ONSTACK) != 0) && has_alternative_signal_stack() && !is_in_alternative_signal_stack(); + bool use_alternative_stack = ((action.flags & SA_ONSTACK) != 0) && m_alternative_signal_stack.has_value() && !is_in_alternative_signal_stack(); auto setup_stack = [&](RegisterState& state) -> ErrorOr { FlatPtr stack; if (use_alternative_stack) - stack = m_alternative_signal_stack + m_alternative_signal_stack_size; + stack = m_alternative_signal_stack->end().get(); else stack = state.userspace_sp(); @@ -1042,7 +1038,7 @@ DispatchSignalResult Thread::dispatch_signal(u8 signal) .uc_stack = { .ss_sp = bit_cast(stack), .ss_flags = action.flags & SA_ONSTACK, - .ss_size = use_alternative_stack ? m_alternative_signal_stack_size : 0, + .ss_size = use_alternative_stack ? m_alternative_signal_stack->size() : 0, }, .uc_mcontext = {}, }; diff --git a/Kernel/Tasks/Thread.h b/Kernel/Tasks/Thread.h index 15f79f6bde..4113a302cd 100644 --- a/Kernel/Tasks/Thread.h +++ b/Kernel/Tasks/Thread.h @@ -884,7 +884,6 @@ public: u32 pending_signals() const; u32 pending_signals_for_state() const; - [[nodiscard]] bool has_alternative_signal_stack() const; [[nodiscard]] bool is_in_alternative_signal_stack() const; FPUState& fpu_state() { return m_fpu_state; } @@ -1177,8 +1176,7 @@ private: u32 m_pending_signals { 0 }; u8 m_currently_handled_signal { 0 }; u32 m_signal_mask { 0 }; - FlatPtr m_alternative_signal_stack { 0 }; - FlatPtr m_alternative_signal_stack_size { 0 }; + Optional m_alternative_signal_stack; SignalBlockerSet m_signal_blocker_set; FlatPtr m_kernel_stack_base { 0 }; FlatPtr m_kernel_stack_top { 0 };