From 49259777efd6e1db22ee9ff6a89f373fa5f8b5d6 Mon Sep 17 00:00:00 2001 From: Luke Wilde Date: Thu, 7 Oct 2021 20:10:56 +0100 Subject: [PATCH] Kernel: Note if the page fault address is a destroyed smart pointer While I was working on LibWeb, I got a page fault at 0xe0e0e0e4. This indicates a destroyed RefPtr if compiled with SANITIZE_PTRS defined. However, the page fault handler didn't print out this indication. This makes the page fault handler print out a note if the faulting address looks like a recently destroyed RefPtr, OwnPtr, NonnullRefPtr, NonnullOwnPtr, ThreadSafeRefPtr or ThreadSafeNonnullRefPtr. It will only do this if SANITIZE_PTRS is defined, as smart pointers don't get scrubbed without it being defined. --- AK/NonnullOwnPtr.h | 4 ++- AK/NonnullRefPtr.h | 4 ++- AK/OwnPtr.h | 4 ++- AK/RefPtr.h | 4 ++- Kernel/Arch/x86/common/Interrupts.cpp | 33 +++++++++++++++++++----- Kernel/Library/ThreadSafeNonnullRefPtr.h | 4 ++- Kernel/Library/ThreadSafeRefPtr.h | 4 ++- 7 files changed, 45 insertions(+), 12 deletions(-) diff --git a/AK/NonnullOwnPtr.h b/AK/NonnullOwnPtr.h index 91ea1f15ce..96df3a26f8 100644 --- a/AK/NonnullOwnPtr.h +++ b/AK/NonnullOwnPtr.h @@ -13,6 +13,8 @@ #include #include +#define NONNULLOWNPTR_SCRUB_BYTE 0xf1 + namespace AK { template @@ -51,7 +53,7 @@ public: { clear(); #ifdef SANITIZE_PTRS - m_ptr = (T*)(explode_byte(0xe3)); + m_ptr = (T*)(explode_byte(NONNULLOWNPTR_SCRUB_BYTE)); #endif } diff --git a/AK/NonnullRefPtr.h b/AK/NonnullRefPtr.h index fb2632a66e..e97e5ce5e0 100644 --- a/AK/NonnullRefPtr.h +++ b/AK/NonnullRefPtr.h @@ -6,6 +6,8 @@ #pragma once +#define NONNULLREFPTR_SCRUB_BYTE 0xe1 + #ifdef KERNEL # include #else @@ -97,7 +99,7 @@ public: unref_if_not_null(m_ptr); m_ptr = nullptr; # ifdef SANITIZE_PTRS - m_ptr = reinterpret_cast(explode_byte(0xb0)); + m_ptr = reinterpret_cast(explode_byte(NONNULLREFPTR_SCRUB_BYTE)); # endif } diff --git a/AK/OwnPtr.h b/AK/OwnPtr.h index 0b69ce5d81..30e567aae1 100644 --- a/AK/OwnPtr.h +++ b/AK/OwnPtr.h @@ -12,6 +12,8 @@ # include #endif +#define OWNPTR_SCRUB_BYTE 0xf0 + namespace AK { template @@ -43,7 +45,7 @@ public: { clear(); #ifdef SANITIZE_PTRS - m_ptr = (T*)(explode_byte(0xe1)); + m_ptr = (T*)(explode_byte(OWNPTR_SCRUB_BYTE)); #endif } diff --git a/AK/RefPtr.h b/AK/RefPtr.h index 7076a2da1a..2df9109987 100644 --- a/AK/RefPtr.h +++ b/AK/RefPtr.h @@ -6,6 +6,8 @@ #pragma once +#define REFPTR_SCRUB_BYTE 0xe0 + #ifdef KERNEL # include #else @@ -100,7 +102,7 @@ public: { clear(); # ifdef SANITIZE_PTRS - m_ptr = reinterpret_cast(explode_byte(0xe0)); + m_ptr = reinterpret_cast(explode_byte(REFPTR_SCRUB_BYTE)); # endif } diff --git a/Kernel/Arch/x86/common/Interrupts.cpp b/Kernel/Arch/x86/common/Interrupts.cpp index 847dcbf236..5a4ae7c71f 100644 --- a/Kernel/Arch/x86/common/Interrupts.cpp +++ b/Kernel/Arch/x86/common/Interrupts.cpp @@ -357,12 +357,12 @@ void page_fault_handler(TrapFrame* trap) regs.exception_code & PageFaultFlags::InstructionFetch ? "instruction fetch / " : "", regs.exception_code & PageFaultFlags::Write ? "write to" : "read from", VirtualAddress(fault_address)); - FlatPtr malloc_scrub_pattern = explode_byte(MALLOC_SCRUB_BYTE); - FlatPtr free_scrub_pattern = explode_byte(FREE_SCRUB_BYTE); - FlatPtr kmalloc_scrub_pattern = explode_byte(KMALLOC_SCRUB_BYTE); - FlatPtr kfree_scrub_pattern = explode_byte(KFREE_SCRUB_BYTE); - FlatPtr slab_alloc_scrub_pattern = explode_byte(SLAB_ALLOC_SCRUB_BYTE); - FlatPtr slab_dealloc_scrub_pattern = explode_byte(SLAB_DEALLOC_SCRUB_BYTE); + constexpr FlatPtr malloc_scrub_pattern = explode_byte(MALLOC_SCRUB_BYTE); + constexpr FlatPtr free_scrub_pattern = explode_byte(FREE_SCRUB_BYTE); + constexpr FlatPtr kmalloc_scrub_pattern = explode_byte(KMALLOC_SCRUB_BYTE); + constexpr FlatPtr kfree_scrub_pattern = explode_byte(KFREE_SCRUB_BYTE); + constexpr FlatPtr slab_alloc_scrub_pattern = explode_byte(SLAB_ALLOC_SCRUB_BYTE); + constexpr FlatPtr slab_dealloc_scrub_pattern = explode_byte(SLAB_DEALLOC_SCRUB_BYTE); if ((fault_address & 0xffff0000) == (malloc_scrub_pattern & 0xffff0000)) { dbgln("Note: Address {} looks like it may be uninitialized malloc() memory", VirtualAddress(fault_address)); } else if ((fault_address & 0xffff0000) == (free_scrub_pattern & 0xffff0000)) { @@ -377,6 +377,27 @@ void page_fault_handler(TrapFrame* trap) dbgln("Note: Address {} looks like it may be recently slab_dealloc()'d memory", VirtualAddress(fault_address)); } else if (fault_address < 4096) { dbgln("Note: Address {} looks like a possible nullptr dereference", VirtualAddress(fault_address)); + } else if constexpr (SANITIZE_PTRS) { + constexpr FlatPtr refptr_scrub_pattern = explode_byte(REFPTR_SCRUB_BYTE); + constexpr FlatPtr nonnullrefptr_scrub_pattern = explode_byte(NONNULLREFPTR_SCRUB_BYTE); + constexpr FlatPtr ownptr_scrub_pattern = explode_byte(OWNPTR_SCRUB_BYTE); + constexpr FlatPtr nonnullownptr_scrub_pattern = explode_byte(NONNULLOWNPTR_SCRUB_BYTE); + constexpr FlatPtr threadsaferefptr_scrub_pattern = explode_byte(THREADSAFEREFPTR_SCRUB_BYTE); + constexpr FlatPtr threadsafenonnullrefptr_scrub_pattern = explode_byte(THREADSAFENONNULLREFPTR_SCRUB_BYTE); + + if ((fault_address & 0xffff0000) == (refptr_scrub_pattern & 0xffff0000)) { + dbgln("Note: Address {} looks like it may be a recently destroyed RefPtr", VirtualAddress(fault_address)); + } else if ((fault_address & 0xffff0000) == (nonnullrefptr_scrub_pattern & 0xffff0000)) { + dbgln("Note: Address {} looks like it may be a recently destroyed NonnullRefPtr", VirtualAddress(fault_address)); + } else if ((fault_address & 0xffff0000) == (ownptr_scrub_pattern & 0xffff0000)) { + dbgln("Note: Address {} looks like it may be a recently destroyed OwnPtr", VirtualAddress(fault_address)); + } else if ((fault_address & 0xffff0000) == (nonnullownptr_scrub_pattern & 0xffff0000)) { + dbgln("Note: Address {} looks like it may be a recently destroyed NonnullOwnPtr", VirtualAddress(fault_address)); + } else if ((fault_address & 0xffff0000) == (threadsaferefptr_scrub_pattern & 0xffff0000)) { + dbgln("Note: Address {} looks like it may be a recently destroyed ThreadSafeRefPtr", VirtualAddress(fault_address)); + } else if ((fault_address & 0xffff0000) == (threadsafenonnullrefptr_scrub_pattern & 0xffff0000)) { + dbgln("Note: Address {} looks like it may be a recently destroyed ThreadSafeNonnullRefPtr", VirtualAddress(fault_address)); + } } if (current_thread) { diff --git a/Kernel/Library/ThreadSafeNonnullRefPtr.h b/Kernel/Library/ThreadSafeNonnullRefPtr.h index 094d8bba44..523fdbec42 100644 --- a/Kernel/Library/ThreadSafeNonnullRefPtr.h +++ b/Kernel/Library/ThreadSafeNonnullRefPtr.h @@ -16,6 +16,8 @@ # include #endif +#define THREADSAFENONNULLREFPTR_SCRUB_BYTE 0xa1 + namespace AK { template @@ -95,7 +97,7 @@ public: { assign(nullptr); #ifdef SANITIZE_PTRS - m_bits.store(explode_byte(0xb0), AK::MemoryOrder::memory_order_relaxed); + m_bits.store(explode_byte(THREADSAFENONNULLREFPTR_SCRUB_BYTE), AK::MemoryOrder::memory_order_relaxed); #endif } diff --git a/Kernel/Library/ThreadSafeRefPtr.h b/Kernel/Library/ThreadSafeRefPtr.h index e8fa9fc5ec..03b7d2e11e 100644 --- a/Kernel/Library/ThreadSafeRefPtr.h +++ b/Kernel/Library/ThreadSafeRefPtr.h @@ -19,6 +19,8 @@ # include #endif +#define THREADSAFEREFPTR_SCRUB_BYTE 0xa0 + namespace AK { template @@ -182,7 +184,7 @@ public: { clear(); #ifdef SANITIZE_PTRS - m_bits.store(explode_byte(0xe0), AK::MemoryOrder::memory_order_relaxed); + m_bits.store(explode_byte(THREADSAFEREFPTR_SCRUB_BYTE), AK::MemoryOrder::memory_order_relaxed); #endif }