From a685809e754005dca5dd8abc78b3de677bf64f8e Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Thu, 1 Nov 2018 01:04:02 +0100 Subject: [PATCH] Waiters should be notified when a waitee is killed. Ran into a horrendous bug where VirtualConsole would overrun its buffer and scribble right into some other object if we were interrupted while processing a character. Slapped an InterruptDisabler onto onChar for now. This provokes an interesting question though.. if a process is killed while its in kernel space, how the heck do we release any locks it held? I'm sure there are many different solutions to this problem, but I'll have to think about it. --- Kernel/Task.cpp | 27 ++++++++++++++++++------- Kernel/Task.h | 3 ++- Kernel/VirtualConsole.cpp | 5 +++++ LibC/unistd.h | 1 + VirtualFileSystem/FileHandle.cpp | 1 + VirtualFileSystem/UnixTypes.h | 5 +++++ VirtualFileSystem/VirtualFileSystem.cpp | 2 ++ 7 files changed, 36 insertions(+), 8 deletions(-) diff --git a/Kernel/Task.cpp b/Kernel/Task.cpp index 818e137782..b47a3e1192 100644 --- a/Kernel/Task.cpp +++ b/Kernel/Task.cpp @@ -518,6 +518,15 @@ void Task::dumpRegions() } } +void Task::notify_waiters(pid_t waitee, int exit_status, int signal) +{ + ASSERT_INTERRUPTS_DISABLED(); + for (auto* task = s_tasks->head(); task; task = task->next()) { + if (task->waitee() == waitee) + task->m_waiteeStatus = (exit_status << 8) | (signal); + } +} + void Task::sys$exit(int status) { cli(); @@ -531,10 +540,7 @@ void Task::sys$exit(int status) s_tasks->remove(this); - for (auto* task = s_tasks->head(); task; task = task->next()) { - if (task->waitee() == m_pid) - task->m_waiteeStatus = status << 8; - } + notify_waiters(m_pid, status, 0); if (!scheduleNewTask()) { kprintf("Task::sys$exit: Failed to schedule a new task :(\n"); @@ -546,13 +552,17 @@ void Task::sys$exit(int status) switchNow(); } -void Task::murder() +void Task::murder(int signal) { ASSERT_INTERRUPTS_DISABLED(); bool wasCurrent = current == this; setState(Exiting); s_tasks->remove(this); + + notify_waiters(m_pid, 0, signal); + if (wasCurrent) { + kprintf("Current task committing suicide!\n"); MM.unmapRegionsForTask(*this); if (!scheduleNewTask()) { kprintf("Task::murder: Failed to schedule a new task :(\n"); @@ -578,6 +588,8 @@ void Task::taskDidCrash(Task* crashedTask) s_tasks->remove(crashedTask); + notify_waiters(crashedTask->m_pid, 0, SIGSEGV); + MM.unmapRegionsForTask(*crashedTask); if (!scheduleNewTask()) { @@ -991,8 +1003,9 @@ int Task::sys$kill(pid_t pid, int sig) auto* peer = Task::fromPID(pid); if (!peer) return -ESRCH; - if (sig == 9) { - peer->murder(); + if (sig == SIGKILL) { + peer->murder(SIGKILL); + return 0; } else { ASSERT_NOT_REACHED(); } diff --git a/Kernel/Task.h b/Kernel/Task.h index 8f7e77716c..20c5000d7d 100644 --- a/Kernel/Task.h +++ b/Kernel/Task.h @@ -218,7 +218,8 @@ private: pid_t m_parentPID { 0 }; - void murder(); + static void notify_waiters(pid_t waitee, int exit_status, int signal); + void murder(int signal); Vector m_arguments; Vector m_initialEnvironment; diff --git a/Kernel/VirtualConsole.cpp b/Kernel/VirtualConsole.cpp index 433b438d48..6395fa03e1 100644 --- a/Kernel/VirtualConsole.cpp +++ b/Kernel/VirtualConsole.cpp @@ -300,6 +300,8 @@ void VirtualConsole::scrollUp() void VirtualConsole::setCursor(unsigned row, unsigned column) { + ASSERT(row < m_rows); + ASSERT(column < m_columns); m_cursorRow = row; m_cursorColumn = column; if (m_active) @@ -308,6 +310,8 @@ void VirtualConsole::setCursor(unsigned row, unsigned column) void VirtualConsole::putCharacterAt(unsigned row, unsigned column, byte ch) { + ASSERT(row < m_rows); + ASSERT(column < m_columns); word cur = (row * 160) + (column * 2); m_buffer[cur] = ch; m_buffer[cur + 1] = m_currentAttribute; @@ -317,6 +321,7 @@ void VirtualConsole::putCharacterAt(unsigned row, unsigned column, byte ch) void VirtualConsole::onChar(byte ch, bool shouldEmit) { + InterruptDisabler disabler; if (shouldEmit) emit(ch); diff --git a/LibC/unistd.h b/LibC/unistd.h index fcddc917e2..9212214998 100644 --- a/LibC/unistd.h +++ b/LibC/unistd.h @@ -30,6 +30,7 @@ off_t lseek(int fd, off_t, int whence); #define WEXITSTATUS(status) (((status) & 0xff00) >> 8) #define WTERMSIG(status) ((status) & 0x7f) #define WIFEXITED(status) (WTERMSIG(status) == 0) +#define WIFSIGNALED(status) (((char) (((status) & 0x7f) + 1) >> 1) > 0) #define HOST_NAME_MAX 64 diff --git a/VirtualFileSystem/FileHandle.cpp b/VirtualFileSystem/FileHandle.cpp index 7e795906e3..9285be1a87 100644 --- a/VirtualFileSystem/FileHandle.cpp +++ b/VirtualFileSystem/FileHandle.cpp @@ -5,6 +5,7 @@ #include "UnixTypes.h" #include "TTY.h" #include +#include FileHandle::FileHandle(RetainPtr&& vnode) : m_vnode(move(vnode)) diff --git a/VirtualFileSystem/UnixTypes.h b/VirtualFileSystem/UnixTypes.h index eeed7bb94b..a55422a921 100644 --- a/VirtualFileSystem/UnixTypes.h +++ b/VirtualFileSystem/UnixTypes.h @@ -8,6 +8,11 @@ namespace Unix { #define SEEK_CUR 1 #define SEEK_END 2 +#define SIGINT 2 +#define SIGKILL 9 +#define SIGSEGV 11 +#define SIGTERM 15 + typedef dword dev_t; typedef dword ino_t; typedef dword mode_t; diff --git a/VirtualFileSystem/VirtualFileSystem.cpp b/VirtualFileSystem/VirtualFileSystem.cpp index 19de414dcf..4ef6ba5df1 100644 --- a/VirtualFileSystem/VirtualFileSystem.cpp +++ b/VirtualFileSystem/VirtualFileSystem.cpp @@ -558,11 +558,13 @@ InodeIdentifier VirtualFileSystem::resolvePath(const String& path, int& error, I void VirtualFileSystem::Node::retain() { + InterruptDisabler disabler; // FIXME: Make a Retainable with atomic retain count instead. ++retainCount; } void VirtualFileSystem::Node::release() { + InterruptDisabler disabler; // FIXME: Make a Retainable with atomic retain count instead. ASSERT(retainCount); if (--retainCount == 0) { m_vfs->freeNode(this);