mirror of
https://github.com/SerenityOS/serenity
synced 2024-10-07 08:29:58 +00:00
Kernel: Validate the full range of user memory passed to syscalls
We now validate the full range of userspace memory passed into syscalls instead of just checking that the first and last byte of the memory are in process-owned regions. This fixes an issue where it was possible to avoid rejection of invalid addresses that sat between two valid ones, simply by passing a valid address and a size large enough to put the end of the range at another valid address. I added a little test utility that tries to provoke EFAULT in various ways to help verify this. I'm sure we can think of more ways to test this but it's at least a start. :^) Thanks to mozjag for pointing out that this code was still lacking! Incidentally this also makes backtraces work again. Fixes #989.
This commit is contained in:
parent
e5ffa960d7
commit
3dcec260ed
|
@ -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)
|
||||
|
|
|
@ -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
|
||||
{
|
||||
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::Kernel, AccessType::Read>(process, vaddr, size);
|
||||
}
|
||||
|
||||
bool MemoryManager::validate_user_write(const Process& process, VirtualAddress vaddr) const
|
||||
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_writable();
|
||||
return validate_range<AccessSpace::User, AccessType::Read>(process, vaddr, size);
|
||||
}
|
||||
|
||||
bool MemoryManager::validate_user_write(const Process& process, VirtualAddress vaddr, size_t size) const
|
||||
{
|
||||
if (!is_user_address(vaddr))
|
||||
return false;
|
||||
return validate_range<AccessSpace::User, AccessType::Write>(process, vaddr, size);
|
||||
}
|
||||
|
||||
void MemoryManager::register_vmobject(VMObject& vmobject)
|
||||
|
|
|
@ -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&);
|
||||
|
|
69
Userland/test_efault.cpp
Normal file
69
Userland/test_efault.cpp
Normal file
|
@ -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;
|
||||
}
|
Loading…
Reference in a new issue