From 4ef764e62e7f39b741e222d0e201c2dc42b80ad3 Mon Sep 17 00:00:00 2001 From: apio Date: Mon, 2 Jan 2023 11:51:08 +0100 Subject: [PATCH] mprotect(): Validate the entire range to protect is in userspace memory Before this patch, sys_mprotect() only validated the starting address. This was as bad as it sounds, in fact it let me write a fun exploit using it. Now, this exploit is no longer possible. This patch is probably not relevant, since this branch will be gone in the future, as soon as restart gets merged into main. I made it anyways :^) --- kernel/src/memory/Memory.cpp | 6 +++--- kernel/src/sys/mem.cpp | 21 ++++++++++++++------- 2 files changed, 17 insertions(+), 10 deletions(-) diff --git a/kernel/src/memory/Memory.cpp b/kernel/src/memory/Memory.cpp index 6ff02898..673b6a4e 100644 --- a/kernel/src/memory/Memory.cpp +++ b/kernel/src/memory/Memory.cpp @@ -45,10 +45,10 @@ uint64_t Memory::get_usable() bool Memory::is_kernel_address(uintptr_t address) { - return address >= 0xfffffffff8000000; + return address >= 0xffff800000000000; } bool Memory::is_user_address(uintptr_t address) { - return address && address < 0xfffffffff8000000; -} \ No newline at end of file + return address && address < 0x00007fffffffffff; +} diff --git a/kernel/src/sys/mem.cpp b/kernel/src/sys/mem.cpp index 99a6a0b0..bc240b0f 100644 --- a/kernel/src/sys/mem.cpp +++ b/kernel/src/sys/mem.cpp @@ -55,9 +55,9 @@ void sys_mmap(Context* context, void* address, size_t size, int prot, int fd, of if (address) { kdbgln("mmap(): %ld pages at address %p, %s, fd %d", size / PAGE_SIZE, address, format_prot(prot), fd); - if (Memory::is_kernel_address((uintptr_t)address)) + if (!Memory::is_user_address((uintptr_t)address)) { - kwarnln("munmap() failed: attempted to unmap a kernel page"); + kwarnln("mmap() failed: attempted to map a non-user page"); context->rax = MAP_FAIL(ENOMEM); return; } @@ -153,9 +153,9 @@ void sys_munmap(Context* context, void* address, size_t size) context->rax = -EINVAL; return; } - if (Memory::is_kernel_address((uintptr_t)address)) + if (!Memory::is_user_address((uintptr_t)address)) { - kwarnln("munmap() failed: attempted to unmap a kernel page"); + kwarnln("munmap() failed: attempted to unmap a non-user page"); context->rax = -EINVAL; return; } @@ -202,12 +202,19 @@ void sys_mprotect(Context* context, void* address, size_t size, int prot) context->rax = -EINVAL; return; } - if (Memory::is_kernel_address((uintptr_t)address)) + if (!Memory::is_user_address((uintptr_t)address)) { - kwarnln("mprotect() failed: attempted to protect a kernel page"); + kwarnln("mprotect() failed: attempted to protect a non-user page"); context->rax = -EINVAL; return; } + // FIXME: Check for overflow when adding address + size. + if (!Memory::is_user_address((uintptr_t)address + size)) + { + kwarnln("mprotect() failed: end of given range is out of user memory"); + context->rax = -EINVAL; + return; + } uint64_t flags = VMM::get_flags((uint64_t)address); if (flags == (uint64_t)-1) { @@ -223,4 +230,4 @@ void sys_mprotect(Context* context, void* address, size_t size, int prot) kdbgln("mprotect() succeeded"); context->rax = 0; return; -} \ No newline at end of file +}