From a27c5d2fb7e7f040f59ab668b6478dd6100f1d6d Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Wed, 29 Jan 2020 12:39:27 +0100 Subject: [PATCH] Kernel: Fail with EFAULT for any address+size that would wrap around Previously we were only checking that each of the virtual pages in the specified range were valid. This made it possible to pass in negative buffer sizes to some syscalls as long as (address) and (address+size) were on the same page. --- Kernel/VM/MemoryManager.cpp | 5 +++++ Userland/test_efault.cpp | 11 +++++++++++ 2 files changed, 16 insertions(+) diff --git a/Kernel/VM/MemoryManager.cpp b/Kernel/VM/MemoryManager.cpp index 9053f201f3..5f6147ae54 100644 --- a/Kernel/VM/MemoryManager.cpp +++ b/Kernel/VM/MemoryManager.cpp @@ -569,6 +569,11 @@ template base_vaddr.offset(size)) { + dbg() << "Shenanigans! Asked to validate wrappy " << base_vaddr << " size=" << size; + return false; + } + VirtualAddress vaddr = base_vaddr.page_base(); VirtualAddress end_vaddr = base_vaddr.offset(size - 1).page_base(); if (end_vaddr < vaddr) { diff --git a/Userland/test_efault.cpp b/Userland/test_efault.cpp index e2d2d91eb9..d02140701c 100644 --- a/Userland/test_efault.cpp +++ b/Userland/test_efault.cpp @@ -47,6 +47,14 @@ } \ } while(0) +#define EXPECT_EFAULT_NO_FD(syscall, address, size) \ + do { \ + rc = syscall((address), (size_t)(size)); \ + if (rc >= 0 || errno != EFAULT) { \ + fprintf(stderr, "Expected EFAULT: " #syscall "(%p, %zu), got rc=%d, errno=%d\n", (void*)(address), (size_t)(size), rc, errno); \ + } \ + } while(0) + int main(int, char**) { @@ -81,6 +89,9 @@ int main(int, char**) EXPECT_EFAULT(read, (void*)kernel_address, 1); } + char buffer[4096]; + EXPECT_EFAULT_NO_FD(dbgputstr, buffer, 0xffffff00); + // Test the page just below where the kernel VM begins. u8* jerk_page = (u8*)mmap((void*)(0xc0000000 - PAGE_SIZE), PAGE_SIZE, PROT_READ | PROT_WRITE, MAP_ANONYMOUS | MAP_PRIVATE | MAP_FIXED, 0, 0); ASSERT(jerk_page == (void*)(0xc0000000 - PAGE_SIZE));