Skip to content

Commit

Permalink
Kernel+LibC: Make page fault crashes a bit more readable.
Browse files Browse the repository at this point in the history
We'll now try to detect crashes that were due to dereferencing nullptr,
uninitialized malloc() memory, or recently free()'d memory.
It's not perfect but I think it's pretty good. :^)

Also added some color to the most important parts of the crash log,
and added some more modes to /bin/crash for exercising this code.

Fixes SerenityOS#243.
  • Loading branch information
awesomekling committed Jun 19, 2019
1 parent 15bea71 commit 8c0ae71
Show file tree
Hide file tree
Showing 5 changed files with 76 additions and 11 deletions.
10 changes: 10 additions & 0 deletions AK/Types.h
Original file line number Diff line number Diff line change
Expand Up @@ -53,3 +53,13 @@ constexpr unsigned GB = KB * KB * KB;
namespace std {
typedef decltype(nullptr) nullptr_t;
}

static constexpr dword explode_byte(byte b)
{
return b << 24 | b << 16 | b << 8 | b;
}

static_assert(explode_byte(0xff) == 0xffffffff);
static_assert(explode_byte(0x80) == 0x80808080);
static_assert(explode_byte(0x7f) == 0x7f7f7f7f);
static_assert(explode_byte(0) == 0);
26 changes: 19 additions & 7 deletions Kernel/Arch/i386/CPU.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include <Kernel/Arch/i386/CPU.h>
#include <Kernel/KSyms.h>
#include <Kernel/VM/MemoryManager.h>
#include <LibC/mallocdefs.h>

//#define PAGE_FAULT_DEBUG

Expand Down Expand Up @@ -254,9 +255,9 @@ void exception_14_handler(RegisterDumpWithExceptionCode& regs)
{
ASSERT(current);

dword faultAddress;
dword fault_address;
asm("movl %%cr2, %%eax"
: "=a"(faultAddress));
: "=a"(fault_address));

dword fault_page_directory;
asm("movl %%cr3, %%eax"
Expand All @@ -270,22 +271,33 @@ void exception_14_handler(RegisterDumpWithExceptionCode& regs)
regs.exception_code & 1 ? "PV" : "NP",
fault_page_directory,
regs.exception_code & 2 ? "write" : "read",
faultAddress);
fault_address);
#endif

#ifdef PAGE_FAULT_DEBUG
dump(regs);
#endif

auto response = MM.handle_page_fault(PageFault(regs.exception_code, VirtualAddress(faultAddress)));
auto response = MM.handle_page_fault(PageFault(regs.exception_code, VirtualAddress(fault_address)));

if (response == PageFaultResponse::ShouldCrash) {
kprintf("%s(%u:%u) unrecoverable page fault, %s vaddr=%p\n",
dbgprintf("\033[31;1m%s(%u:%u) Unrecoverable page fault, %s address %p\033[0m\n",
current->process().name().characters(),
current->pid(),
current->tid(),
regs.exception_code & 2 ? "write" : "read",
faultAddress);
regs.exception_code & 2 ? "write to" : "read from",
fault_address);

dword malloc_scrub_pattern = explode_byte(MALLOC_SCRUB_BYTE);
dword free_scrub_pattern = explode_byte(FREE_SCRUB_BYTE);
if ((fault_address & 0xffff0000) == (malloc_scrub_pattern & 0xffff0000)) {
dbgprintf("\033[33;1mNote: Address %p looks like it may be uninitialized malloc() memory\033[0m\n", fault_address);
} else if ((fault_address & 0xffff0000) == (free_scrub_pattern & 0xffff0000)) {
dbgprintf("\033[33;1mNote: Address %p looks like it may be recently free()'d memory\033[0m\n", fault_address);
} else if (fault_address < 4096) {
dbgprintf("\033[33;1mNote: Address %p looks like a possible nullptr dereference\033[0m\n", fault_address);
}

dump(regs);
current->process().crash(SIGSEGV, regs.eip);
} else if (response == PageFaultResponse::Continue) {
Expand Down
3 changes: 1 addition & 2 deletions LibC/malloc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
#include <AK/InlineLinkedList.h>
#include <AK/Vector.h>
#include <assert.h>
#include <mallocdefs.h>
#include <serenity.h>
#include <stdio.h>
#include <stdlib.h>
Expand All @@ -12,8 +13,6 @@
//#define MALLOC_DEBUG
#define RECYCLE_BIG_ALLOCATIONS

#define MALLOC_SCRUB_BYTE 0x85
#define FREE_SCRUB_BYTE 0x82
#define MAGIC_PAGE_HEADER 0x42657274
#define MAGIC_BIGALLOC_HEADER 0x42697267
#define PAGE_ROUND_UP(x) ((((size_t)(x)) + PAGE_SIZE - 1) & (~(PAGE_SIZE - 1)))
Expand Down
5 changes: 5 additions & 0 deletions LibC/mallocdefs.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
#pragma once

#define MALLOC_SCRUB_BYTE 0xdc
#define FREE_SCRUB_BYTE 0xed

43 changes: 41 additions & 2 deletions Userland/crash.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,22 @@

static void print_usage_and_exit()
{
printf("usage: crash -[sdia]\n");
printf("usage: crash -[sdiamfMF]\n");
exit(0);
}

#pragma GCC optimize("O0")
int main(int argc, char** argv)
{
enum Mode {
SegmentationViolation,
DivisionByZero,
IllegalInstruction,
Abort
Abort,
WriteToUninitializedMallocMemory,
WriteToFreedMemory,
ReadFromUninitializedMallocMemory,
ReadFromFreedMemory,
};
Mode mode = SegmentationViolation;

Expand All @@ -29,6 +34,14 @@ int main(int argc, char** argv)
mode = IllegalInstruction;
else if (String(argv[1]) == "-a")
mode = Abort;
else if (String(argv[1]) == "-m")
mode = ReadFromUninitializedMallocMemory;
else if (String(argv[1]) == "-f")
mode = ReadFromFreedMemory;
else if (String(argv[1]) == "-M")
mode = WriteToUninitializedMallocMemory;
else if (String(argv[1]) == "-F")
mode = WriteToFreedMemory;
else
print_usage_and_exit();

Expand All @@ -55,6 +68,32 @@ int main(int argc, char** argv)
ASSERT_NOT_REACHED();
}

if (mode == ReadFromUninitializedMallocMemory) {
auto* uninitialized_memory = (volatile dword**)malloc(1024);
volatile auto x = uninitialized_memory[0][0];
ASSERT_NOT_REACHED();
}

if (mode == ReadFromFreedMemory) {
auto* uninitialized_memory = (volatile dword**)malloc(1024);
free(uninitialized_memory);
volatile auto x = uninitialized_memory[4][0];
ASSERT_NOT_REACHED();
}

if (mode == WriteToUninitializedMallocMemory) {
auto* uninitialized_memory = (volatile dword**)malloc(1024);
uninitialized_memory[4][0] = 1;
ASSERT_NOT_REACHED();
}

if (mode == WriteToFreedMemory) {
auto* uninitialized_memory = (volatile dword**)malloc(1024);
free(uninitialized_memory);
uninitialized_memory[4][0] = 1;
ASSERT_NOT_REACHED();
}

ASSERT_NOT_REACHED();
return 0;
}

0 comments on commit 8c0ae71

Please sign in to comment.