From f7d1b8cd0cba181b19eb10872ef2e84751af3733 Mon Sep 17 00:00:00 2001 From: Liav A Date: Thu, 23 Dec 2021 22:09:19 +0200 Subject: [PATCH] Kernel: Avoid potential memory info leak when doing mmap on /dev/mem Although we can still consider this impossible to happen now, because the mmap syscall entry code verifies that specified offset must be page aligned, it's still a good practice to VERIFY we actually take a start address as page-aligned in case of doing mmap on /dev/mem. As for read(2) on /dev/mem, we don't map anything to userspace so it's safe to read from whatever offset userspace specified as long as it does not break the original rules of reading physical memory from /dev/mem. --- Kernel/Devices/MemoryDevice.cpp | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/Kernel/Devices/MemoryDevice.cpp b/Kernel/Devices/MemoryDevice.cpp index c94667a601..30367eed6a 100644 --- a/Kernel/Devices/MemoryDevice.cpp +++ b/Kernel/Devices/MemoryDevice.cpp @@ -49,6 +49,16 @@ ErrorOr MemoryDevice::mmap(Process& process, OpenFileDescriptio { auto viewed_address = PhysicalAddress(offset); + // Note: This check happens to guard against possible memory leak. + // For example, if we try to mmap physical memory from 0x1000 to 0x2000 and you + // can actually mmap only from 0x1001, then we would fail as usual. + // However, in such case if we mmap from 0x1002, we are technically not violating + // any rules, besides the fact that we mapped an entire page with two bytes which we + // were not supposed to see. To prevent that, if we use mmap(2) syscall, we should + // always consider the start page to be aligned on PAGE_SIZE, or to be more precise + // is to be set to the page base of that start address. + VERIFY(viewed_address == viewed_address.page_base()); + dbgln("MemoryDevice: Trying to mmap physical memory at {} for range of {} bytes", viewed_address, range.size()); if (!MM.is_allowed_to_read_physical_memory_for_userspace(viewed_address, range.size())) { dbgln("MemoryDevice: Trying to mmap physical memory at {} for range of {} bytes failed due to violation of access", viewed_address, range.size());