Kernel+CrashReporter: Add metadata about page faults to crash reports

Crash reports for page faults now tell you what kind of memory access failed and where. :^)
AmusedNetwork · Apr 4, 2021 · 0b82268 · 0b82268
1 parent e238435
commit 0b82268
Show file tree

Hide file tree

Showing 4 changed files with 27 additions and 2 deletions.
diff --git a/Kernel/Arch/i386/CPU.cpp b/Kernel/Arch/i386/CPU.cpp
@@ -284,7 +284,8 @@ void page_fault_handler(TrapFrame* trap)
  PANIC("Attempt to access UNMAP_AFTER_INIT section");
  }
 
- auto response = MM.handle_page_fault(PageFault(regs.exception_code, VirtualAddress(fault_address)));
+ PageFault fault { regs.exception_code, VirtualAddress { fault_address } };
+ auto response = MM.handle_page_fault(fault);
 
  if (response == PageFaultResponse::ShouldCrash || response == PageFaultResponse::OutOfMemory) {
  if (faulted_in_kernel && handle_safe_access_fault(regs, fault_address)) {
@@ -328,6 +329,18 @@ void page_fault_handler(TrapFrame* trap)
  dbgln("Note: Address {} looks like a possible nullptr dereference", VirtualAddress(fault_address));
  }
 
+ auto& current_process = current_thread->process();
+ if (current_process.is_user_process()) {
+ current_process.set_coredump_metadata("fault_address", String::formatted("{:p}", fault_address));
+ current_process.set_coredump_metadata("fault_type", fault.type() == PageFault::Type::PageNotPresent ? "NotPresent" : "ProtectionViolation");
+ String fault_access;
+ if (fault.is_instruction_fetch())
+ fault_access = "Execute";
+ else
+ fault_access = fault.access() == PageFault::Access::Read ? "Read" : "Write";
+ current_process.set_coredump_metadata("fault_access", fault_access);
+ }
+
  handle_crash(regs, "Page Fault", SIGSEGV, response == PageFaultResponse::OutOfMemory);
  } else if (response == PageFaultResponse::Continue) {
 #if PAGE_FAULT_DEBUG
@@ -2397,7 +2410,6 @@ void copy_ptrace_registers_into_kernel_registers(RegisterState& kernel_regs, con
  kernel_regs.eip = ptrace_regs.eip;
  kernel_regs.eflags = (kernel_regs.eflags & ~safe_eflags_mask) | (ptrace_regs.eflags & safe_eflags_mask);
 }
-
 }
 
 #ifdef DEBUG

diff --git a/Kernel/Process.cpp b/Kernel/Process.cpp
@@ -727,4 +727,9 @@ void Process::set_dumpable(bool dumpable)
  m_dumpable = dumpable;
 }
 
+void Process::set_coredump_metadata(const String& key, String value)
+{
+ m_coredump_metadata.set(key, move(value));
+}
+
 }
diff --git a/Kernel/Process.h b/Kernel/Process.h
@@ -494,6 +494,7 @@ class Process
 
  HashMap<String, String>& coredump_metadata() { return m_coredump_metadata; }
  const HashMap<String, String>& coredump_metadata() const { return m_coredump_metadata; }
+ void set_coredump_metadata(const String& key, String value);
 
  const NonnullRefPtrVector<Thread>& threads_for_coredump(Badge<CoreDump>) const { return m_threads_for_coredump; }
 

diff --git a/Userland/Applications/CrashReporter/main.cpp b/Userland/Applications/CrashReporter/main.cpp
@@ -79,6 +79,13 @@ static TitleAndText build_backtrace(const CoreDump::Reader& coredump, const ELF:
  else if (metadata.contains("pledge_violation"))
  prepend_metadata("pledge_violation", "Has not pledged {}");
 
+ auto fault_address = metadata.get("fault_address");
+ auto fault_type = metadata.get("fault_type");
+ auto fault_access = metadata.get("fault_access");
+ if (fault_address.has_value() && fault_type.has_value() && fault_access.has_value()) {
+ builder.appendff("{} fault on {} at address {}\n\n", fault_type.value(), fault_access.value(), fault_address.value());
+ }
+
  auto first_entry = true;
  for (auto& entry : backtrace.entries()) {
  if (first_entry)