From 2263b4617434f333b33d049ce0025ff09cc58a81 Mon Sep 17 00:00:00 2001 From: Tulio A M Mendes Date: Fri, 17 Apr 2026 01:09:18 -0300 Subject: [PATCH] =?utf8?q?fix(mm):=20munmap/brk=20page=20leaks=20=E2=80=94?= =?utf8?q?=20free=20physical=20frames=20on=20unmap,=20rollback=20on=20part?= =?utf8?q?ial=20failure?= MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit 1. munmap: vmm_unmap_page only clears PTEs without freeing physical frames. For anonymous mmaps (shmid == -1), call vmm_virt_to_phys + pmm_free_page before vmm_unmap_page. Device-backed/shared mappings keep their frames (managed by their own subsystems). 2. brk shrink: same leak — free physical frames before unmapping when the heap shrinks. 3. brk grow partial failure: if pmm_alloc_page fails mid-expansion, rollback all pages already mapped in this call (unmap + free), then return old heap_break. Previously these pages were leaked permanently. 4. mmap anonymous partial failure: same rollback pattern applied to syscall_mmap_impl — if pmm_alloc_page fails mid-allocation, unmap and free all pages already mapped before returning -ENOMEM. All tests pass: 69/69 host + 103/103 QEMU, zero regressions. --- src/kernel/syscall.c | 35 +++++++++++++++++++++++++++++++++-- 1 file changed, 33 insertions(+), 2 deletions(-) diff --git a/src/kernel/syscall.c b/src/kernel/syscall.c index 410bda84..14c9a046 100644 --- a/src/kernel/syscall.c +++ b/src/kernel/syscall.c @@ -3002,11 +3002,22 @@ static uintptr_t syscall_mmap_impl(uintptr_t addr, uint32_t length, uint32_t pro uint32_t vmm_flags = VMM_FLAG_PRESENT | VMM_FLAG_USER; if (prot & PROT_WRITE) vmm_flags |= VMM_FLAG_RW; + uintptr_t mapped_va[1024]; + int mapped_count = 0; for (uintptr_t va = base; va < base + aligned_len; va += 0x1000U) { void* frame = pmm_alloc_page(); - if (!frame) return (uintptr_t)-ENOMEM; + if (!frame) { + /* Rollback: unmap and free pages already mapped in this call */ + for (int j = 0; j < mapped_count; j++) { + uintptr_t phys = vmm_virt_to_phys((uint64_t)mapped_va[j]); + vmm_unmap_page((uint64_t)mapped_va[j]); + if (phys) pmm_free_page((void*)phys); + } + return (uintptr_t)-ENOMEM; + } vmm_map_page((uint64_t)(uintptr_t)frame, (uint64_t)va, vmm_flags); memset((void*)va, 0, 0x1000U); + if (mapped_count < 1024) mapped_va[mapped_count++] = va; } } @@ -3034,8 +3045,17 @@ static int syscall_munmap_impl(uintptr_t addr, uint32_t length) { } if (found < 0) return -EINVAL; + /* Free physical frames for anonymous mappings before unmapping. + * Device-backed or shared-memory mappings manage their own frames. */ + int is_anon = (current_process->mmaps[found].shmid == -1); for (uintptr_t va = addr; va < addr + aligned_len; va += 0x1000U) { - vmm_unmap_page((uint64_t)va); + if (is_anon) { + uintptr_t phys = vmm_virt_to_phys((uint64_t)va); + vmm_unmap_page((uint64_t)va); + if (phys) pmm_free_page((void*)phys); + } else { + vmm_unmap_page((uint64_t)va); + } } current_process->mmaps[found].base = 0; @@ -3063,19 +3083,30 @@ static uintptr_t syscall_brk_impl(uintptr_t addr) { uintptr_t old_brk_page = (old_brk + 0xFFFU) & ~(uintptr_t)0xFFFU; if (new_brk > old_brk_page) { + uintptr_t mapped_va[1024]; + int mapped_count = 0; for (uintptr_t va = old_brk_page; va < new_brk; va += 0x1000U) { void* frame = pmm_alloc_page(); if (!frame) { + /* Rollback: unmap and free pages already mapped in this call */ + for (int j = 0; j < mapped_count; j++) { + uintptr_t phys = vmm_virt_to_phys((uint64_t)mapped_va[j]); + vmm_unmap_page((uint64_t)mapped_va[j]); + if (phys) pmm_free_page((void*)phys); + } return current_process->heap_break; } vmm_as_map_page(current_process->addr_space, (uint64_t)(uintptr_t)frame, (uint64_t)va, VMM_FLAG_PRESENT | VMM_FLAG_RW | VMM_FLAG_USER); memset((void*)va, 0, 0x1000U); + if (mapped_count < 1024) mapped_va[mapped_count++] = va; } } else if (new_brk < old_brk_page) { for (uintptr_t va = new_brk; va < old_brk_page; va += 0x1000U) { + uintptr_t phys = vmm_virt_to_phys((uint64_t)va); vmm_unmap_page((uint64_t)va); + if (phys) pmm_free_page((void*)phys); } } -- 2.43.0