From 40a942d28b979f26bb3442f9ad7bbdbd7882ff93 Mon Sep 17 00:00:00 2001 From: Brian Gianforcaro Date: Thu, 12 Aug 2021 22:04:31 -0700 Subject: [PATCH] Kernel: Remove char* versions of path argument / kstring copy methods The only two paths for copying strings in the kernel should be going through the existing Userspace, or StringArgument methods. Lets enforce this by removing the option for using the raw cstring APIs that were previously available. --- Kernel/Net/IPv4Socket.cpp | 3 ++- Kernel/Process.cpp | 5 +++-- Kernel/Process.h | 9 +++------ Kernel/StdLib.cpp | 15 +++++---------- Kernel/StdLib.h | 1 - Kernel/Syscalls/inode_watcher.cpp | 2 +- 6 files changed, 14 insertions(+), 21 deletions(-) diff --git a/Kernel/Net/IPv4Socket.cpp b/Kernel/Net/IPv4Socket.cpp index 35ee2f9646..ad2994bea1 100644 --- a/Kernel/Net/IPv4Socket.cpp +++ b/Kernel/Net/IPv4Socket.cpp @@ -592,7 +592,8 @@ KResult IPv4Socket::ioctl(FileDescription&, unsigned request, Userspace a if (!copy_from_user(&route, user_route)) return EFAULT; - auto ifname_or_error = try_copy_kstring_from_user(route.rt_dev, IFNAMSIZ); + Userspace user_rt_dev((FlatPtr)route.rt_dev); + auto ifname_or_error = try_copy_kstring_from_user(user_rt_dev, IFNAMSIZ); if (ifname_or_error.is_error()) return ifname_or_error.error(); diff --git a/Kernel/Process.cpp b/Kernel/Process.cpp index e3ec84c56c..4eb4d69c89 100644 --- a/Kernel/Process.cpp +++ b/Kernel/Process.cpp @@ -498,7 +498,7 @@ Custody& Process::current_directory() return *m_cwd; } -KResultOr> Process::get_syscall_path_argument(char const* user_path, size_t path_length) const +KResultOr> Process::get_syscall_path_argument(Userspace user_path, size_t path_length) const { if (path_length == 0) return EINVAL; @@ -512,7 +512,8 @@ KResultOr> Process::get_syscall_path_argument(char const* KResultOr> Process::get_syscall_path_argument(Syscall::StringArgument const& path) const { - return get_syscall_path_argument(path.characters, path.length); + Userspace path_characters((FlatPtr)path.characters); + return get_syscall_path_argument(path_characters, path.length); } bool Process::dump_core() diff --git a/Kernel/Process.h b/Kernel/Process.h index d28d0314b1..b26950b66b 100644 --- a/Kernel/Process.h +++ b/Kernel/Process.h @@ -539,11 +539,7 @@ private: KResultOr do_waitid(idtype_t idtype, int id, int options); - KResultOr> get_syscall_path_argument(const char* user_path, size_t path_length) const; - KResultOr> get_syscall_path_argument(Userspace user_path, size_t path_length) const - { - return get_syscall_path_argument(user_path.unsafe_userspace_ptr(), path_length); - } + KResultOr> get_syscall_path_argument(Userspace user_path, size_t path_length) const; KResultOr> get_syscall_path_argument(const Syscall::StringArgument&) const; bool has_tracee_thread(ProcessID tracer_pid); @@ -963,7 +959,8 @@ inline static String copy_string_from_user(const Kernel::Syscall::StringArgument inline static KResultOr> try_copy_kstring_from_user(const Kernel::Syscall::StringArgument& string) { - return try_copy_kstring_from_user(string.characters, string.length); + Userspace characters((FlatPtr)string.characters); + return try_copy_kstring_from_user(characters, string.length); } template<> diff --git a/Kernel/StdLib.cpp b/Kernel/StdLib.cpp index 838dd01465..4571c8b1ec 100644 --- a/Kernel/StdLib.cpp +++ b/Kernel/StdLib.cpp @@ -42,16 +42,16 @@ String copy_string_from_user(Userspace user_str, size_t user_str_si return copy_string_from_user(user_str.unsafe_userspace_ptr(), user_str_size); } -Kernel::KResultOr> try_copy_kstring_from_user(const char* user_str, size_t user_str_size) +Kernel::KResultOr> try_copy_kstring_from_user(Userspace user_str, size_t user_str_size) { bool is_user = Kernel::Memory::is_user_range(VirtualAddress(user_str), user_str_size); if (!is_user) return EFAULT; Kernel::SmapDisabler disabler; void* fault_at; - ssize_t length = Kernel::safe_strnlen(user_str, user_str_size, fault_at); + ssize_t length = Kernel::safe_strnlen(user_str.unsafe_userspace_ptr(), user_str_size, fault_at); if (length < 0) { - dbgln("copy_kstring_from_user({:p}, {}) failed at {} (strnlen)", static_cast(user_str), user_str_size, VirtualAddress { fault_at }); + dbgln("copy_kstring_from_user({:p}, {}) failed at {} (strnlen)", static_cast(user_str.unsafe_userspace_ptr()), user_str_size, VirtualAddress { fault_at }); return EFAULT; } char* buffer; @@ -64,18 +64,13 @@ Kernel::KResultOr> try_copy_kstring_from_user(con if (length == 0) return new_string.release_nonnull(); - if (!Kernel::safe_memcpy(buffer, user_str, (size_t)length, fault_at)) { - dbgln("copy_kstring_from_user({:p}, {}) failed at {} (memcpy)", static_cast(user_str), user_str_size, VirtualAddress { fault_at }); + if (!Kernel::safe_memcpy(buffer, user_str.unsafe_userspace_ptr(), (size_t)length, fault_at)) { + dbgln("copy_kstring_from_user({:p}, {}) failed at {} (memcpy)", static_cast(user_str.unsafe_userspace_ptr()), user_str_size, VirtualAddress { fault_at }); return EFAULT; } return new_string.release_nonnull(); } -Kernel::KResultOr> try_copy_kstring_from_user(Userspace user_str, size_t user_str_size) -{ - return try_copy_kstring_from_user(user_str.unsafe_userspace_ptr(), user_str_size); -} - [[nodiscard]] Optional