summaryrefslogtreecommitdiff
path: root/Kernel
diff options
context:
space:
mode:
authorAli Mohammad Pur <ali.mpfard@gmail.com>2022-02-24 22:25:49 +0330
committerAndreas Kling <kling@serenityos.org>2022-03-04 20:07:05 +0100
commitcf63447044f7f26bc955d6dbfceb10eedfbd3112 (patch)
tree9c244265fd0b8f39efddcb5c837e5cc315cfbef4 /Kernel
parent18b9d02edd427589eaa7fb7da1a7f6d6696cff4f (diff)
downloadserenity-cf63447044f7f26bc955d6dbfceb10eedfbd3112.zip
Kernel: Move signal handlers from being thread state to process state
POSIX requires that sigaction() and friends set a _process-wide_ signal handler, so move signal handlers and flags inside Process. This also fixes a "pid/tid confusion" FIXME, as we can now send the signal to the process and let that decide which thread should get the signal (which is the thread with tid==pid, but that's now the Process's problem). Note that each thread still retains its signal mask, as that is local to each thread.
Diffstat (limited to 'Kernel')
-rw-r--r--Kernel/Process.cpp7
-rw-r--r--Kernel/Process.h6
-rw-r--r--Kernel/Syscalls/sigaction.cpp4
-rw-r--r--Kernel/Thread.cpp13
-rw-r--r--Kernel/Thread.h8
5 files changed, 19 insertions, 19 deletions
diff --git a/Kernel/Process.cpp b/Kernel/Process.cpp
index 2f637473fe..85d3e65802 100644
--- a/Kernel/Process.cpp
+++ b/Kernel/Process.cpp
@@ -612,10 +612,9 @@ void Process::finalize()
m_state.store(State::Dead, AK::MemoryOrder::memory_order_release);
{
- // FIXME: PID/TID BUG
- if (auto parent_thread = Thread::from_tid(ppid().value())) {
- if (parent_thread->process().is_user_process() && (parent_thread->m_signal_action_data[SIGCHLD].flags & SA_NOCLDWAIT) != SA_NOCLDWAIT)
- parent_thread->send_signal(SIGCHLD, this);
+ if (auto parent_process = Process::from_pid(ppid())) {
+ if (parent_process->is_user_process() && (parent_process->m_signal_action_data[SIGCHLD].flags & SA_NOCLDWAIT) != SA_NOCLDWAIT)
+ (void)parent_process->send_signal(SIGCHLD, this);
}
}
diff --git a/Kernel/Process.h b/Kernel/Process.h
index 7a89ac9891..97300c5d66 100644
--- a/Kernel/Process.h
+++ b/Kernel/Process.h
@@ -827,6 +827,12 @@ private:
NonnullRefPtrVector<Thread> m_threads_for_coredump;
mutable RefPtr<ProcessProcFSTraits> m_procfs_traits;
+ struct SignalActionData {
+ VirtualAddress handler_or_sigaction;
+ int flags { 0 };
+ u32 mask { 0 };
+ };
+ Array<SignalActionData, NSIG> m_signal_action_data;
static_assert(sizeof(ProtectedValues) < (PAGE_SIZE));
alignas(4096) ProtectedValues m_protected_values;
diff --git a/Kernel/Syscalls/sigaction.cpp b/Kernel/Syscalls/sigaction.cpp
index 79450ca974..80e14c44a8 100644
--- a/Kernel/Syscalls/sigaction.cpp
+++ b/Kernel/Syscalls/sigaction.cpp
@@ -58,15 +58,17 @@ ErrorOr<FlatPtr> Process::sys$sigaction(int signum, Userspace<const sigaction*>
return EINVAL;
InterruptDisabler disabler; // FIXME: This should use a narrower lock. Maybe a way to ignore signals temporarily?
- auto& action = Thread::current()->m_signal_action_data[signum];
+ auto& action = m_signal_action_data[signum];
if (user_old_act) {
sigaction old_act {};
old_act.sa_flags = action.flags;
old_act.sa_sigaction = reinterpret_cast<decltype(old_act.sa_sigaction)>(action.handler_or_sigaction.as_ptr());
+ old_act.sa_mask = action.mask;
TRY(copy_to_user(user_old_act, &old_act));
}
if (user_act) {
auto act = TRY(copy_typed_from_user(user_act));
+ action.mask = act.sa_mask;
action.flags = act.sa_flags;
action.handler_or_sigaction = VirtualAddress { reinterpret_cast<void*>(act.sa_sigaction) };
}
diff --git a/Kernel/Thread.cpp b/Kernel/Thread.cpp
index ca7038f50d..7286b87c95 100644
--- a/Kernel/Thread.cpp
+++ b/Kernel/Thread.cpp
@@ -790,7 +790,7 @@ void Thread::reset_signals_for_exec()
// The signal mask is preserved across execve(2).
// The pending signal set is preserved across an execve(2).
m_have_any_unmasked_pending_signals.store(false, AK::memory_order_release);
- m_signal_action_data.fill({});
+ 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;
@@ -902,7 +902,7 @@ static DefaultSignalAction default_signal_action(u8 signal)
bool Thread::should_ignore_signal(u8 signal) const
{
VERIFY(signal < 32);
- auto const& action = m_signal_action_data[signal];
+ auto const& action = m_process->m_signal_action_data[signal];
if (action.handler_or_sigaction.is_null())
return default_signal_action(signal) == DefaultSignalAction::Ignore;
return ((sighandler_t)action.handler_or_sigaction.get() == SIG_IGN);
@@ -911,7 +911,7 @@ bool Thread::should_ignore_signal(u8 signal) const
bool Thread::has_signal_handler(u8 signal) const
{
VERIFY(signal < 32);
- auto const& action = m_signal_action_data[signal];
+ auto const& action = m_process->m_signal_action_data[signal];
return !action.handler_or_sigaction.is_null();
}
@@ -975,7 +975,7 @@ DispatchSignalResult Thread::dispatch_signal(u8 signal)
return DispatchSignalResult::Deferred;
}
- auto& action = m_signal_action_data[signal];
+ auto& action = m_process->m_signal_action_data[signal];
// FIXME: Implement SA_SIGINFO signal handlers.
VERIFY(!(action.flags & SA_SIGINFO));
@@ -1045,7 +1045,7 @@ DispatchSignalResult Thread::dispatch_signal(u8 signal)
ScopedAddressSpaceSwitcher switcher(m_process);
u32 old_signal_mask = m_signal_mask;
- u32 new_signal_mask = action.mask;
+ u32 new_signal_mask = m_signal_action_masks[signal].value_or(action.mask);
if ((action.flags & SA_NODEFER) == SA_NODEFER)
new_signal_mask &= ~(1 << (signal - 1));
else
@@ -1182,8 +1182,7 @@ RegisterState& Thread::get_register_dump_from_stack()
ErrorOr<NonnullRefPtr<Thread>> Thread::try_clone(Process& process)
{
auto clone = TRY(Thread::try_create(process));
- auto signal_action_data_span = m_signal_action_data.span();
- signal_action_data_span.copy_to(clone->m_signal_action_data.span());
+ m_signal_action_masks.span().copy_to(clone->m_signal_action_masks);
clone->m_signal_mask = m_signal_mask;
clone->m_fpu_state = m_fpu_state;
clone->m_thread_specific_data = m_thread_specific_data;
diff --git a/Kernel/Thread.h b/Kernel/Thread.h
index 11d3363848..a0b5324300 100644
--- a/Kernel/Thread.h
+++ b/Kernel/Thread.h
@@ -46,12 +46,6 @@ enum class DispatchSignalResult {
Continue
};
-struct SignalActionData {
- VirtualAddress handler_or_sigaction;
- u32 mask { 0 };
- int flags { 0 };
-};
-
struct ThreadSpecificData {
ThreadSpecificData* self;
};
@@ -1225,7 +1219,7 @@ private:
NonnullOwnPtr<Memory::Region> m_kernel_stack_region;
VirtualAddress m_thread_specific_data;
Optional<Memory::VirtualRange> m_thread_specific_range;
- Array<SignalActionData, NSIG> m_signal_action_data;
+ Array<Optional<u32>, NSIG> m_signal_action_masks;
Blocker* m_blocker { nullptr };
Kernel::Mutex* m_blocking_mutex { nullptr };
u32 m_lock_requested_count { 0 };