From 00ea1bdfe3d0758dc42942c5994664c51badf2f8 Mon Sep 17 00:00:00 2001 From: Alex Auvolat Date: Mon, 9 Mar 2015 17:27:27 +0100 Subject: Remove kernel region pf handlers ; fix forget-to-zero-out bug. --- src/kernel/core/kmalloc.c | 2 +- src/kernel/core/paging.c | 33 +++++++++++++++--------------- src/kernel/core/region.c | 10 ++++----- src/kernel/core/thread.c | 2 +- src/kernel/include/region.h | 8 +------- src/kernel/user/pager.c | 8 ++++---- src/kernel/user/process.c | 10 +++++++-- src/tests/ktests/region1/test.c | 8 ++++---- src/tests/ktests/region2/Makefile | 1 - src/tests/ktests/region2/test.c | 43 --------------------------------------- 10 files changed, 39 insertions(+), 86 deletions(-) delete mode 120000 src/tests/ktests/region2/Makefile delete mode 100644 src/tests/ktests/region2/test.c (limited to 'src') diff --git a/src/kernel/core/kmalloc.c b/src/kernel/core/kmalloc.c index 85e6173..c977e54 100644 --- a/src/kernel/core/kmalloc.c +++ b/src/kernel/core/kmalloc.c @@ -9,7 +9,7 @@ #include static void* page_alloc_fun_for_kmalloc(size_t bytes) { - void* addr = region_alloc(bytes, "Core kernel heap", 0); + void* addr = region_alloc(bytes, "Core kernel heap"); if (addr == 0) return 0; // Map physical memory diff --git a/src/kernel/core/paging.c b/src/kernel/core/paging.c index f2289bd..eec21f6 100644 --- a/src/kernel/core/paging.c +++ b/src/kernel/core/paging.c @@ -8,6 +8,8 @@ #include #include +#include + #define PAGE_OF_ADDR(x) (((size_t)(x) >> PAGE_SHIFT) % N_PAGES_IN_PT) #define PT_OF_ADDR(x) ((size_t)(x) >> (PAGE_SHIFT + PT_SHIFT)) @@ -103,18 +105,13 @@ void page_fault_handler(registers_t *regs) { } region_info_t *i = find_region(vaddr); - if (i == 0) { - dbg_printf("Kernel pagefault in non-existing region at 0x%p\n", vaddr); - dbg_dump_registers(regs); - PANIC("Unhandled kernel space page fault"); - } - if (i->pf == 0) { - dbg_printf("Kernel pagefault in region with no handler at 0x%p (%s region)\n", vaddr, i->type); - dbg_dump_registers(regs); - dbg_print_region_info(); - PANIC("Unhandled kernel space page fault"); - } - i->pf(get_current_pagedir(), i, vaddr); + + char* region = (i == 0 ? "non-exting" : i->type); + + dbg_printf("Kernel pagefault in region with no handler at 0x%p (%s region)\n", vaddr, region); + dbg_dump_registers(regs); + dbg_print_region_info(); + PANIC("Unhandled kernel space page fault"); } } } @@ -203,6 +200,7 @@ bool pd_map_page(void* vaddr, uint32_t frame_id, bool rw) { const uint32_t page = PAGE_OF_ADDR(vaddr); ASSERT((size_t)vaddr < PD_MIRROR_ADDR); + ASSERT(frame_id != 0); bool on_kernel_pd = (size_t)vaddr >= K_HIGHHALF_ADDR || current_thread == 0; @@ -212,7 +210,6 @@ bool pd_map_page(void* vaddr, uint32_t frame_id, bool rw) { mutex_lock(&pdd->mutex); if (!(pd->page[pt] & PTE_PRESENT)) { - uint32_t new_pt_frame; int tries = 0; while ((new_pt_frame = frame_alloc(1)) == 0 && (tries++) < 3) { @@ -226,13 +223,15 @@ bool pd_map_page(void* vaddr, uint32_t frame_id, bool rw) { } current_pd->page[pt] = pd->page[pt] = - (new_pt_frame << PTE_FRAME_SHIFT) | PTE_PRESENT | PTE_RW + (new_pt_frame << PTE_FRAME_SHIFT) + | PTE_PRESENT | PTE_RW | ((size_t)vaddr < K_HIGHHALF_ADDR ? PTE_USER : 0); + invlpg(¤t_pt[pt]); + memset(¤t_pt[pt], 0, PAGE_SIZE); } current_pt[pt].page[page] = - (frame_id << PTE_FRAME_SHIFT) - | PTE_PRESENT + (frame_id << PTE_FRAME_SHIFT) | PTE_PRESENT | ((size_t)vaddr < K_HIGHHALF_ADDR ? PTE_USER : PTE_GLOBAL) | (rw ? PTE_RW : 0); invlpg(vaddr); @@ -273,7 +272,7 @@ pagedir_t *create_pagedir(user_pf_handler_t pf, void* pfd) { pd = (pagedir_t*)malloc(sizeof(pagedir_t)); if (pd == 0) goto error; - temp = region_alloc(PAGE_SIZE, "Temporary pagedir mapping", 0); + temp = region_alloc(PAGE_SIZE, "Temporary pagedir mapping"); if (temp == 0) goto error; bool map_ok = pd_map_page(temp, pd_phys, true); diff --git a/src/kernel/core/region.c b/src/kernel/core/region.c index d22a98f..1f1958f 100644 --- a/src/kernel/core/region.c +++ b/src/kernel/core/region.c @@ -216,7 +216,6 @@ void region_allocator_init(void* kernel_data_end) { u0->used.i.addr = (void*)K_HIGHHALF_ADDR; u0->used.i.size = PAGE_ALIGN_UP(kernel_data_end) - K_HIGHHALF_ADDR; u0->used.i.type = "Kernel code & data"; - u0->used.i.pf = 0; u0->used.next_by_addr = 0; first_used_region = u0; } @@ -238,7 +237,7 @@ void region_free(void* addr) { mutex_unlock(&ra_mutex); } -static void* region_alloc_inner(size_t size, char* type, kernel_pf_handler_t pf, bool use_reserve) { +static void* region_alloc_inner(size_t size, char* type, bool use_reserve) { size = PAGE_ALIGN_UP(size); for (descriptor_t *i = first_free_region_by_size; i != 0; i = i->free.first_bigger) { @@ -273,7 +272,6 @@ static void* region_alloc_inner(size_t size, char* type, kernel_pf_handler_t pf, i->used.i.addr = addr; i->used.i.size = size; i->used.i.type = type; - i->used.i.pf = pf; add_used_region(i); return addr; @@ -282,7 +280,7 @@ static void* region_alloc_inner(size_t size, char* type, kernel_pf_handler_t pf, return 0; //No big enough block found } -void* region_alloc(size_t size, char* type, kernel_pf_handler_t pf) { +void* region_alloc(size_t size, char* type) { void* result = 0; mutex_lock(&ra_mutex); @@ -290,7 +288,7 @@ void* region_alloc(size_t size, char* type, kernel_pf_handler_t pf) { uint32_t frame = frame_alloc(1); if (frame == 0) goto try_anyway; - void* descriptor_region = region_alloc_inner(PAGE_SIZE, "Region descriptors", 0, true); + void* descriptor_region = region_alloc_inner(PAGE_SIZE, "Region descriptors", true); ASSERT(descriptor_region != 0); bool map_ok = pd_map_page(descriptor_region, frame, 1); @@ -312,7 +310,7 @@ void* region_alloc(size_t size, char* type, kernel_pf_handler_t pf) { // even if we don't have enough unused descriptors, we might find // a free region that has exactly the right size and therefore // does not require splitting, so we try the allocation in all cases - result = region_alloc_inner(size, type, pf, false); + result = region_alloc_inner(size, type, false); mutex_unlock(&ra_mutex); return result; diff --git a/src/kernel/core/thread.c b/src/kernel/core/thread.c index 74e5a30..1689433 100644 --- a/src/kernel/core/thread.c +++ b/src/kernel/core/thread.c @@ -157,7 +157,7 @@ thread_t *new_thread(entry_t entry, void* data) { thread_t *t = (thread_t*)malloc(sizeof(thread_t)); if (t == 0) return 0; - void* stack = region_alloc(KPROC_STACK_SIZE + PAGE_SIZE, "Stack", 0); + void* stack = region_alloc(KPROC_STACK_SIZE + PAGE_SIZE, "Stack"); if (stack == 0) { free(t); return 0; diff --git a/src/kernel/include/region.h b/src/kernel/include/region.h index eb6f7e8..2593dba 100644 --- a/src/kernel/include/region.h +++ b/src/kernel/include/region.h @@ -8,25 +8,19 @@ #include struct region_info; -typedef void (*kernel_pf_handler_t)(pagedir_t *pd, struct region_info *r, void* addr); typedef struct region_info { void* addr; size_t size; char* type; - kernel_pf_handler_t pf; } region_info_t; void region_allocator_init(void* kernel_data_end); -void* region_alloc(size_t size, char* type, kernel_pf_handler_t pf); // returns 0 on error +void* region_alloc(size_t size, char* type); // returns 0 on error region_info_t *find_region(void* addr); void region_free(void* addr); -// some usefull PF handlers -// void pf_handler_unexpected(pagedir_t *pd, struct region_info *r, void* addr); // Expects never to be called -// void pf_handler_stackoverflow(pagedir_t *pd, struct region_info *r, void* addr); // Stack overflow detected - // some functions for freeing regions and frames // region_free_unmap_free : deletes a region and frees all frames that were mapped in it void region_free_unmap_free(void* addr); diff --git a/src/kernel/user/pager.c b/src/kernel/user/pager.c index b1ce9c3..45d2970 100644 --- a/src/kernel/user/pager.c +++ b/src/kernel/user/pager.c @@ -47,7 +47,7 @@ error: void swap_page_in(pager_t *p, size_t offset, size_t len) { ASSERT(PAGE_ALIGNED(offset)); - void *region = region_alloc(PAGE_SIZE, "Page zeroing area", 0); + void *region = region_alloc(PAGE_SIZE, "Page zeroing area"); if (region == 0) return; for (size_t page = offset; page < offset + len; page += PAGE_SIZE) { @@ -92,7 +92,7 @@ bool swap_pager_resize(pager_t *p, size_t new_size) { size_t last_page = PAGE_ALIGN_DOWN(new_size); if (!PAGE_ALIGNED(new_size) && hashtbl_find(p->pages, (void*)last_page) != 0) { - void *region = region_alloc(PAGE_SIZE, "Page zeroing area", 0); + void *region = region_alloc(PAGE_SIZE, "Page zeroing area"); if (!region) PANIC("TODO"); uint32_t frame = (uint32_t)hashtbl_find(p->pages, (void*)last_page) >> ENT_FRAME_SHIFT; @@ -154,7 +154,7 @@ void vfs_page_in(pager_t *p, size_t offset, size_t len) { ASSERT(PAGE_ALIGNED(offset)); ASSERT(p->vfs_pager.ops->read != 0); - void *region = region_alloc(PAGE_SIZE, "Page loading area", 0); + void *region = region_alloc(PAGE_SIZE, "Page loading area"); if (region == 0) return; for (size_t page = offset; page < offset + len; page += PAGE_SIZE) { @@ -332,7 +332,7 @@ size_t pager_do_rw(pager_t *p, size_t offset, size_t len, char* buf, bool write) size_t first_page = PAGE_ALIGN_DOWN(offset); size_t region_len = offset + len - first_page; - region = region_alloc(PAGE_ALIGN_UP(region_len), "Temporary pager read/write zone", 0); + region = region_alloc(PAGE_ALIGN_UP(region_len), "Temporary pager read/write zone"); if (region == 0) goto end_read; p->ops->page_in(p, first_page, region_len); diff --git a/src/kernel/user/process.c b/src/kernel/user/process.c index d8f1eed..4d79327 100644 --- a/src/kernel/user/process.c +++ b/src/kernel/user/process.c @@ -682,8 +682,14 @@ void proc_usermem_pf(void* p, registers_t *regs, void* addr) { current_process_exit(PS_FAILURE, (addr < (void*)PAGE_SIZE ? FAIL_ZEROPTR : FAIL_SEGFAULT)); } - bool pr = ((regs->err_code & PF_PRESENT_BIT) != 0); - ASSERT(!pr); + bool pr = (regs->err_code & PF_PRESENT_BIT) != 0; + if (pr) { + uint32_t frame = pd_get_entry(addr); + dbg_printf("UPF 0x%p %s present %d 0x%p\n", + addr, (wr ? "write" : "read"), + frame >> PTE_FRAME_SHIFT, frame); + PANIC("Unexpected fault on present page."); + } addr = (void*)((uint32_t)addr & PAGE_MASK); diff --git a/src/tests/ktests/region1/test.c b/src/tests/ktests/region1/test.c index 1a42638..58b4933 100644 --- a/src/tests/ktests/region1/test.c +++ b/src/tests/ktests/region1/test.c @@ -2,16 +2,16 @@ void test_region_1() { BEGIN_TEST("region-test-1"); - void* p = region_alloc(0x1000, "Test region", 0); + void* p = region_alloc(0x1000, "Test region"); dbg_printf("Allocated one-page region: 0x%p\n", p); dbg_print_region_info(); - void* q = region_alloc(0x1000, "Test region", 0); + void* q = region_alloc(0x1000, "Test region"); dbg_printf("Allocated one-page region: 0x%p\n", q); dbg_print_region_info(); - void* r = region_alloc(0x2000, "Test region", 0); + void* r = region_alloc(0x2000, "Test region"); dbg_printf("Allocated two-page region: 0x%p\n", r); dbg_print_region_info(); - void* s = region_alloc(0x10000, "Test region", 0); + void* s = region_alloc(0x10000, "Test region"); dbg_printf("Allocated 16-page region: 0x%p\n", s); dbg_print_region_info(); region_free(p); diff --git a/src/tests/ktests/region2/Makefile b/src/tests/ktests/region2/Makefile deleted file mode 120000 index 4630a7c..0000000 --- a/src/tests/ktests/region2/Makefile +++ /dev/null @@ -1 +0,0 @@ -../rules.make \ No newline at end of file diff --git a/src/tests/ktests/region2/test.c b/src/tests/ktests/region2/test.c deleted file mode 100644 index 232bd12..0000000 --- a/src/tests/ktests/region2/test.c +++ /dev/null @@ -1,43 +0,0 @@ - -void pf_allocate_on_demand(pagedir_t *pd, struct region_info *r, void* addr) { - ASSERT(pd_get_frame(addr) == 0); // if error is of another type (RO, protected), we don't do anyting - - uint32_t f = frame_alloc(1); - if (f == 0) PANIC("Out Of Memory"); - bool map_ok = pd_map_page(addr, f, 1); - if (!map_ok) PANIC("Could not map frame (OOM)"); -} - -void test_region_2() { - BEGIN_TEST("region-test-2"); - - // allocate a big region and try to write into it - const size_t n = 200; - void* p0 = region_alloc(n * PAGE_SIZE, "Test big region", pf_allocate_on_demand); - for (size_t i = 0; i < n; i++) { - uint32_t *x = (uint32_t*)(p0 + i * PAGE_SIZE); - x[0] = 12; - x[1] = (i * 20422) % 122; - } - // unmap memory - for (size_t i = 0; i < n; i++) { - void* p = p0 + i * PAGE_SIZE; - uint32_t *x = (uint32_t*)p; - ASSERT(x[1] == (i * 20422) % 122); - - uint32_t f = pd_get_frame(p); - ASSERT(f != 0); - pd_unmap_page(p); - ASSERT(pd_get_frame(p) == 0); - - frame_free(f, 1); - } - region_free(p0); - - TEST_OK; -} - -#undef TEST_PLACEHOLDER_AFTER_REGION -#define TEST_PLACEHOLDER_AFTER_REGION { test_region_2(); } - -/* vim: set ts=4 sw=4 tw=0 noet :*/ -- cgit v1.2.3