RGBCube/serenity
1
Fork 0
mirror of https://github.com/RGBCube/serenity synced 2025-07-25 04:17:35 +00:00
Code Issues Activity Actions

Kernel: Don't let signals unblock threads while handling a page fault

Browse source 
It was possible to signal a process while it was paging in an inode
backed VM object. This would cause the inode read to EINTR, and the
page fault handler would assert.

Solve this by simply not unblocking threads due to signals if they are
currently busy handling a page fault. This is probably not the best way
to solve this issue, so I've added a FIXME to that effect.
This commit is contained in:
Andreas Kling 2021-01-21 00:06:19 +01:00
parent 1f53dd0943
commit 928ee2c791
3 changed files with 21 additions and 0 deletions
Download patch file Download diff file Expand all files Collapse all files

9
Kernel/Arch/i386/CPU.cpp
View file

@ -25,6 +25,7 @@
*/
#include <AK/Assertions.h>
#include <AK/ScopeGuard.h>
#include <AK/String.h>
#include <AK/StringBuilder.h>
#include <AK/Types.h>
@ -257,6 +258,14 @@ void page_fault_handler(TrapFrame* trap)
}
auto current_thread = Thread::current();
if (current_thread)
current_thread->set_handling_page_fault(true);
ScopeGuard guard = [current_thread] {
if (current_thread)
current_thread->set_handling_page_fault(false);
};
if (!faulted_in_kernel && !MM.validate_user_stack(current_thread->process(), VirtualAddress(regs.userspace_esp))) {
dbgln("Invalid stack pointer: {}", VirtualAddress(regs.userspace_esp));
handle_crash(regs, "Bad stack on page fault", SIGSTKFLT);

8
Kernel/Thread.cpp
View file

@ -166,6 +166,12 @@ void Thread::unblock(u8 signal)
return;
ASSERT(m_blocker);
if (signal != 0) {
if (is_handling_page_fault()) {
// Don't let signals unblock threads that are blocked inside a page fault handler.
// This prevents threads from EINTR'ing the inode read in an inode page fault.
// FIXME: There's probably a better way to solve this.
return;
}
if (!m_blocker->can_be_interrupted() && !m_should_die)
return;
m_blocker->set_interrupted_by_signal(signal);
@ -461,6 +467,8 @@ u32 Thread::pending_signals_for_state() const
{
ASSERT(g_scheduler_lock.own_lock());
constexpr u32 stopped_signal_mask = (1 << (SIGCONT - 1)) | (1 << (SIGKILL - 1)) | (1 << (SIGTRAP - 1));
if (is_handling_page_fault())
return 0;
return m_state != Stopped ? m_pending_signals : m_pending_signals & stopped_signal_mask;
}

4
Kernel/Thread.h
View file

@ -1155,6 +1155,9 @@ public:
return m_kernel_stack_region;
}
bool is_handling_page_fault() const { return m_handling_page_fault; }
void set_handling_page_fault(bool b) { m_handling_page_fault = b; }
private:
IntrusiveListNode m_runnable_list_node;
@ -1258,6 +1261,7 @@ private:
JoinBlockCondition m_join_condition;
Atomic<bool> m_is_active { false };
bool m_is_joinable { true };
bool m_handling_page_fault { false };
unsigned m_syscall_count { 0 };
unsigned m_inode_faults { 0 };