diff options
| -rw-r--r-- | Kernel/Process.cpp | 18 | ||||
| -rw-r--r-- | Kernel/VM/MemoryManager.cpp | 44 | ||||
| -rw-r--r-- | Kernel/VM/MemoryManager.h | 11 | ||||
| -rw-r--r-- | Userland/test_efault.cpp | 69 |
4 files changed, 119 insertions, 23 deletions
diff --git a/Kernel/Process.cpp b/Kernel/Process.cpp index 7c5a834fc9..f5532c696a 100644 --- a/Kernel/Process.cpp +++ b/Kernel/Process.cpp @@ -1958,7 +1958,7 @@ bool Process::validate_read_from_kernel(VirtualAddress vaddr, ssize_t size) cons return false; if (is_kmalloc_address(vaddr.as_ptr())) return true; - return validate_read(vaddr.as_ptr(), size); + return MM.validate_kernel_read(*this, vaddr, size); } bool Process::validate_read_str(const char* str) @@ -1984,23 +1984,15 @@ bool Process::validate_read(const void* address, ssize_t size) const if (is_kmalloc_address(address)) return true; } - ASSERT(size); if (!size) return false; - if (first_address.page_base() != last_address.page_base()) { - if (!MM.validate_user_read(*this, last_address)) - return false; - } - return MM.validate_user_read(*this, first_address); + return MM.validate_user_read(*this, first_address, size); } bool Process::validate_write(void* address, ssize_t size) const { ASSERT(size >= 0); VirtualAddress first_address((u32)address); - VirtualAddress last_address = first_address.offset(size - 1); - if (last_address < first_address) - return false; if (is_ring0()) { if (is_kmalloc_address(address)) return true; @@ -2012,11 +2004,7 @@ bool Process::validate_write(void* address, ssize_t size) const } if (!size) return false; - if (first_address.page_base() != last_address.page_base()) { - if (!MM.validate_user_write(*this, last_address)) - return false; - } - return MM.validate_user_write(*this, first_address); + return MM.validate_user_write(*this, first_address, size); } pid_t Process::sys$getsid(pid_t pid) diff --git a/Kernel/VM/MemoryManager.cpp b/Kernel/VM/MemoryManager.cpp index 0b67633d52..e3728b4ae9 100644 --- a/Kernel/VM/MemoryManager.cpp +++ b/Kernel/VM/MemoryManager.cpp @@ -606,6 +606,35 @@ static inline bool is_user_address(VirtualAddress vaddr) return vaddr.get() >= (8 * MB) && vaddr.get() < 0xc0000000; } +template<MemoryManager::AccessSpace space, MemoryManager::AccessType access_type> +bool MemoryManager::validate_range(const Process& process, VirtualAddress base_vaddr, size_t size) const +{ + ASSERT(size); + VirtualAddress vaddr = base_vaddr.page_base(); + VirtualAddress end_vaddr = base_vaddr.offset(size - 1).page_base(); + if (end_vaddr < vaddr) { + dbg() << *current << " Shenanigans! Asked to validate " << base_vaddr << " size=" << size; + return false; + } + const Region* region = nullptr; + while (vaddr <= end_vaddr) { + if (!region || !region->contains(vaddr)) { + if (space == AccessSpace::Kernel) + region = kernel_region_from_vaddr(vaddr); + if (!region || !region->contains(vaddr)) + region = user_region_from_vaddr(const_cast<Process&>(process), vaddr); + if (!region + || (space == AccessSpace::User && !region->is_user_accessible()) + || (access_type == AccessType::Read && !region->is_readable()) + || (access_type == AccessType::Write && !region->is_writable())) { + return false; + } + } + vaddr = vaddr.offset(PAGE_SIZE); + } + return true; +} + bool MemoryManager::validate_user_stack(const Process& process, VirtualAddress vaddr) const { if (!is_user_address(vaddr)) @@ -614,20 +643,23 @@ bool MemoryManager::validate_user_stack(const Process& process, VirtualAddress v return region && region->is_user_accessible() && region->is_stack(); } -bool MemoryManager::validate_user_read(const Process& process, VirtualAddress vaddr) const +bool MemoryManager::validate_kernel_read(const Process& process, VirtualAddress vaddr, size_t size) const +{ + return validate_range<AccessSpace::Kernel, AccessType::Read>(process, vaddr, size); +} + +bool MemoryManager::validate_user_read(const Process& process, VirtualAddress vaddr, size_t size) const { if (!is_user_address(vaddr)) return false; - auto* region = user_region_from_vaddr(const_cast<Process&>(process), vaddr); - return region && region->is_user_accessible() && region->is_readable(); + return validate_range<AccessSpace::User, AccessType::Read>(process, vaddr, size); } -bool MemoryManager::validate_user_write(const Process& process, VirtualAddress vaddr) const +bool MemoryManager::validate_user_write(const Process& process, VirtualAddress vaddr, size_t size) const { if (!is_user_address(vaddr)) return false; - auto* region = user_region_from_vaddr(const_cast<Process&>(process), vaddr); - return region && region->is_user_accessible() && region->is_writable(); + return validate_range<AccessSpace::User, AccessType::Write>(process, vaddr, size); } void MemoryManager::register_vmobject(VMObject& vmobject) diff --git a/Kernel/VM/MemoryManager.h b/Kernel/VM/MemoryManager.h index 967e681a2c..517106cad9 100644 --- a/Kernel/VM/MemoryManager.h +++ b/Kernel/VM/MemoryManager.h @@ -45,8 +45,10 @@ public: void enter_process_paging_scope(Process&); bool validate_user_stack(const Process&, VirtualAddress) const; - bool validate_user_read(const Process&, VirtualAddress) const; - bool validate_user_write(const Process&, VirtualAddress) const; + bool validate_user_read(const Process&, VirtualAddress, size_t) const; + bool validate_user_write(const Process&, VirtualAddress, size_t) const; + + bool validate_kernel_read(const Process&, VirtualAddress, size_t) const; enum class ShouldZeroFill { No, @@ -85,6 +87,11 @@ private: MemoryManager(u32 physical_address_for_kernel_page_tables); ~MemoryManager(); + enum class AccessSpace { Kernel, User }; + enum class AccessType { Read, Write }; + template<AccessSpace, AccessType> + bool validate_range(const Process&, VirtualAddress, size_t) const; + void register_vmobject(VMObject&); void unregister_vmobject(VMObject&); void register_region(Region&); diff --git a/Userland/test_efault.cpp b/Userland/test_efault.cpp new file mode 100644 index 0000000000..ad5f004074 --- /dev/null +++ b/Userland/test_efault.cpp @@ -0,0 +1,69 @@ +#include <AK/Assertions.h> +#include <AK/Types.h> +#include <fcntl.h> +#include <stdio.h> +#include <sys/mman.h> +#include <unistd.h> + +#define EXPECT_OK(syscall, address, size) \ + do { \ + rc = syscall(fd, (void*)(address), (size_t)(size)); \ + if (rc < 0) { \ + fprintf(stderr, "Expected success: " #syscall "(%p, %zu), got rc=%d, errno=%d\n", (void*)(address), (size_t)(size), rc, errno); \ + } \ + } while(0) + +#define EXPECT_EFAULT(syscall, address, size) \ + do { \ + rc = syscall(fd, (void*)(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**) +{ + int fd = open("/dev/zero", O_RDONLY); + int rc; + + // Test a one-page mapping (4KB) + u8* one_page = (u8*)mmap(nullptr, 4096, PROT_READ | PROT_WRITE, MAP_ANONYMOUS | MAP_PRIVATE, 0, 0); + ASSERT(one_page); + EXPECT_OK(read, one_page, 4096); + EXPECT_EFAULT(read, one_page, 4097); + EXPECT_EFAULT(read, one_page - 1, 4096); + + // Test a two-page mapping (8KB) + u8* two_page = (u8*)mmap(nullptr, 8192, PROT_READ | PROT_WRITE, MAP_ANONYMOUS | MAP_PRIVATE, 0, 0); + ASSERT(two_page); + + EXPECT_OK(read, two_page, 4096); + EXPECT_OK(read, two_page + 4096, 4096); + EXPECT_OK(read, two_page, 8192); + EXPECT_OK(read, two_page + 4095, 4097); + EXPECT_OK(read, two_page + 1, 8191); + EXPECT_EFAULT(read, two_page, 8193); + EXPECT_EFAULT(read, two_page - 1, 1); + + // Check validation of pages between the first and last address. + ptrdiff_t distance = two_page - one_page; + EXPECT_EFAULT(read, one_page, (u32)distance + 1024); + + // Test every kernel page just because. + for (u64 kernel_address = 0xc0000000; kernel_address <= 0xffffffff; kernel_address += PAGE_SIZE) { + EXPECT_EFAULT(read, (void*)kernel_address, 1); + } + + // 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)); + + EXPECT_OK(read, jerk_page, 4096); + EXPECT_EFAULT(read, jerk_page, 4097); + + // Test something that would wrap around the 2^32 mark. + EXPECT_EFAULT(read, jerk_page, 0x50000000); + + return 0; +} |