From 4c6d990440cdba6c7dd294adb7e435770ffcbcc4 Mon Sep 17 00:00:00 2001 From: Minep Date: Thu, 3 Mar 2022 22:37:10 +0000 Subject: [PATCH] fix bugs found in brk & add simple security checks on lx_free --- lunaix-os/includes/lunaix/mm/kalloc.h | 12 +++-- lunaix-os/includes/lunaix/spike.h | 5 ++ lunaix-os/kernel/k_init.c | 2 +- lunaix-os/kernel/k_main.c | 13 +++++- lunaix-os/kernel/mm/dmm.c | 66 ++++++++++++++++----------- lunaix-os/kernel/spike.c | 2 +- lunaix-os/makefile | 4 +- 7 files changed, 68 insertions(+), 36 deletions(-) diff --git a/lunaix-os/includes/lunaix/mm/kalloc.h b/lunaix-os/includes/lunaix/mm/kalloc.h index eb309c5..d34b3be 100644 --- a/lunaix-os/includes/lunaix/mm/kalloc.h +++ b/lunaix-os/includes/lunaix/mm/kalloc.h @@ -7,8 +7,11 @@ int kalloc_init(); /** - * @brief Allocate a space in kernel heap.This is NOT the same as kmalloc in Linux! - * LunaixOS does NOT guarantee the continuity in physical pages. + * @brief Allocate an accessible memory region in kernel heap. + * + * @remarks + * This is NOT the same as kmalloc in Linux! + * LunaixOS does NOT guarantee the continuity in physical pages. * * @param size * @return void* @@ -18,15 +21,14 @@ kmalloc(size_t size); /** * @brief calloc for kernel heap. A wrapper for kmalloc - * * @param size - * @return void* + * @return void* */ void* kcalloc(size_t size); /** - * @brief free for kernel heap + * @brief Free the memory region allocated by kmalloc * * @param size * @return void* diff --git a/lunaix-os/includes/lunaix/spike.h b/lunaix-os/includes/lunaix/spike.h index 7e50693..0764822 100644 --- a/lunaix-os/includes/lunaix/spike.h +++ b/lunaix-os/includes/lunaix/spike.h @@ -24,6 +24,11 @@ inline static void spin() { if (!(cond)) { \ __assert_fail(#cond, __FILE__, __LINE__); \ } + +#define assert_msg(cond, msg) \ + if (!(cond)) { \ + __assert_fail(msg, __FILE__, __LINE__); \ + } void __assert_fail(const char* expr, const char* file, unsigned int line) __attribute__((noinline, noreturn)); #else #define assert(cond) //nothing diff --git a/lunaix-os/kernel/k_init.c b/lunaix-os/kernel/k_init.c index 5896ee4..444aab0 100644 --- a/lunaix-os/kernel/k_init.c +++ b/lunaix-os/kernel/k_init.c @@ -61,7 +61,7 @@ _kernel_post_init() { vmm_unmap_page((void*)(i << PG_SIZE_BITS)); } - assert(kalloc_init()); + assert_msg(kalloc_init(), "Fail to initialize heap"); } // 按照 Memory map 标识可用的物理页 diff --git a/lunaix-os/kernel/k_main.c b/lunaix-os/kernel/k_main.c index 52684d8..565e5bc 100644 --- a/lunaix-os/kernel/k_main.c +++ b/lunaix-os/kernel/k_main.c @@ -30,13 +30,24 @@ _kernel_main() arr[i] = (uint32_t*) kmalloc((i + 1) * 2); } - void* big_ = kmalloc(8192); for (size_t i = 0; i < 10; i++) { kfree(arr[i]); } + + void* big_ = kmalloc(8192); + // good free kfree(arr); kfree(big_); + + uint8_t* bad1 = kmalloc(123); + void* bad2 = kmalloc(1); + + *((uint32_t*)(bad1 - 4)) = 0xc2343312UL; + + // bad free + kfree(bad1); + kfree(bad2 - 2); } \ No newline at end of file diff --git a/lunaix-os/kernel/mm/dmm.c b/lunaix-os/kernel/mm/dmm.c index 31c28b6..02d094c 100644 --- a/lunaix-os/kernel/mm/dmm.c +++ b/lunaix-os/kernel/mm/dmm.c @@ -1,16 +1,20 @@ /** * @file dmm.c * @author Lunaixsky - * @brief Dynamic memory manager dedicated to kernel heap. Using implicit free list implementation. - * This is designed to be portable, so it can serve as syscalls to malloc/free in the c std lib. - * @version 0.1 - * @date 2022-02-28 + * @brief Dynamic memory manager dedicated to kernel heap. Using implicit free + * list implementation. This is designed to be portable, so it can serve as + * syscalls to malloc/free in the c std lib. + * + * This version of code is however the simplest and yet insecured, + * it just to demonstrate how the malloc/free works behind the stage + * + * @version 0.2 + * @date 2022-03-3 * * @copyright Copyright (c) Lunaixsky 2022 * */ - #include #include #include @@ -47,7 +51,8 @@ coalesce(uint8_t* chunk_ptr); void* lx_grow_heap(heap_context_t* heap, size_t sz); -void place_chunk(uint8_t* ptr, size_t size); +void +place_chunk(uint8_t* ptr, size_t size); int dmm_init(heap_context_t* heap) @@ -55,10 +60,10 @@ dmm_init(heap_context_t* heap) assert((uintptr_t)heap->start % BOUNDARY == 0); heap->brk = heap->start; - + vmm_alloc_page(heap->brk, PG_PREM_RW); - SW(heap->start, PACK(4, M_ALLOCATED)); + SW(heap->start, PACK(4, M_ALLOCATED)); SW(heap->start + WSIZE, PACK(0, M_ALLOCATED)); heap->brk += WSIZE; @@ -73,45 +78,44 @@ lxsbrk(heap_context_t* heap, void* addr) void* lxbrk(heap_context_t* heap, size_t size) -{ +{ if (size == 0) { return heap->brk; } - // plus WSIZE is the overhead for epilogue marker - size += WSIZE; - void* next = heap->brk + ROUNDUP((uintptr_t)size, WSIZE); + // The upper bound of our next brk of heap given the size. + // This will be used to calculate the page we need to allocate. + // The "+ WSIZE" capture the overhead for epilogue marker + void* next = heap->brk + ROUNDUP(size + WSIZE, WSIZE); if ((uintptr_t)next >= K_STACK_START) { return NULL; } - // Check the invariant - assert(size % BOUNDARY == 0) - uintptr_t heap_top_pg = PG_ALIGN(heap->brk); - if (heap_top_pg != PG_ALIGN(next)) - { + if (heap_top_pg != PG_ALIGN(next)) { // if next do require new pages to be allocated - if (!vmm_alloc_pages((void*)(heap_top_pg + PG_SIZE), ROUNDUP(size, PG_SIZE), PG_PREM_RW)) { + if (!vmm_alloc_pages((void*)(heap_top_pg + PG_SIZE), + ROUNDUP(size, PG_SIZE), + PG_PREM_RW)) { return NULL; } - } void* old = heap->brk; - heap->brk = next - WSIZE; + heap->brk += size; return old; } void* -lx_grow_heap(heap_context_t* heap, size_t sz) { +lx_grow_heap(heap_context_t* heap, size_t sz) +{ void* start; - sz = ROUNDUP(sz, BOUNDARY); if (!(start = lxbrk(heap, sz))) { return NULL; } + sz = ROUNDUP(sz, BOUNDARY); uint32_t old_marker = *((uint32_t*)start); uint32_t free_hdr = PACK(sz, CHUNK_PF(old_marker)); @@ -142,7 +146,8 @@ lx_malloc(heap_context_t* heap, size_t size) ptr += chunk_size; } - // if heap is full (seems to be!), then allocate more space (if it's okay...) + // if heap is full (seems to be!), then allocate more space (if it's + // okay...) if ((ptr = lx_grow_heap(heap, size))) { place_chunk(ptr, size); return BPTR(ptr); @@ -152,7 +157,9 @@ lx_malloc(heap_context_t* heap, size_t size) return NULL; } -void place_chunk(uint8_t* ptr, size_t size) { +void +place_chunk(uint8_t* ptr, size_t size) +{ uint32_t header = *((uint32_t*)ptr); size_t chunk_size = CHUNK_S(header); *((uint32_t*)ptr) = PACK(size, CHUNK_PF(header) | M_ALLOCATED); @@ -165,8 +172,7 @@ void place_chunk(uint8_t* ptr, size_t size) { SW(n_hdrptr, n_hdr & ~0x2); } else { // if there is remaining free space left - uint32_t remainder_hdr = - PACK(diff, M_NOT_ALLOCATED | M_PREV_ALLOCATED); + uint32_t remainder_hdr = PACK(diff, M_NOT_ALLOCATED | M_PREV_ALLOCATED); SW(n_hdrptr, remainder_hdr); SW(FPTR(n_hdrptr, diff), remainder_hdr); @@ -186,6 +192,14 @@ lx_free(void* ptr) size_t sz = CHUNK_S(hdr); uint8_t* next_hdr = chunk_ptr + sz; + // make sure the ptr we are 'bout to free makes sense + // the size trick comes from: + // https://sourceware.org/git/?p=glibc.git;a=blob;f=malloc/malloc.c;h=1a1ac1d8f05b6f9bf295d7fdd0f12c2e4650a33c;hb=HEAD#l4437 + assert_msg(((uintptr_t)ptr < (uintptr_t)(-sz)) && !((uintptr_t)ptr & ~0x3), + "free(): invalid pointer"); + assert_msg(sz > WSIZE && (sz & ~0x3), + "free(): invalid size"); + SW(chunk_ptr, hdr & ~M_ALLOCATED); SW(FPTR(chunk_ptr, sz), hdr & ~M_ALLOCATED); SW(next_hdr, LW(next_hdr) | M_PREV_FREE); diff --git a/lunaix-os/kernel/spike.c b/lunaix-os/kernel/spike.c index 91f597d..8f42660 100644 --- a/lunaix-os/kernel/spike.c +++ b/lunaix-os/kernel/spike.c @@ -5,7 +5,7 @@ static char buffer[1024]; void __assert_fail(const char* expr, const char* file, unsigned int line) { - sprintf(buffer, "Assert %s failed (%s:%u)", expr, file, line); + sprintf(buffer, "[ASSERT] %s (%s:%u)", expr, file, line); // Here we load the buffer's address into %edi ("D" constraint) // This is a convention we made that the LUNAIX_SYS_PANIC syscall will diff --git a/lunaix-os/makefile b/lunaix-os/makefile index 163f91f..4ed0d3b 100644 --- a/lunaix-os/makefile +++ b/lunaix-os/makefile @@ -43,8 +43,8 @@ all-debug: O := -Og all-debug: CFLAGS := -g -std=gnu99 -ffreestanding $(O) $(W) $(ARCH_OPT) -D__LUNAIXOS_DEBUG__ all-debug: LDFLAGS := -g -ffreestanding $(O) -nostdlib -lgcc all-debug: clean $(BUILD_DIR)/$(OS_ISO) - # @echo "Dumping the disassembled kernel code to $(BUILD_DIR)/kdump.txt" - # @i686-elf-objdump -D $(BIN_DIR)/$(OS_BIN) > $(BUILD_DIR)/kdump.txt + @echo "Dumping the disassembled kernel code to $(BUILD_DIR)/kdump.txt" + @i686-elf-objdump -S $(BIN_DIR)/$(OS_BIN) > $(BUILD_DIR)/kdump.txt clean: @rm -rf $(BUILD_DIR) -- 2.27.0