From 6a6eb8844a79e317aa87de8c8200b7feb47f8ff9 Mon Sep 17 00:00:00 2001
From: Andreas Kling <kling@serenityos.org>
Date: Mon, 1 Mar 2021 14:05:42 +0100
Subject: Kernel: Use Userspace<T> in sys$sigaction()

fuzz-syscalls found a bunch of unaligned accesses into struct sigaction
via this syscall. This patch fixes that issue by porting the syscall
to Userspace<T> which we should have done anyway. :^)

Fixes #5500.
---
 Kernel/Process.h              |  2 +-
 Kernel/Syscalls/sigaction.cpp | 22 +++++++++++++---------
 2 files changed, 14 insertions(+), 10 deletions(-)

diff --git a/Kernel/Process.h b/Kernel/Process.h
index 13140665bd..d663448dd3 100644
--- a/Kernel/Process.h
+++ b/Kernel/Process.h
@@ -279,7 +279,7 @@ public:
     KResultOr<pid_t> sys$fork(RegisterState&);
     KResultOr<int> sys$execve(Userspace<const Syscall::SC_execve_params*>);
     KResultOr<int> sys$dup2(int old_fd, int new_fd);
-    KResultOr<int> sys$sigaction(int signum, const sigaction* act, sigaction* old_act);
+    KResultOr<int> sys$sigaction(int signum, Userspace<const sigaction*> act, Userspace<sigaction*> old_act);
     KResultOr<int> sys$sigprocmask(int how, Userspace<const sigset_t*> set, Userspace<sigset_t*> old_set);
     KResultOr<int> sys$sigpending(Userspace<sigset_t*>);
     KResultOr<int> sys$getgroups(ssize_t, Userspace<gid_t*>);
diff --git a/Kernel/Syscalls/sigaction.cpp b/Kernel/Syscalls/sigaction.cpp
index dcd875d087..cd59d44477 100644
--- a/Kernel/Syscalls/sigaction.cpp
+++ b/Kernel/Syscalls/sigaction.cpp
@@ -69,23 +69,27 @@ KResultOr<int> Process::sys$sigpending(Userspace<sigset_t*> set)
     return 0;
 }
 
-KResultOr<int> Process::sys$sigaction(int signum, const sigaction* act, sigaction* old_act)
+KResultOr<int> Process::sys$sigaction(int signum, Userspace<const sigaction*> user_act, Userspace<sigaction*> user_old_act)
 {
     REQUIRE_PROMISE(sigaction);
     if (signum < 1 || signum >= 32 || signum == SIGKILL || signum == SIGSTOP)
         return EINVAL;
+
+    sigaction act {};
+    if (!copy_from_user(&act, user_act))
+        return EFAULT;
+
     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];
-    if (old_act) {
-        if (!copy_to_user(&old_act->sa_flags, &action.flags))
-            return EFAULT;
-        if (!copy_to_user(&old_act->sa_sigaction, &action.handler_or_sigaction, sizeof(action.handler_or_sigaction)))
+    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());
+        if (!copy_to_user(user_old_act, &old_act))
             return EFAULT;
     }
-    if (!copy_from_user(&action.flags, &act->sa_flags))
-        return EFAULT;
-    if (!copy_from_user(&action.handler_or_sigaction, &act->sa_sigaction, sizeof(action.handler_or_sigaction)))
-        return EFAULT;
+    action.flags = act.sa_flags;
+    action.handler_or_sigaction = VirtualAddress { reinterpret_cast<void*>(act.sa_sigaction) };
     return 0;
 }
 
-- 
cgit v1.2.3