diff options
author | Andreas Kling <kling@serenityos.org> | 2021-03-01 14:05:42 +0100 |
---|---|---|
committer | Andreas Kling <kling@serenityos.org> | 2021-03-01 14:06:20 +0100 |
commit | 6a6eb8844a79e317aa87de8c8200b7feb47f8ff9 (patch) | |
tree | 819cc047b69f1a56a07f23715a7d3a50b571dc39 | |
parent | 261b30e120924dd33ff517125dd9770d8be1d4d1 (diff) | |
download | serenity-6a6eb8844a79e317aa87de8c8200b7feb47f8ff9.zip |
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.
-rw-r--r-- | Kernel/Process.h | 2 | ||||
-rw-r--r-- | 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; } |