Kernel: Utilize AK::Userspace<T> in the ioctl interface

It's easy to forget the responsibility of validating and safely copying
kernel parameters in code that is far away from syscalls. ioctl's are
one such example, and bugs there are just as dangerous as at the root
syscall level.

To avoid this case, utilize the AK::Userspace<T> template in the ioctl
kernel interface so that implementors have no choice but to properly
validate and copy ioctl pointer arguments.

4 files changed, 34 insertions, 30 deletions

diff --git a/Kernel/Graphics/FramebufferDevice.cpp b/Kernel/Graphics/FramebufferDevice.cpp
index 182e4ebc34..435868ccb3 100644
--- a/Kernel/Graphics/FramebufferDevice.cpp
+++ b/Kernel/Graphics/FramebufferDevice.cpp
@@ -143,37 +143,38 @@ size_t FramebufferDevice::framebuffer_size_in_bytes() const
     return m_framebuffer_pitch * m_framebuffer_height;
 }
 
-int FramebufferDevice::ioctl(FileDescription&, unsigned request, FlatPtr arg)
+int FramebufferDevice::ioctl(FileDescription&, unsigned request, Userspace<void*> arg)
 {
     REQUIRE_PROMISE(video);
     switch (request) {
     case FB_IOCTL_GET_SIZE_IN_BYTES: {
-        auto* out = (size_t*)arg;
+        auto user_size = static_ptr_cast<size_t*>(arg);
         size_t value = framebuffer_size_in_bytes();
-        if (!copy_to_user(out, &value))
+        if (!copy_to_user(user_size, &value))
             return -EFAULT;
         return 0;
     }
     case FB_IOCTL_GET_BUFFER: {
-        auto* index = (int*)arg;
+        auto user_index = static_ptr_cast<int*>(arg);
         int value = m_y_offset == 0 ? 0 : 1;
-        if (!copy_to_user(index, &value))
+        if (!copy_to_user(user_index, &value))
             return -EFAULT;
         if (!m_graphics_adapter->double_framebuffering_capable())
             return -ENOTIMPL;
         return 0;
     }
     case FB_IOCTL_SET_BUFFER: {
-        if (arg != 0 && arg != 1)
+        auto buffer = static_cast<int>(arg.ptr());
+        if (buffer != 0 && buffer != 1)
             return -EINVAL;
         if (!m_graphics_adapter->double_framebuffering_capable())
             return -ENOTIMPL;
-        m_graphics_adapter->set_y_offset(m_output_port_index, arg == 0 ? 0 : m_framebuffer_height);
+        m_graphics_adapter->set_y_offset(m_output_port_index, buffer == 0 ? 0 : m_framebuffer_height);
         return 0;
     }
     case FB_IOCTL_GET_RESOLUTION: {
-        auto* user_resolution = (FBResolution*)arg;
-        FBResolution resolution;
+        auto user_resolution = static_ptr_cast<FBResolution*>(arg);
+        FBResolution resolution {};
         resolution.pitch = m_framebuffer_pitch;
         resolution.width = m_framebuffer_width;
         resolution.height = m_framebuffer_height;
@@ -182,7 +183,7 @@ int FramebufferDevice::ioctl(FileDescription&, unsigned request, FlatPtr arg)
         return 0;
     }
     case FB_IOCTL_SET_RESOLUTION: {
-        auto* user_resolution = (FBResolution*)arg;
+        auto user_resolution = static_ptr_cast<FBResolution*>(arg);
         FBResolution resolution;
         if (!copy_from_user(&resolution, user_resolution))
             return -EFAULT;
@@ -225,13 +226,14 @@ int FramebufferDevice::ioctl(FileDescription&, unsigned request, FlatPtr arg)
         return 0;
     }
     case FB_IOCTL_GET_BUFFER_OFFSET: {
+        auto user_buffer_offset = static_ptr_cast<FBBufferOffset*>(arg);
         FBBufferOffset buffer_offset;
-        if (!copy_from_user(&buffer_offset, (FBBufferOffset*)arg))
+        if (!copy_from_user(&buffer_offset, user_buffer_offset))
             return -EFAULT;
         if (buffer_offset.buffer_index != 0 && buffer_offset.buffer_index != 1)
             return -EINVAL;
         buffer_offset.offset = (size_t)buffer_offset.buffer_index * m_framebuffer_pitch * m_framebuffer_height;
-        if (!copy_to_user((FBBufferOffset*)arg, &buffer_offset))
+        if (!copy_to_user(user_buffer_offset, &buffer_offset))
             return -EFAULT;
         return 0;
     }
diff --git a/Kernel/Graphics/FramebufferDevice.h b/Kernel/Graphics/FramebufferDevice.h
index ebe3bbcc77..bde59ddbe3 100644
--- a/Kernel/Graphics/FramebufferDevice.h
+++ b/Kernel/Graphics/FramebufferDevice.h
@@ -22,7 +22,7 @@ class FramebufferDevice : public BlockDevice {
 public:
     static NonnullRefPtr<FramebufferDevice> create(const GraphicsDevice&, size_t, PhysicalAddress, size_t, size_t, size_t);
 
-    virtual int ioctl(FileDescription&, unsigned request, FlatPtr arg) override;
+    virtual int ioctl(FileDescription&, unsigned request, Userspace<void*> arg) override;
     virtual KResultOr<Region*> mmap(Process&, FileDescription&, const Range&, u64 offset, int prot, bool shared) override;
 
     // ^Device
diff --git a/Kernel/Graphics/VirtIOGPU/FrameBufferDevice.cpp b/Kernel/Graphics/VirtIOGPU/FrameBufferDevice.cpp
index 49b9bee8f7..126a1313a9 100644
--- a/Kernel/Graphics/VirtIOGPU/FrameBufferDevice.cpp
+++ b/Kernel/Graphics/VirtIOGPU/FrameBufferDevice.cpp
@@ -140,19 +140,19 @@ void FrameBufferDevice::set_buffer(int buffer_index)
     buffer.dirty_rect = {};
 }
 
-int FrameBufferDevice::ioctl(FileDescription&, unsigned request, FlatPtr arg)
+int FrameBufferDevice::ioctl(FileDescription&, unsigned request, Userspace<void*> arg)
 {
     REQUIRE_PROMISE(video);
     switch (request) {
     case FB_IOCTL_GET_SIZE_IN_BYTES: {
-        auto* out = (size_t*)arg;
+        auto out = static_ptr_cast<size_t*>(arg);
         size_t value = m_buffer_size * 2;
         if (!copy_to_user(out, &value))
             return -EFAULT;
         return 0;
     }
     case FB_IOCTL_SET_RESOLUTION: {
-        auto* user_resolution = (FBResolution*)arg;
+        auto user_resolution = static_ptr_cast<FBResolution*>(arg);
         FBResolution resolution;
         if (!copy_from_user(&resolution, user_resolution))
             return -EFAULT;
@@ -164,8 +164,8 @@ int FrameBufferDevice::ioctl(FileDescription&, unsigned request, FlatPtr arg)
         return 0;
     }
     case FB_IOCTL_GET_RESOLUTION: {
-        auto* user_resolution = (FBResolution*)arg;
-        FBResolution resolution;
+        auto user_resolution = static_ptr_cast<FBResolution*>(arg);
+        FBResolution resolution {};
         resolution.pitch = pitch();
         resolution.width = width();
         resolution.height = height();
@@ -174,7 +174,7 @@ int FrameBufferDevice::ioctl(FileDescription&, unsigned request, FlatPtr arg)
         return 0;
     }
     case FB_IOCTL_SET_BUFFER: {
-        auto buffer_index = (int)arg;
+        auto buffer_index = static_cast<int>(arg.ptr());
         if (!is_valid_buffer_index(buffer_index))
             return -EINVAL;
         if (m_last_set_buffer_index.exchange(buffer_index) != buffer_index && m_are_writes_active)
@@ -182,19 +182,20 @@ int FrameBufferDevice::ioctl(FileDescription&, unsigned request, FlatPtr arg)
         return 0;
     }
     case FB_IOCTL_FLUSH_BUFFERS: {
-        FBFlushRects user_flush_rects;
-        if (!copy_from_user(&user_flush_rects, (FBFlushRects*)arg))
+        auto user_flush_rects = static_ptr_cast<FBFlushRects*>(arg);
+        FBFlushRects flush_rects;
+        if (!copy_from_user(&flush_rects, user_flush_rects))
             return -EFAULT;
-        if (!is_valid_buffer_index(user_flush_rects.buffer_index))
+        if (!is_valid_buffer_index(flush_rects.buffer_index))
             return -EINVAL;
-        if (Checked<unsigned>::multiplication_would_overflow(user_flush_rects.count, sizeof(FBRect)))
+        if (Checked<unsigned>::multiplication_would_overflow(flush_rects.count, sizeof(FBRect)))
             return -EFAULT;
-        if (m_are_writes_active && user_flush_rects.count > 0) {
-            auto& buffer = buffer_from_index(user_flush_rects.buffer_index);
+        if (m_are_writes_active && flush_rects.count > 0) {
+            auto& buffer = buffer_from_index(flush_rects.buffer_index);
             MutexLocker locker(m_gpu.operation_lock());
-            for (unsigned i = 0; i < user_flush_rects.count; i++) {
+            for (unsigned i = 0; i < flush_rects.count; i++) {
                 FBRect user_dirty_rect;
-                if (!copy_from_user(&user_dirty_rect, &user_flush_rects.rects[i]))
+                if (!copy_from_user(&user_dirty_rect, &flush_rects.rects[i]))
                     return -EFAULT;
                 Protocol::Rect dirty_rect {
                     .x = user_dirty_rect.x,
@@ -224,13 +225,14 @@ int FrameBufferDevice::ioctl(FileDescription&, unsigned request, FlatPtr arg)
         return 0;
     }
     case FB_IOCTL_GET_BUFFER_OFFSET: {
+        auto user_buffer_offset = static_ptr_cast<FBBufferOffset*>(arg);
         FBBufferOffset buffer_offset;
-        if (!copy_from_user(&buffer_offset, (FBBufferOffset*)arg))
+        if (!copy_from_user(&buffer_offset, user_buffer_offset))
             return -EFAULT;
         if (!is_valid_buffer_index(buffer_offset.buffer_index))
             return -EINVAL;
         buffer_offset.offset = (size_t)buffer_offset.buffer_index * m_buffer_size;
-        if (!copy_to_user((FBBufferOffset*)arg, &buffer_offset))
+        if (!copy_to_user(user_buffer_offset, &buffer_offset))
             return -EFAULT;
         return 0;
     }
diff --git a/Kernel/Graphics/VirtIOGPU/FrameBufferDevice.h b/Kernel/Graphics/VirtIOGPU/FrameBufferDevice.h
index b7cb7f5bde..e7bbae8624 100644
--- a/Kernel/Graphics/VirtIOGPU/FrameBufferDevice.h
+++ b/Kernel/Graphics/VirtIOGPU/FrameBufferDevice.h
@@ -60,7 +60,7 @@ private:
     void create_buffer(Buffer&, size_t, size_t);
     void set_buffer(int);
 
-    virtual int ioctl(FileDescription&, unsigned request, FlatPtr arg) override;
+    virtual int ioctl(FileDescription&, unsigned request, Userspace<void*> arg) override;
     virtual KResultOr<Region*> mmap(Process&, FileDescription&, const Range&, u64 offset, int prot, bool shared) override;
     virtual bool can_read(const FileDescription&, size_t) const override { return true; }
     virtual KResultOr<size_t> read(FileDescription&, u64, UserOrKernelBuffer&, size_t) override { return EINVAL; }