From 9a04f53a0fcb8d6c3d449ca89f7fdbb83db68dd5 Mon Sep 17 00:00:00 2001 From: Brian Gianforcaro Date: Mon, 26 Jul 2021 02:47:00 -0700 Subject: [PATCH] Kernel: Utilize AK::Userspace 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 template in the ioctl kernel interface so that implementors have no choice but to properly validate and copy ioctl pointer arguments. --- Kernel/Devices/HID/KeyboardDevice.cpp | 17 ++--- Kernel/Devices/HID/KeyboardDevice.h | 2 +- Kernel/FileSystem/File.cpp | 3 +- Kernel/FileSystem/File.h | 2 +- Kernel/FileSystem/InodeFile.cpp | 7 ++- Kernel/FileSystem/InodeFile.h | 2 +- Kernel/Graphics/FramebufferDevice.cpp | 26 ++++---- Kernel/Graphics/FramebufferDevice.h | 2 +- .../Graphics/VirtIOGPU/FrameBufferDevice.cpp | 34 +++++----- Kernel/Graphics/VirtIOGPU/FrameBufferDevice.h | 2 +- Kernel/Net/IPv4Socket.cpp | 63 +++++++++---------- Kernel/Net/IPv4Socket.h | 2 +- Kernel/TTY/MasterPTY.cpp | 2 +- Kernel/TTY/MasterPTY.h | 2 +- Kernel/TTY/TTY.cpp | 24 +++---- Kernel/TTY/TTY.h | 2 +- 16 files changed, 99 insertions(+), 93 deletions(-) diff --git a/Kernel/Devices/HID/KeyboardDevice.cpp b/Kernel/Devices/HID/KeyboardDevice.cpp index 7d2b9d246c..a95465228e 100644 --- a/Kernel/Devices/HID/KeyboardDevice.cpp +++ b/Kernel/Devices/HID/KeyboardDevice.cpp @@ -311,31 +311,34 @@ KResultOr KeyboardDevice::write(FileDescription&, u64, const UserOrKerne return 0; } -int KeyboardDevice::ioctl(FileDescription&, unsigned request, FlatPtr arg) +int KeyboardDevice::ioctl(FileDescription&, unsigned request, Userspace arg) { switch (request) { case KEYBOARD_IOCTL_GET_NUM_LOCK: { - auto* output = (bool*)arg; + auto output = static_ptr_cast(arg); if (!copy_to_user(output, &m_num_lock_on)) return -EFAULT; return 0; } case KEYBOARD_IOCTL_SET_NUM_LOCK: { - if (arg != 0 && arg != 1) + // In this case we expect the value to be a boolean and not a pointer. + auto num_lock_value = static_cast(arg.ptr()); + if (num_lock_value != 0 && num_lock_value != 1) return -EINVAL; - m_num_lock_on = arg; + m_num_lock_on = !!num_lock_value; return 0; } case KEYBOARD_IOCTL_GET_CAPS_LOCK: { - auto* output = (bool*)arg; + auto output = static_ptr_cast(arg); if (!copy_to_user(output, &m_caps_lock_on)) return -EFAULT; return 0; } case KEYBOARD_IOCTL_SET_CAPS_LOCK: { - if (arg != 0 && arg != 1) + auto caps_lock_value = static_cast(arg.ptr()); + if (caps_lock_value != 0 && caps_lock_value != 1) return -EINVAL; - m_caps_lock_on = arg; + m_caps_lock_on = !!caps_lock_value; return 0; } default: diff --git a/Kernel/Devices/HID/KeyboardDevice.h b/Kernel/Devices/HID/KeyboardDevice.h index fc20140bd9..5bd43a9767 100644 --- a/Kernel/Devices/HID/KeyboardDevice.h +++ b/Kernel/Devices/HID/KeyboardDevice.h @@ -37,7 +37,7 @@ public: virtual mode_t required_mode() const override { return 0440; } // ^File - virtual int ioctl(FileDescription&, unsigned request, FlatPtr arg) override; + virtual int ioctl(FileDescription&, unsigned request, Userspace arg) override; virtual String device_name() const override { return String::formatted("keyboard{}", minor()); } diff --git a/Kernel/FileSystem/File.cpp b/Kernel/FileSystem/File.cpp index cc59666497..29bc1fc5e9 100644 --- a/Kernel/FileSystem/File.cpp +++ b/Kernel/FileSystem/File.cpp @@ -5,6 +5,7 @@ */ #include +#include #include #include #include @@ -34,7 +35,7 @@ KResult File::close() return KSuccess; } -int File::ioctl(FileDescription&, unsigned, FlatPtr) +int File::ioctl(FileDescription&, unsigned, Userspace) { return -ENOTTY; } diff --git a/Kernel/FileSystem/File.h b/Kernel/FileSystem/File.h index f7424e6aaa..f51569897e 100644 --- a/Kernel/FileSystem/File.h +++ b/Kernel/FileSystem/File.h @@ -87,7 +87,7 @@ public: virtual void did_seek(FileDescription&, off_t) { } virtual KResultOr read(FileDescription&, u64, UserOrKernelBuffer&, size_t) = 0; virtual KResultOr write(FileDescription&, u64, const UserOrKernelBuffer&, size_t) = 0; - virtual int ioctl(FileDescription&, unsigned request, FlatPtr arg); + virtual int ioctl(FileDescription&, unsigned request, Userspace arg); virtual KResultOr mmap(Process&, FileDescription&, const Range&, u64 offset, int prot, bool shared); virtual KResult stat(::stat&) const { return EBADF; } diff --git a/Kernel/FileSystem/InodeFile.cpp b/Kernel/FileSystem/InodeFile.cpp index 6a6a34d956..a0d657b88b 100644 --- a/Kernel/FileSystem/InodeFile.cpp +++ b/Kernel/FileSystem/InodeFile.cpp @@ -62,7 +62,7 @@ KResultOr InodeFile::write(FileDescription& description, u64 offset, con return nwritten; } -int InodeFile::ioctl(FileDescription& description, unsigned request, FlatPtr arg) +int InodeFile::ioctl(FileDescription& description, unsigned request, Userspace arg) { (void)description; @@ -71,8 +71,9 @@ int InodeFile::ioctl(FileDescription& description, unsigned request, FlatPtr arg if (!Process::current()->is_superuser()) return -EPERM; + auto user_block_number = static_ptr_cast(arg); int block_number = 0; - if (!copy_from_user(&block_number, (int*)arg)) + if (!copy_from_user(&block_number, user_block_number)) return -EFAULT; if (block_number < 0) @@ -82,7 +83,7 @@ int InodeFile::ioctl(FileDescription& description, unsigned request, FlatPtr arg if (block_address.is_error()) return block_address.error(); - if (!copy_to_user((int*)arg, &block_address.value())) + if (!copy_to_user(user_block_number, &block_address.value())) return -EFAULT; return 0; diff --git a/Kernel/FileSystem/InodeFile.h b/Kernel/FileSystem/InodeFile.h index 7c556fcd82..2fb6a38d17 100644 --- a/Kernel/FileSystem/InodeFile.h +++ b/Kernel/FileSystem/InodeFile.h @@ -32,7 +32,7 @@ public: virtual KResultOr read(FileDescription&, u64, UserOrKernelBuffer&, size_t) override; virtual KResultOr write(FileDescription&, u64, const UserOrKernelBuffer&, size_t) override; - virtual int ioctl(FileDescription&, unsigned request, FlatPtr arg) override; + virtual int ioctl(FileDescription&, unsigned request, Userspace arg) override; virtual KResultOr mmap(Process&, FileDescription&, const Range&, u64 offset, int prot, bool shared) override; virtual KResult stat(::stat& buffer) const override { return inode().metadata().stat(buffer); } 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 arg) { REQUIRE_PROMISE(video); switch (request) { case FB_IOCTL_GET_SIZE_IN_BYTES: { - auto* out = (size_t*)arg; + auto user_size = static_ptr_cast(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(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(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(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(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(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 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 arg) override; virtual KResultOr 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 arg) { REQUIRE_PROMISE(video); switch (request) { case FB_IOCTL_GET_SIZE_IN_BYTES: { - auto* out = (size_t*)arg; + auto out = static_ptr_cast(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(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(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(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(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::multiplication_would_overflow(user_flush_rects.count, sizeof(FBRect))) + if (Checked::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(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 arg) override; virtual KResultOr 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 read(FileDescription&, u64, UserOrKernelBuffer&, size_t) override { return EINVAL; } diff --git a/Kernel/Net/IPv4Socket.cpp b/Kernel/Net/IPv4Socket.cpp index aded09dc5a..9aa0fd61bc 100644 --- a/Kernel/Net/IPv4Socket.cpp +++ b/Kernel/Net/IPv4Socket.cpp @@ -568,13 +568,14 @@ KResult IPv4Socket::getsockopt(FileDescription& description, int level, int opti } } -int IPv4Socket::ioctl(FileDescription&, unsigned request, FlatPtr arg) +int IPv4Socket::ioctl(FileDescription&, unsigned request, Userspace arg) { REQUIRE_PROMISE(inet); auto ioctl_route = [request, arg]() { + auto user_route = static_ptr_cast(arg); rtentry route; - if (!copy_from_user(&route, (rtentry*)arg)) + if (!copy_from_user(&route, user_route)) return -EFAULT; auto copied_ifname = copy_string_from_user(route.rt_dev, IFNAMSIZ); @@ -605,8 +606,9 @@ int IPv4Socket::ioctl(FileDescription&, unsigned request, FlatPtr arg) }; auto ioctl_arp = [request, arg]() { + auto user_req = static_ptr_cast(arg); arpreq arp_req; - if (!copy_from_user(&arp_req, (arpreq*)arg)) + if (!copy_from_user(&arp_req, user_req)) return -EFAULT; switch (request) { @@ -631,7 +633,7 @@ int IPv4Socket::ioctl(FileDescription&, unsigned request, FlatPtr arg) }; auto ioctl_interface = [request, arg]() { - ifreq* user_ifr = (ifreq*)arg; + auto user_ifr = static_ptr_cast(arg); ifreq ifr; if (!copy_from_user(&ifr, user_ifr)) return -EFAULT; @@ -662,72 +664,65 @@ int IPv4Socket::ioctl(FileDescription&, unsigned request, FlatPtr arg) return 0; case SIOCGIFADDR: { - u16 sa_family = AF_INET; - if (!copy_to_user(&user_ifr->ifr_addr.sa_family, &sa_family)) - return -EFAULT; auto ip4_addr = adapter->ipv4_address().to_u32(); - if (!copy_to_user(&((sockaddr_in&)user_ifr->ifr_addr).sin_addr.s_addr, &ip4_addr, sizeof(ip4_addr))) + auto& socket_address_in = reinterpret_cast(ifr.ifr_addr); + socket_address_in.sin_family = AF_INET; + socket_address_in.sin_addr.s_addr = ip4_addr; + if (!copy_to_user(user_ifr, &ifr)) return -EFAULT; return 0; } case SIOCGIFNETMASK: { - u16 sa_family = AF_INET; - if (!copy_to_user(&user_ifr->ifr_addr.sa_family, &sa_family)) - return -EFAULT; auto ip4_netmask = adapter->ipv4_netmask().to_u32(); + auto& socket_address_in = reinterpret_cast(ifr.ifr_addr); + socket_address_in.sin_family = AF_INET; // NOTE: NOT ifr_netmask. - if (!copy_to_user(&((sockaddr_in&)user_ifr->ifr_addr).sin_addr.s_addr, &ip4_netmask, sizeof(ip4_netmask))) + socket_address_in.sin_addr.s_addr = ip4_netmask; + + if (!copy_to_user(user_ifr, &ifr)) return -EFAULT; return 0; } case SIOCGIFHWADDR: { - u16 sa_family = AF_INET; - if (!copy_to_user(&user_ifr->ifr_hwaddr.sa_family, &sa_family)) - return -EFAULT; auto mac_address = adapter->mac_address(); - if (!copy_to_user(ifr.ifr_hwaddr.sa_data, &mac_address, sizeof(MACAddress))) + ifr.ifr_hwaddr.sa_family = AF_INET; + mac_address.copy_to(Bytes { ifr.ifr_hwaddr.sa_data, sizeof(ifr.ifr_hwaddr.sa_data) }); + if (!copy_to_user(user_ifr, &ifr)) return -EFAULT; return 0; } case SIOCGIFBRDADDR: { - u16 sa_family = AF_INET; - if (!copy_to_user(&user_ifr->ifr_addr.sa_family, &sa_family)) - return -EFAULT; - // Broadcast address is basically the reverse of the netmask, i.e. // instead of zeroing out the end, you OR with 1 instead. auto ip4_netmask = adapter->ipv4_netmask().to_u32(); auto broadcast_addr = adapter->ipv4_address().to_u32() | ~ip4_netmask; - - if (!copy_to_user(&((sockaddr_in&)user_ifr->ifr_addr).sin_addr.s_addr, &broadcast_addr, sizeof(broadcast_addr))) + auto& socket_address_in = reinterpret_cast(ifr.ifr_addr); + socket_address_in.sin_family = AF_INET; + socket_address_in.sin_addr.s_addr = broadcast_addr; + if (!copy_to_user(user_ifr, &ifr)) return -EFAULT; return 0; } case SIOCGIFMTU: { - u16 sa_family = AF_INET; - if (!copy_to_user(&user_ifr->ifr_addr.sa_family, &sa_family)) - return -EFAULT; - auto ip4_metric = adapter->mtu(); - if (!copy_to_user(&user_ifr->ifr_metric, &ip4_metric, sizeof(ip4_metric))) + ifr.ifr_addr.sa_family = AF_INET; + ifr.ifr_metric = ip4_metric; + if (!copy_to_user(user_ifr, &ifr)) return -EFAULT; return 0; } case SIOCGIFFLAGS: { - u16 sa_family = AF_INET; - if (!copy_to_user(&user_ifr->ifr_addr.sa_family, &sa_family)) - return -EFAULT; - // FIXME: stub! - short flags = 1; - - if (!copy_to_user(&user_ifr->ifr_flags, &flags, sizeof(flags))) + constexpr short flags = 1; + ifr.ifr_addr.sa_family = AF_INET; + ifr.ifr_flags = flags; + if (!copy_to_user(user_ifr, &ifr)) return -EFAULT; return 0; } diff --git a/Kernel/Net/IPv4Socket.h b/Kernel/Net/IPv4Socket.h index 7f025882ee..1eed0a4d09 100644 --- a/Kernel/Net/IPv4Socket.h +++ b/Kernel/Net/IPv4Socket.h @@ -46,7 +46,7 @@ public: virtual KResult setsockopt(int level, int option, Userspace, socklen_t) override; virtual KResult getsockopt(FileDescription&, int level, int option, Userspace, Userspace) override; - virtual int ioctl(FileDescription&, unsigned request, FlatPtr arg) override; + virtual int ioctl(FileDescription&, unsigned request, Userspace arg) override; bool did_receive(const IPv4Address& peer_address, u16 peer_port, ReadonlyBytes, const Time&); diff --git a/Kernel/TTY/MasterPTY.cpp b/Kernel/TTY/MasterPTY.cpp index 113e9622a1..6cf84f2c9f 100644 --- a/Kernel/TTY/MasterPTY.cpp +++ b/Kernel/TTY/MasterPTY.cpp @@ -106,7 +106,7 @@ KResult MasterPTY::close() return KSuccess; } -int MasterPTY::ioctl(FileDescription& description, unsigned request, FlatPtr arg) +int MasterPTY::ioctl(FileDescription& description, unsigned request, Userspace arg) { REQUIRE_PROMISE(tty); if (!m_slave) diff --git a/Kernel/TTY/MasterPTY.h b/Kernel/TTY/MasterPTY.h index fa54d7f837..96fcadea7a 100644 --- a/Kernel/TTY/MasterPTY.h +++ b/Kernel/TTY/MasterPTY.h @@ -40,7 +40,7 @@ private: virtual bool can_write(const FileDescription&, size_t) const override; virtual KResult close() override; virtual bool is_master_pty() const override { return true; } - virtual int ioctl(FileDescription&, unsigned request, FlatPtr arg) override; + virtual int ioctl(FileDescription&, unsigned request, Userspace arg) override; virtual StringView class_name() const override { return "MasterPTY"; } RefPtr m_slave; diff --git a/Kernel/TTY/TTY.cpp b/Kernel/TTY/TTY.cpp index 96b3d7d869..a58b882b4e 100644 --- a/Kernel/TTY/TTY.cpp +++ b/Kernel/TTY/TTY.cpp @@ -454,12 +454,12 @@ int TTY::set_termios(const termios& t) return rc; } -int TTY::ioctl(FileDescription&, unsigned request, FlatPtr arg) +int TTY::ioctl(FileDescription&, unsigned request, Userspace arg) { REQUIRE_PROMISE(tty); auto& current_process = *Process::current(); - termios* user_termios; - winsize* user_winsize; + Userspace user_termios; + Userspace user_winsize; #if 0 // FIXME: When should we block things? @@ -472,7 +472,7 @@ int TTY::ioctl(FileDescription&, unsigned request, FlatPtr arg) case TIOCGPGRP: return this->pgid().value(); case TIOCSPGRP: { - ProcessGroupID pgid = static_cast(arg); + ProcessGroupID pgid = static_cast(arg.ptr()); if (pgid <= 0) return -EINVAL; InterruptDisabler disabler; @@ -500,7 +500,7 @@ int TTY::ioctl(FileDescription&, unsigned request, FlatPtr arg) return 0; } case TCGETS: { - user_termios = reinterpret_cast(arg); + user_termios = static_ptr_cast(arg); if (!copy_to_user(user_termios, &m_termios)) return -EFAULT; return 0; @@ -508,7 +508,7 @@ int TTY::ioctl(FileDescription&, unsigned request, FlatPtr arg) case TCSETS: case TCSETSF: case TCSETSW: { - user_termios = reinterpret_cast(arg); + user_termios = static_ptr_cast(arg); termios termios; if (!copy_from_user(&termios, user_termios)) return -EFAULT; @@ -517,16 +517,18 @@ int TTY::ioctl(FileDescription&, unsigned request, FlatPtr arg) flush_input(); return rc; } - case TCFLSH: + case TCFLSH: { // Serenity's TTY implementation does not use an output buffer, so ignore TCOFLUSH. - if (arg == TCIFLUSH || arg == TCIOFLUSH) { + auto operation = static_cast(arg.ptr()); + if (operation == TCIFLUSH || operation == TCIOFLUSH) { flush_input(); - } else if (arg != TCOFLUSH) { + } else if (operation != TCOFLUSH) { return -EINVAL; } return 0; + } case TIOCGWINSZ: - user_winsize = reinterpret_cast(arg); + user_winsize = static_ptr_cast(arg); winsize ws; ws.ws_row = m_rows; ws.ws_col = m_columns; @@ -536,7 +538,7 @@ int TTY::ioctl(FileDescription&, unsigned request, FlatPtr arg) return -EFAULT; return 0; case TIOCSWINSZ: { - user_winsize = reinterpret_cast(arg); + user_winsize = static_ptr_cast(arg); winsize ws; if (!copy_from_user(&ws, user_winsize)) return -EFAULT; diff --git a/Kernel/TTY/TTY.h b/Kernel/TTY/TTY.h index 20c70a0343..6d611b3332 100644 --- a/Kernel/TTY/TTY.h +++ b/Kernel/TTY/TTY.h @@ -25,7 +25,7 @@ public: virtual KResultOr write(FileDescription&, u64, const UserOrKernelBuffer&, size_t) override; virtual bool can_read(const FileDescription&, size_t) const override; virtual bool can_write(const FileDescription&, size_t) const override; - virtual int ioctl(FileDescription&, unsigned request, FlatPtr arg) override final; + virtual int ioctl(FileDescription&, unsigned request, Userspace arg) override final; virtual String absolute_path(const FileDescription&) const override { return tty_name(); } virtual String const& tty_name() const = 0;