diff --git a/Kernel/BXVGADevice.cpp b/Kernel/BXVGADevice.cpp index 0d6e2fd48d3..186329ddbcb 100644 --- a/Kernel/BXVGADevice.cpp +++ b/Kernel/BXVGADevice.cpp @@ -61,8 +61,6 @@ void BXVGADevice::set_resolution(int width, int height) set_register(VBE_DISPI_INDEX_BPP, 32); set_register(VBE_DISPI_INDEX_ENABLE, VBE_DISPI_ENABLED | VBE_DISPI_LFB_ENABLED); set_register(VBE_DISPI_INDEX_BANK, 0); - - m_framebuffer_size = { width, height }; } void BXVGADevice::set_y_offset(int offset) diff --git a/Kernel/Process.cpp b/Kernel/Process.cpp index 2d317a296b3..43b8d98c58a 100644 --- a/Kernel/Process.cpp +++ b/Kernel/Process.cpp @@ -26,7 +26,7 @@ //#define DEBUG_IO //#define TASK_DEBUG //#define FORK_DEBUG -#define SIGNAL_DEBUG +//#define SIGNAL_DEBUG #define MAX_PROCESS_GIDS 32 //#define SHARED_BUFFER_DEBUG @@ -370,7 +370,8 @@ int Process::do_exec(String path, Vector arguments, Vector envir } } - m_signal_stack_kernel_region = nullptr; + kfree(m_kernel_stack_for_signal_handler); + m_kernel_stack_for_signal_handler = nullptr; m_signal_stack_user_region = nullptr; m_display_framebuffer_region = nullptr; set_default_signal_dispositions(); @@ -725,6 +726,11 @@ Process::~Process() kfree(m_kernel_stack); m_kernel_stack = nullptr; } + + if (m_kernel_stack_for_signal_handler) { + kfree(m_kernel_stack_for_signal_handler); + m_kernel_stack_for_signal_handler = nullptr; + } } void Process::dump_regions() @@ -912,8 +918,11 @@ ShouldUnblockProcess Process::dispatch_signal(byte signal) word ret_cs = m_tss.cs; dword ret_eip = m_tss.eip; dword ret_eflags = m_tss.eflags; - bool interrupting_in_kernel = (ret_cs & 3) == 0; + + ProcessPagingScope paging_scope(*this); + create_signal_trampolines_if_needed(); + if (interrupting_in_kernel) { #ifdef SIGNAL_DEBUG kprintf("dispatch_signal to %s(%u) in state=%s with return to %w:%x\n", name().characters(), pid(), to_string(state()), ret_cs, ret_eip); @@ -921,23 +930,23 @@ ShouldUnblockProcess Process::dispatch_signal(byte signal) ASSERT(is_blocked()); m_tss_to_resume_kernel = m_tss; #ifdef SIGNAL_DEBUG - kprintf("resume tss pc: %w:%x\n", m_tss_to_resume_kernel.cs, m_tss_to_resume_kernel.eip); + kprintf("resume tss pc: %w:%x stack: %w:%x flags: %x cr3: %x\n", m_tss_to_resume_kernel.cs, m_tss_to_resume_kernel.eip, m_tss_to_resume_kernel.ss, m_tss_to_resume_kernel.esp, m_tss_to_resume_kernel.eflags, m_tss_to_resume_kernel.cr3); #endif - } - ProcessPagingScope paging_scope(*this); - - if (interrupting_in_kernel) { if (!m_signal_stack_user_region) { - m_signal_stack_user_region = allocate_region(LinearAddress(), default_userspace_stack_size, "signal stack (user)"); + m_signal_stack_user_region = allocate_region(LinearAddress(), default_userspace_stack_size, "Signal stack (user)"); ASSERT(m_signal_stack_user_region); - m_signal_stack_kernel_region = allocate_region(LinearAddress(), default_userspace_stack_size, "signal stack (kernel)"); - ASSERT(m_signal_stack_kernel_region); + } + if (!m_kernel_stack_for_signal_handler) { + m_kernel_stack_for_signal_handler = kmalloc(default_kernel_stack_size); + ASSERT(m_kernel_stack_for_signal_handler); } m_tss.ss = 0x23; m_tss.esp = m_signal_stack_user_region->laddr().offset(default_userspace_stack_size).get(); m_tss.ss0 = 0x10; - m_tss.esp0 = m_signal_stack_kernel_region->laddr().offset(default_userspace_stack_size).get(); + m_tss.esp0 = (dword)m_kernel_stack_for_signal_handler + default_kernel_stack_size; + + push_value_on_stack(0); } else { push_value_on_stack(ret_eip); push_value_on_stack(ret_eflags); @@ -952,6 +961,9 @@ ShouldUnblockProcess Process::dispatch_signal(byte signal) push_value_on_stack(m_tss.ebp); push_value_on_stack(m_tss.esi); push_value_on_stack(m_tss.edi); + + // Align the stack. + m_tss.esp -= 12; } // PUSH old_signal_mask @@ -964,42 +976,6 @@ ShouldUnblockProcess Process::dispatch_signal(byte signal) m_tss.gs = 0x23; m_tss.eip = handler_laddr.get(); - if (m_return_to_ring3_from_signal_trampoline.is_null()) { - // FIXME: This should be a global trampoline shared by all processes, not one created per process! - // FIXME: Remap as read-only after setup. - auto* region = allocate_region(LinearAddress(), PAGE_SIZE, "signal_trampoline", true, true); - m_return_to_ring3_from_signal_trampoline = region->laddr(); - byte* code_ptr = m_return_to_ring3_from_signal_trampoline.as_ptr(); - *code_ptr++ = 0x58; // pop eax (Skip over signal argument) - *code_ptr++ = 0x5a; // pop edx - *code_ptr++ = 0xb8; // mov eax, - *(dword*)code_ptr = Syscall::SC_restore_signal_mask; - code_ptr += sizeof(dword); - *code_ptr++ = 0xcd; // int 0x82 - *code_ptr++ = 0x82; - *code_ptr++ = 0x61; // popa - *code_ptr++ = 0x9d; // popf - *code_ptr++ = 0xc3; // ret - *code_ptr++ = 0x0f; // ud2 - *code_ptr++ = 0x0b; - - m_return_to_ring0_from_signal_trampoline = LinearAddress((dword)code_ptr); - *code_ptr++ = 0x58; // pop eax (Skip over signal argument) - *code_ptr++ = 0x5a; // pop edx - *code_ptr++ = 0xb8; // mov eax, - *(dword*)code_ptr = Syscall::SC_restore_signal_mask; - code_ptr += sizeof(dword); - *code_ptr++ = 0xcd; // int 0x82 - *code_ptr++ = 0x82; - *code_ptr++ = 0xb8; // mov eax, - *(dword*)code_ptr = Syscall::SC_sigreturn; - code_ptr += sizeof(dword); - *code_ptr++ = 0xcd; // int 0x82 - *code_ptr++ = 0x82; - *code_ptr++ = 0x0f; // ud2 - *code_ptr++ = 0x0b; - } - // FIXME: Should we worry about the stack being 16 byte aligned when entering a signal handler? push_value_on_stack(signal); @@ -1008,6 +984,8 @@ ShouldUnblockProcess Process::dispatch_signal(byte signal) else push_value_on_stack(m_return_to_ring3_from_signal_trampoline.get()); + ASSERT((m_tss.esp % 16) == 0); + // FIXME: This state is such a hack. It avoids trouble if 'current' is the process receiving a signal. set_state(Skip1SchedulerPass); @@ -1017,6 +995,54 @@ ShouldUnblockProcess Process::dispatch_signal(byte signal) return ShouldUnblockProcess::Yes; } +void Process::create_signal_trampolines_if_needed() +{ + if (!m_return_to_ring3_from_signal_trampoline.is_null()) + return; + // FIXME: This should be a global trampoline shared by all processes, not one created per process! + // FIXME: Remap as read-only after setup. + auto* region = allocate_region(LinearAddress(), PAGE_SIZE, "Signal trampolines", true, true); + m_return_to_ring3_from_signal_trampoline = region->laddr(); + byte* code_ptr = m_return_to_ring3_from_signal_trampoline.as_ptr(); + *code_ptr++ = 0x58; // pop eax (Argument to signal handler (ignored here)) + *code_ptr++ = 0x5a; // pop edx (Original signal mask to restore) + *code_ptr++ = 0xb8; // mov eax, + *(dword*)code_ptr = Syscall::SC_restore_signal_mask; + code_ptr += sizeof(dword); + *code_ptr++ = 0xcd; // int 0x82 + *code_ptr++ = 0x82; + + *code_ptr++ = 0x83; // add esp, (stack alignment padding) + *code_ptr++ = 0xc4; + *code_ptr++ = sizeof(dword) * 3; + + *code_ptr++ = 0x61; // popa + *code_ptr++ = 0x9d; // popf + *code_ptr++ = 0xc3; // ret + *code_ptr++ = 0x0f; // ud2 + *code_ptr++ = 0x0b; + + m_return_to_ring0_from_signal_trampoline = LinearAddress((dword)code_ptr); + *code_ptr++ = 0x58; // pop eax (Argument to signal handler (ignored here)) + *code_ptr++ = 0x5a; // pop edx (Original signal mask to restore) + *code_ptr++ = 0xb8; // mov eax, + *(dword*)code_ptr = Syscall::SC_restore_signal_mask; + code_ptr += sizeof(dword); + *code_ptr++ = 0xcd; // int 0x82 + + // NOTE: Stack alignment padding doesn't matter when returning to ring0. + // Nothing matters really, as we're returning by replacing the entire TSS. + + *code_ptr++ = 0x82; + *code_ptr++ = 0xb8; // mov eax, + *(dword*)code_ptr = Syscall::SC_sigreturn; + code_ptr += sizeof(dword); + *code_ptr++ = 0xcd; // int 0x82 + *code_ptr++ = 0x82; + *code_ptr++ = 0x0f; // ud2 + *code_ptr++ = 0x0b; +} + int Process::sys$restore_signal_mask(dword mask) { m_signal_mask = mask; @@ -1030,7 +1056,7 @@ void Process::sys$sigreturn() m_tss = m_tss_to_resume_kernel; #ifdef SIGNAL_DEBUG kprintf("sys$sigreturn in %s(%u)\n", name().characters(), pid()); - kprintf(" -> resuming execution at %w:%x\n", m_tss.cs, m_tss.eip); + kprintf(" -> resuming execution at %w:%x stack %w:%x flags %x cr3 %x\n", m_tss.cs, m_tss.eip, m_tss.ss, m_tss.esp, m_tss.eflags, m_tss.cr3); #endif set_state(Skip1SchedulerPass); Scheduler::yield(); diff --git a/Kernel/Process.h b/Kernel/Process.h index db1080ad521..70cc33f2200 100644 --- a/Kernel/Process.h +++ b/Kernel/Process.h @@ -146,6 +146,9 @@ public: void set_ticks_left(dword t) { m_ticks_left = t; } dword ticks_left() const { return m_ticks_left; } + dword kernel_stack_base() const { return (dword)m_kernel_stack; }; + dword kernel_stack_for_signal_handler_base() const { return (dword)m_kernel_stack_for_signal_handler; }; + void set_selector(word s) { m_far_ptr.selector = s; } void set_state(State s) { m_state = s; } void die(); @@ -319,6 +322,8 @@ private: void set_default_signal_dispositions(); void disown_all_shared_buffers(); + void create_signal_trampolines_if_needed(); + RetainPtr m_page_directory; Process* m_prev { nullptr }; @@ -355,6 +360,7 @@ private: RingLevel m_ring { Ring0 }; int m_error { 0 }; void* m_kernel_stack { nullptr }; + void* m_kernel_stack_for_signal_handler { nullptr }; dword m_times_scheduled { 0 }; pid_t m_waitee_pid { -1 }; int m_blocked_fd { -1 }; @@ -397,7 +403,6 @@ private: HashTable m_gids; Region* m_signal_stack_user_region { nullptr }; - Region* m_signal_stack_kernel_region { nullptr }; RetainPtr m_display_framebuffer_region; diff --git a/Kernel/Scheduler.cpp b/Kernel/Scheduler.cpp index d0e091ee346..032ca74ab1b 100644 --- a/Kernel/Scheduler.cpp +++ b/Kernel/Scheduler.cpp @@ -162,6 +162,11 @@ bool Scheduler::pick_next() Process::for_each_living([] (auto& process) { if (!process.has_unmasked_pending_signals()) return true; + // FIXME: It would be nice if the Scheduler didn't have to worry about who is "current" + // For now, avoid dispatching signals to "current" and do it in a scheduling pass + // while some other process is interrupted. Otherwise a mess will be made. + if (&process == current) + return true; // We know how to interrupt blocked processes, but if they are just executing // at some random point in the kernel, let them continue. They'll be in userspace // sooner or later and we can deliver the signal then. @@ -199,7 +204,7 @@ bool Scheduler::pick_next() if (process->state() == Process::Runnable || process->state() == Process::Running) { #ifdef SCHEDULER_DEBUG - dbgprintf("switch to %s(%u) @ %w:%x\n", process->name().characters(), process->pid(), process->tss().cs, process->tss().eip); + kprintf("switch to %s(%u) @ %w:%x\n", process->name().characters(), process->pid(), process->tss().cs, process->tss().eip); #endif return context_switch(*process); } diff --git a/Kernel/i386.cpp b/Kernel/i386.cpp index 33fabb49e7f..2561b4a4f67 100644 --- a/Kernel/i386.cpp +++ b/Kernel/i386.cpp @@ -137,8 +137,10 @@ static void dump(const DumpType& regs) if constexpr (IsSame::value) { kprintf("exception code: %w\n", regs.exception_code); } - kprintf("pc=%w:%x ds=%w es=%w fs=%w gs=%w\n", regs.cs, regs.eip, regs.ds, regs.es, regs.fs, regs.gs); - kprintf("stk=%w:%x\n", ss, esp); + kprintf(" pc=%w:%x ds=%w es=%w fs=%w gs=%w\n", regs.cs, regs.eip, regs.ds, regs.es, regs.fs, regs.gs); + kprintf(" stk=%w:%x\n", ss, esp); + if (current) + kprintf("kstk=%w:%x, base=%x, sigbase=%x\n", current->tss().ss0, current->tss().esp0, current->kernel_stack_base(), current->kernel_stack_for_signal_handler_base()); kprintf("eax=%x ebx=%x ecx=%x edx=%x\n", regs.eax, regs.ebx, regs.ecx, regs.edx); kprintf("ebp=%x esp=%x esi=%x edi=%x\n", regs.ebp, esp, regs.esi, regs.edi); diff --git a/SharedGraphics/Painter.cpp b/SharedGraphics/Painter.cpp index a736f142747..d94ab17d0ad 100644 --- a/SharedGraphics/Painter.cpp +++ b/SharedGraphics/Painter.cpp @@ -75,6 +75,8 @@ void Painter::fill_rect(const Rect& a_rect, Color color) if (rect.is_empty()) return; + ASSERT(m_target->rect().contains(rect)); + RGBA32* dst = m_target->scanline(rect.top()) + rect.left(); const unsigned dst_skip = m_target->width();