From b3a24d732d22a7ebbb2ded76bc056cd985ae48fe Mon Sep 17 00:00:00 2001 From: Sergey Bugaev Date: Wed, 5 Feb 2020 19:42:43 +0300 Subject: [PATCH] Kernel+LibC: Add sys$waitid(), and make sys$waitpid() wrap it sys$waitid() takes an explicit description of whether it's waiting for a single process with the given PID, all of the children, a group, etc., and returns its info as a siginfo_t. It also doesn't automatically imply WEXITED, which clears up the confusion in the kernel. --- Kernel/Process.cpp | 128 ++++++++++++++++++++++-------------- Kernel/Process.h | 6 +- Kernel/Syscall.h | 10 ++- Libraries/LibC/sys/wait.cpp | 47 ++++++++++++- Libraries/LibC/sys/wait.h | 4 +- 5 files changed, 140 insertions(+), 55 deletions(-) diff --git a/Kernel/Process.cpp b/Kernel/Process.cpp index 484147c12c..bff7eaf360 100644 --- a/Kernel/Process.cpp +++ b/Kernel/Process.cpp @@ -2266,12 +2266,23 @@ mode_t Process::sys$umask(mode_t mask) return old_mask; } -int Process::reap(Process& process) +siginfo_t Process::reap(Process& process) { - int exit_status; + siginfo_t siginfo; + siginfo.si_signo = SIGCHLD; + siginfo.si_pid = process.pid(); + siginfo.si_uid = process.uid(); + + if (process.m_termination_signal) { + siginfo.si_status = process.m_termination_signal; + siginfo.si_code = CLD_KILLED; + } else { + siginfo.si_status = process.m_termination_status; + siginfo.si_code = CLD_EXITED; + } + { InterruptDisabler disabler; - exit_status = (process.m_termination_status << 8) | process.m_termination_signal; if (process.ppid()) { auto* parent = Process::from_pid(process.ppid()); @@ -2288,85 +2299,102 @@ int Process::reap(Process& process) g_processes->remove(&process); } delete &process; - return exit_status; + return siginfo; } -pid_t Process::sys$waitpid(pid_t waitee, int* wstatus, int options) +KResultOr Process::do_waitid(idtype_t idtype, int id, int options) { - REQUIRE_PROMISE(stdio); - -#ifdef PROCESS_DEBUG - dbg() << "sys$waitpid(" << waitee << ", " << wstatus << ", " << options << ")"; -#endif - - if (!options) { - // FIXME: This can't be right.. can it? Figure out how this should actually work. - options = WEXITED; - } - - if (wstatus && !validate_write_typed(wstatus)) - return -EFAULT; - - int exit_status = 0; - - { + if (idtype == P_PID) { InterruptDisabler disabler; - if (waitee != -1 && !Process::from_pid(waitee)) - return -ECHILD; + if (idtype == P_PID && !Process::from_pid(id)) + return KResult(-ECHILD); } if (options & WNOHANG) { // FIXME: Figure out what WNOHANG should do with stopped children. - if (waitee == -1) { - pid_t reaped_pid = 0; + if (idtype == P_ALL) { InterruptDisabler disabler; - for_each_child([&reaped_pid, &exit_status](Process& process) { - if (process.is_dead()) { - reaped_pid = process.pid(); - exit_status = reap(process); - } + siginfo_t siginfo; + for_each_child([&siginfo](Process& process) { + if (process.is_dead()) + siginfo = reap(process); return IterationDecision::Continue; }); - return reaped_pid; - } else { - ASSERT(waitee > 0); // FIXME: Implement other PID specs. + return siginfo; + } else if (idtype == P_PID) { InterruptDisabler disabler; - auto* waitee_process = Process::from_pid(waitee); + auto* waitee_process = Process::from_pid(id); if (!waitee_process) - return -ECHILD; - if (waitee_process->is_dead()) { - exit_status = reap(*waitee_process); - return waitee; - } - return 0; + return KResult(-ECHILD); + if (waitee_process->is_dead()) + return reap(*waitee_process); + } else { + // FIXME: Implement other PID specs. + return KResult(-EINVAL); } } - pid_t waitee_pid = waitee; + pid_t waitee_pid; + + // FIXME: WaitBlocker should support idtype/id specs directly. + if (idtype == P_ALL) { + waitee_pid = -1; + } else if (idtype == P_PID) { + waitee_pid = id; + } else { + // FIXME: Implement other PID specs. + return KResult(-EINVAL); + } + if (current->block(options, waitee_pid) != Thread::BlockResult::WokeNormally) - return -EINTR; + return KResult(-EINTR); InterruptDisabler disabler; // NOTE: If waitee was -1, m_waitee_pid will have been filled in by the scheduler. Process* waitee_process = Process::from_pid(waitee_pid); if (!waitee_process) - return -ECHILD; + return KResult(-ECHILD); ASSERT(waitee_process); if (waitee_process->is_dead()) { - exit_status = reap(*waitee_process); + return reap(*waitee_process); } else { auto* waitee_thread = Thread::from_tid(waitee_pid); if (!waitee_thread) - return -ECHILD; + return KResult(-ECHILD); ASSERT(waitee_thread->state() == Thread::State::Stopped); - exit_status = (waitee_thread->m_stop_signal << 8) | 0x7f; + siginfo_t siginfo; + siginfo.si_signo = SIGCHLD; + siginfo.si_pid = waitee_process->pid(); + siginfo.si_uid = waitee_process->uid(); + siginfo.si_status = CLD_STOPPED; + siginfo.si_code = waitee_thread->m_stop_signal; + return siginfo; } +} - if (wstatus) - copy_to_user(wstatus, &exit_status); - return waitee_pid; +pid_t Process::sys$waitid(const Syscall::SC_waitid_params* user_params) +{ + REQUIRE_PROMISE(stdio); + + Syscall::SC_waitid_params params; + if (!validate_read_and_copy_typed(¶ms, user_params)) + return -EFAULT; + + if (!validate_write_typed(params.infop)) + return -EFAULT; + + //#ifdef PROCESS_DEBUG + dbg() << "sys$waitid(" << params.idtype << ", " << params.id << ", " << params.infop << ", " << params.options << ")"; + //#endif + + auto siginfo_or_error = do_waitid(static_cast(params.idtype), params.id, params.options); + if (siginfo_or_error.is_error()) + return siginfo_or_error.error(); + + copy_to_user(params.infop, &siginfo_or_error.value()); + return 0; } bool Process::validate_read_from_kernel(VirtualAddress vaddr, size_t size) const diff --git a/Kernel/Process.h b/Kernel/Process.h index 9dc4538fb7..5085aa4e5e 100644 --- a/Kernel/Process.h +++ b/Kernel/Process.h @@ -201,7 +201,7 @@ public: int sys$kill(pid_t pid, int sig); [[noreturn]] void sys$exit(int status); int sys$sigreturn(RegisterDump& registers); - pid_t sys$waitpid(pid_t, int* wstatus, int options); + pid_t sys$waitid(const Syscall::SC_waitid_params*); void* sys$mmap(const Syscall::SC_mmap_params*); int sys$munmap(void*, size_t size); int sys$set_mmap_name(const Syscall::SC_set_mmap_name_params*); @@ -310,7 +310,7 @@ public: static void initialize(); [[noreturn]] void crash(int signal, u32 eip); - [[nodiscard]] static int reap(Process&); + [[nodiscard]] static siginfo_t reap(Process&); const TTY* tty() const { return m_tty; } void set_tty(TTY* tty) { m_tty = tty; } @@ -430,6 +430,8 @@ private: KResult do_kill(Process&, int signal); KResult do_killpg(pid_t pgrp, int signal); + KResultOr do_waitid(idtype_t idtype, int id, int options); + KResultOr get_syscall_path_argument(const char* user_path, size_t path_length) const; KResultOr get_syscall_path_argument(const Syscall::StringArgument&) const; diff --git a/Kernel/Syscall.h b/Kernel/Syscall.h index 19f755a99e..27dfb1dc7f 100644 --- a/Kernel/Syscall.h +++ b/Kernel/Syscall.h @@ -36,6 +36,7 @@ extern "C" { struct timeval; struct timespec; struct sockaddr; +struct siginfo; typedef u32 socklen_t; } @@ -51,7 +52,7 @@ typedef u32 socklen_t; __ENUMERATE_SYSCALL(exit) \ __ENUMERATE_SYSCALL(getgid) \ __ENUMERATE_SYSCALL(getpid) \ - __ENUMERATE_SYSCALL(waitpid) \ + __ENUMERATE_SYSCALL(waitid) \ __ENUMERATE_SYSCALL(mmap) \ __ENUMERATE_SYSCALL(munmap) \ __ENUMERATE_SYSCALL(get_dir_entries) \ @@ -404,6 +405,13 @@ struct SC_unveil_params { StringArgument permissions; }; +struct SC_waitid_params { + int idtype; + int id; + struct siginfo* infop; + int options; +}; + void initialize(); int sync(); diff --git a/Libraries/LibC/sys/wait.cpp b/Libraries/LibC/sys/wait.cpp index 140b5a3f74..1131c07d57 100644 --- a/Libraries/LibC/sys/wait.cpp +++ b/Libraries/LibC/sys/wait.cpp @@ -38,7 +38,52 @@ pid_t wait(int* wstatus) pid_t waitpid(pid_t waitee, int* wstatus, int options) { - int rc = syscall(SC_waitpid, waitee, wstatus, options); + siginfo_t siginfo; + idtype_t idtype; + id_t id; + + if (waitee < -1) { + idtype = P_PGID; + id = -waitee; + } else if (waitee == -1) { + idtype = P_ALL; + id = 0; + } else if (waitee == 0) { + idtype = P_PGID; + id = getgid(); + } else { + idtype = P_PID; + id = waitee; + } + + int rc = waitid(idtype, id, &siginfo, options | WEXITED); + + if (rc < 0) + return rc; + + if (wstatus) { + switch (siginfo.si_code) { + case CLD_EXITED: + *wstatus = siginfo.si_status << 8; + break; + case CLD_KILLED: + *wstatus = siginfo.si_status; + break; + case CLD_STOPPED: + *wstatus = siginfo.si_status << 8 | 0x7f; + break; + default: + ASSERT_NOT_REACHED(); + } + } + + return siginfo.si_pid; +} + +int waitid(idtype_t idtype, id_t id, siginfo_t* infop, int options) +{ + Syscall::SC_waitid_params params { idtype, id, infop, options }; + int rc = syscall(SC_waitid, ¶ms); __RETURN_WITH_ERRNO(rc, rc, -1); } } diff --git a/Libraries/LibC/sys/wait.h b/Libraries/LibC/sys/wait.h index a0b49a0046..fadc12d780 100644 --- a/Libraries/LibC/sys/wait.h +++ b/Libraries/LibC/sys/wait.h @@ -26,6 +26,7 @@ #pragma once +#include #include #include @@ -35,7 +36,7 @@ __BEGIN_DECLS #define WSTOPSIG(status) WEXITSTATUS(status) #define WTERMSIG(status) ((status)&0x7f) #define WIFEXITED(status) (WTERMSIG(status) == 0) -#define WIFSTOPPED(status) (((status) & 0xff) == 0x7f) +#define WIFSTOPPED(status) (((status)&0xff) == 0x7f) #define WIFSIGNALED(status) (((char)(((status)&0x7f) + 1) >> 1) > 0) #define WNOHANG 1 @@ -52,5 +53,6 @@ typedef enum { pid_t waitpid(pid_t, int* wstatus, int options); pid_t wait(int* wstatus); +int waitid(idtype_t idtype, id_t id, siginfo_t* infop, int options); __END_DECLS