Kernel: Various stability improvements.

- Don't cli() in Process::do_exec() unless current is execing.
  Eventually this should go away once the scheduler is less retarded
  in the face of interrupts.

- Improved memory access validation for ring0 processes.
  We now look at the kernel ELF header to determine if an access
  is appropriate. :^) It's very hackish but also kinda neat.

- Have Process::die() put the process into a new "Dying" state where
  it can still get scheduled but no signals will be dispatched.
  This way we can keep executing in die() but won't get our EIP
  hijacked by signal dispatch. The main problem here was that die()
  wanted to take various locks.
This commit is contained in:
Andreas Kling 2019-02-06 17:28:14 +01:00
parent 03c0e836f0
commit e05237485c
4 changed files with 41 additions and 14 deletions

View file

@ -885,7 +885,6 @@ size_t Region::amount_shared() const
PageDirectory::~PageDirectory()
{
ASSERT_INTERRUPTS_DISABLED();
#ifdef MM_DEBUG
dbgprintf("MM: ~PageDirectory K%x\n", this);
#endif

View file

@ -19,6 +19,7 @@
#include "KSyms.h"
#include <WindowServer/WSWindow.h>
#include "MasterPTY.h"
#include "elf.h"
//#define DEBUG_IO
//#define TASK_DEBUG
@ -394,7 +395,8 @@ int Process::do_exec(const String& path, Vector<String>&& arguments, Vector<Stri
// We cli() manually here because we don't want to get interrupted between do_exec() and Schedule::yield().
// The reason is that the task redirection we've set up above will be clobbered by the timer IRQ.
// If we used an InterruptDisabler that sti()'d on exit, we might timer tick'd too soon in exec().
cli();
if (current == this)
cli();
Scheduler::prepare_to_modify_tss(*this);
@ -1623,10 +1625,22 @@ void sleep(dword ticks)
Scheduler::yield();
}
static bool is_inside_kernel_code(LinearAddress laddr)
static bool check_kernel_memory_access(LinearAddress laddr, bool is_write)
{
// FIXME: What if we're indexing into the ksym with the highest address though?
return laddr.get() >= ksym_lowest_address && laddr.get() <= ksym_highest_address;
auto* kernel_elf_header = (Elf32_Ehdr*)0xf000;
auto* kernel_program_headers = (Elf32_Phdr*)(0xf000 + kernel_elf_header->e_phoff);
for (unsigned i = 0; i < kernel_elf_header->e_phnum; ++i) {
auto& segment = kernel_program_headers[i];
if (laddr.get() < segment.p_vaddr || laddr.get() > (segment.p_vaddr + segment.p_memsz))
continue;
if (is_write && !(kernel_program_headers[i].p_flags & PF_W))
return false;
if (!is_write && !(kernel_program_headers[i].p_flags & PF_R))
return false;
return true;
}
// Returning true in this case means "it's not inside the kernel binary. let the other checks deal with it."
return true;
}
bool Process::validate_read_from_kernel(LinearAddress laddr) const
@ -1635,7 +1649,7 @@ bool Process::validate_read_from_kernel(LinearAddress laddr) const
// This code allows access outside of the known used address ranges to get caught.
InterruptDisabler disabler;
if (is_inside_kernel_code(laddr))
if (check_kernel_memory_access(laddr, false))
return true;
if (is_kmalloc_address(laddr.as_ptr()))
return true;
@ -1645,7 +1659,7 @@ bool Process::validate_read_from_kernel(LinearAddress laddr) const
bool Process::validate_read(const void* address, size_t size) const
{
if (is_ring0()) {
if (is_inside_kernel_code(LinearAddress((dword)address)))
if (check_kernel_memory_access(LinearAddress((dword)address), false))
return true;
if (is_kmalloc_address(address))
return true;
@ -1667,6 +1681,8 @@ bool Process::validate_write(void* address, size_t size) const
if (is_ring0()) {
if (is_kmalloc_address(address))
return true;
if (check_kernel_memory_access(LinearAddress((dword)address), true))
return true;
}
ASSERT(size);
if (!size)
@ -2108,15 +2124,25 @@ int Process::sys$chmod(const char* pathname, mode_t mode)
void Process::die()
{
// This is pretty hairy wrt interrupts. Once we set_state(Dead), we never get scheduled again.
// For this reason, we mark ourselves as Dying, which prevents the scheduler from dispatching signals in this process.
set_state(Dying);
InterruptFlagSaver saver;
// STI so we can take locks.
sti();
destroy_all_windows();
set_state(Dead);
m_fds.clear();
m_tty = nullptr;
InterruptDisabler disabler;
// CLI for Process::from_pid(). This should go away eventually.
cli();
if (auto* parent_process = Process::from_pid(m_ppid)) {
parent_process->send_signal(SIGCHLD, this);
}
// Good night.
set_state(Dead);
}
size_t Process::amount_virtual() const

View file

@ -67,6 +67,7 @@ public:
Running,
Skip1SchedulerPass,
Skip0SchedulerPasses,
Dying,
Dead,
BeingInspected,
BlockedSleep,
@ -125,7 +126,7 @@ public:
template<typename Callback> static void for_each(Callback);
template<typename Callback> static void for_each_in_pgrp(pid_t, Callback);
template<typename Callback> static void for_each_in_state(State, Callback);
template<typename Callback> static void for_each_not_in_state(State, Callback);
template<typename Callback> static void for_each_living(Callback);
template<typename Callback> void for_each_child(Callback);
bool tick() { ++m_ticks; return --m_ticks_left; }
@ -442,6 +443,7 @@ static inline const char* to_string(Process::State state)
case Process::Invalid: return "Invalid";
case Process::Runnable: return "Runnable";
case Process::Running: return "Running";
case Process::Dying: return "Dying";
case Process::Dead: return "Dead";
case Process::Skip1SchedulerPass: return "Skip1";
case Process::Skip0SchedulerPasses: return "Skip0";
@ -516,12 +518,12 @@ inline void Process::for_each_in_state(State state, Callback callback)
}
template<typename Callback>
inline void Process::for_each_not_in_state(State state, Callback callback)
inline void Process::for_each_living(Callback callback)
{
ASSERT_INTERRUPTS_DISABLED();
for (auto* process = g_processes->head(); process;) {
auto* next_process = process->next();
if (process->state() != state)
if (process->state() != Process::Dead && process->state() != Process::Dying)
callback(*process);
process = next_process;
}

View file

@ -132,7 +132,7 @@ bool Scheduler::pick_next()
// Dispatch any pending signals.
// FIXME: Do we really need this to be a separate pass over the process list?
Process::for_each_not_in_state(Process::Dead, [] (auto& process) {
Process::for_each_living([] (auto& process) {
if (!process.has_unmasked_pending_signals())
return true;
// We know how to interrupt blocked processes, but if they are just executing
@ -170,7 +170,7 @@ bool Scheduler::pick_next()
g_processes->append(g_processes->remove_head());
auto* process = g_processes->head();
if (process->state() == Process::Runnable || process->state() == Process::Running) {
if (process->state() == Process::Runnable || process->state() == Process::Running || process->state() == Process::Dying) {
#ifdef SCHEDULER_DEBUG
dbgprintf("switch to %s(%u) @ %w:%x\n", process->name().characters(), process->pid(), process->tss().cs, process->tss().eip);
#endif