diff --git a/Kernel/Process.cpp b/Kernel/Process.cpp index 700e1bd954..614a5539c3 100644 --- a/Kernel/Process.cpp +++ b/Kernel/Process.cpp @@ -616,12 +616,18 @@ int Process::do_exec(String path, Vector arguments, Vector envir } } + // NOTE: We create the new stack before disabling interrupts since it will zero-fault + // and we don't want to deal with faults after this point. + u32 new_userspace_esp = main_thread().make_userspace_stack_for_main_thread(move(arguments), move(environment)); + // 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(). if (¤t->process() == this) cli(); + // NOTE: Be careful to not trigger any page faults below! + Scheduler::prepare_to_modify_tss(main_thread()); m_name = parts.take_last(); @@ -645,7 +651,7 @@ int Process::do_exec(String path, Vector arguments, Vector envir main_thread().m_tss.gs = thread_specific_selector() | 3; main_thread().m_tss.ss = 0x23; main_thread().m_tss.cr3 = page_directory().cr3(); - main_thread().make_userspace_stack_for_main_thread(move(arguments), move(environment)); + main_thread().m_tss.esp = new_userspace_esp; main_thread().m_tss.ss0 = 0x10; main_thread().m_tss.esp0 = old_esp0; main_thread().m_tss.ss2 = m_pid; diff --git a/Kernel/Thread.cpp b/Kernel/Thread.cpp index f0a6a37bd1..44f7daf171 100644 --- a/Kernel/Thread.cpp +++ b/Kernel/Thread.cpp @@ -579,13 +579,16 @@ RegisterDump& Thread::get_register_dump_from_stack() return *(RegisterDump*)(kernel_stack_top() - sizeof(RegisterDump)); } -void Thread::make_userspace_stack_for_main_thread(Vector arguments, Vector environment) +u32 Thread::make_userspace_stack_for_main_thread(Vector arguments, Vector environment) { auto* region = m_process.allocate_region(VirtualAddress(), default_userspace_stack_size, "Stack (Main thread)", PROT_READ | PROT_WRITE, false); ASSERT(region); region->set_stack(true); - m_tss.esp = region->vaddr().offset(default_userspace_stack_size).get(); + u32 new_esp = region->vaddr().offset(default_userspace_stack_size).get(); + + // FIXME: This is weird, we put the argument contents at the base of the stack, + // and the argument pointers at the top? Why? char* stack_base = (char*)region->vaddr().get(); int argc = arguments.size(); char** argv = (char**)stack_base; @@ -608,11 +611,19 @@ void Thread::make_userspace_stack_for_main_thread(Vector arguments, Vect } env[environment.size()] = nullptr; + auto push_on_new_stack = [&new_esp](u32 value) + { + new_esp -= 4; + u32* stack_ptr = (u32*)new_esp; + *stack_ptr = value; + }; + // NOTE: The stack needs to be 16-byte aligned. - push_value_on_stack((u32)env); - push_value_on_stack((u32)argv); - push_value_on_stack((u32)argc); - push_value_on_stack(0); + push_on_new_stack((u32)env); + push_on_new_stack((u32)argv); + push_on_new_stack((u32)argc); + push_on_new_stack(0); + return new_esp; } Thread* Thread::clone(Process& process) diff --git a/Kernel/Thread.h b/Kernel/Thread.h index de100a6ab1..796dbacaf3 100644 --- a/Kernel/Thread.h +++ b/Kernel/Thread.h @@ -351,7 +351,8 @@ public: void set_default_signal_dispositions(); void push_value_on_stack(u32); - void make_userspace_stack_for_main_thread(Vector arguments, Vector environment); + + u32 make_userspace_stack_for_main_thread(Vector arguments, Vector environment); void make_thread_specific_region(Badge);