From 286984750e33a5e1eed60b1a4beef95dfb2b46ae Mon Sep 17 00:00:00 2001 From: Daniel Bertalan Date: Fri, 11 Aug 2023 10:47:31 +0200 Subject: [PATCH] Kernel+LibC: Pass 64-bit integers in syscalls by value Now that support for 32-bit x86 has been removed, we don't have to worry about the top half of `off_t`/`u64` values being chopped off when we try to pass them in registers. Therefore, we no longer need the workaround of pointers to stack-allocated values to syscalls. Note that this changes the system call ABI, so statically linked programs will have to be re-linked. --- Kernel/Syscalls/fallocate.cpp | 4 +--- Kernel/Syscalls/ftruncate.cpp | 5 +---- Kernel/Syscalls/profiling.cpp | 5 +---- Kernel/Syscalls/read.cpp | 5 +---- Kernel/Syscalls/write.cpp | 7 ++----- Kernel/Tasks/Process.h | 10 +++++----- Userland/DevTools/UserspaceEmulator/Emulator.h | 2 +- .../DevTools/UserspaceEmulator/Emulator_syscalls.cpp | 11 +++++------ Userland/Libraries/LibC/fcntl.cpp | 2 +- Userland/Libraries/LibC/serenity.cpp | 2 +- Userland/Libraries/LibC/sys/uio.cpp | 2 +- Userland/Libraries/LibC/unistd.cpp | 4 ++-- 12 files changed, 22 insertions(+), 37 deletions(-) diff --git a/Kernel/Syscalls/fallocate.cpp b/Kernel/Syscalls/fallocate.cpp index 708e165e1a..1251b8b2b6 100644 --- a/Kernel/Syscalls/fallocate.cpp +++ b/Kernel/Syscalls/fallocate.cpp @@ -13,16 +13,14 @@ namespace Kernel { // https://pubs.opengroup.org/onlinepubs/9699919799/functions/posix_fallocate.html -ErrorOr Process::sys$posix_fallocate(int fd, Userspace userspace_offset, Userspace userspace_length) +ErrorOr Process::sys$posix_fallocate(int fd, off_t offset, off_t length) { VERIFY_NO_PROCESS_BIG_LOCK(this); TRY(require_promise(Pledge::stdio)); // [EINVAL] The len argument is less than zero, or the offset argument is less than zero, or the underlying file system does not support this operation. - auto offset = TRY(copy_typed_from_user(userspace_offset)); if (offset < 0) return EINVAL; - auto length = TRY(copy_typed_from_user(userspace_length)); if (length <= 0) return EINVAL; diff --git a/Kernel/Syscalls/ftruncate.cpp b/Kernel/Syscalls/ftruncate.cpp index b0f972b770..a1791f545c 100644 --- a/Kernel/Syscalls/ftruncate.cpp +++ b/Kernel/Syscalls/ftruncate.cpp @@ -9,13 +9,10 @@ namespace Kernel { -// NOTE: The length is passed by pointer because off_t is 64bit, -// hence it can't be passed by register on 32bit platforms. -ErrorOr Process::sys$ftruncate(int fd, Userspace userspace_length) +ErrorOr Process::sys$ftruncate(int fd, off_t length) { VERIFY_NO_PROCESS_BIG_LOCK(this); TRY(require_promise(Pledge::stdio)); - auto length = TRY(copy_typed_from_user(userspace_length)); if (length < 0) return EINVAL; auto description = TRY(open_file_description(fd)); diff --git a/Kernel/Syscalls/profiling.cpp b/Kernel/Syscalls/profiling.cpp index f8a3a13f1d..f6e0c999a6 100644 --- a/Kernel/Syscalls/profiling.cpp +++ b/Kernel/Syscalls/profiling.cpp @@ -16,14 +16,11 @@ bool g_profiling_all_threads; PerformanceEventBuffer* g_global_perf_events; u64 g_profiling_event_mask; -// NOTE: event_mask needs to be passed as a pointer as u64 -// does not fit into a register on 32bit architectures. -ErrorOr Process::sys$profiling_enable(pid_t pid, Userspace userspace_event_mask) +ErrorOr Process::sys$profiling_enable(pid_t pid, u64 event_mask) { VERIFY_PROCESS_BIG_LOCK_ACQUIRED(this); TRY(require_no_promises()); - auto const event_mask = TRY(copy_typed_from_user(userspace_event_mask)); return profiling_enable(pid, event_mask); } diff --git a/Kernel/Syscalls/read.cpp b/Kernel/Syscalls/read.cpp index 6c18561db0..7f8f928f89 100644 --- a/Kernel/Syscalls/read.cpp +++ b/Kernel/Syscalls/read.cpp @@ -101,9 +101,7 @@ ErrorOr Process::read_impl(int fd, Userspace buffer, size_t size) return TRY(description->read(user_buffer, size)); } -// NOTE: The offset is passed by pointer because off_t is 64bit, -// hence it can't be passed by register on 32bit platforms. -ErrorOr Process::sys$pread(int fd, Userspace buffer, size_t size, Userspace userspace_offset) +ErrorOr Process::sys$pread(int fd, Userspace buffer, size_t size, off_t offset) { VERIFY_PROCESS_BIG_LOCK_ACQUIRED(this); TRY(require_promise(Pledge::stdio)); @@ -111,7 +109,6 @@ ErrorOr Process::sys$pread(int fd, Userspace buffer, size_t size, return 0; if (size > NumericLimits::max()) return EINVAL; - auto offset = TRY(copy_typed_from_user(userspace_offset)); if (offset < 0) return EINVAL; dbgln_if(IO_DEBUG, "sys$pread({}, {}, {}, {})", fd, buffer.ptr(), size, offset); diff --git a/Kernel/Syscalls/write.cpp b/Kernel/Syscalls/write.cpp index 4e474b0838..fd59d696cc 100644 --- a/Kernel/Syscalls/write.cpp +++ b/Kernel/Syscalls/write.cpp @@ -11,9 +11,7 @@ namespace Kernel { -// NOTE: The offset is passed by pointer because off_t is 64bit, -// hence it can't be passed by register on 32bit platforms. -ErrorOr Process::sys$pwritev(int fd, Userspace iov, int iov_count, Userspace userspace_offset) +ErrorOr Process::sys$pwritev(int fd, Userspace iov, int iov_count, off_t base_offset) { VERIFY_PROCESS_BIG_LOCK_ACQUIRED(this); TRY(require_promise(Pledge::stdio)); @@ -33,11 +31,10 @@ ErrorOr Process::sys$pwritev(int fd, Userspace iov return EINVAL; } - // NOTE: Negative offset means "operate like writev" which seeks the file. - auto base_offset = TRY(copy_typed_from_user(userspace_offset)); auto description = TRY(open_file_description(fd)); if (!description->is_writable()) return EBADF; + // NOTE: Negative offset means "operate like writev" which seeks the file. if (base_offset >= 0 && !description->file().is_seekable()) return EINVAL; diff --git a/Kernel/Tasks/Process.h b/Kernel/Tasks/Process.h index c127a470ef..c3578a841a 100644 --- a/Kernel/Tasks/Process.h +++ b/Kernel/Tasks/Process.h @@ -337,17 +337,17 @@ public: ErrorOr sys$open(Userspace); ErrorOr sys$close(int fd); ErrorOr sys$read(int fd, Userspace, size_t); - ErrorOr sys$pread(int fd, Userspace, size_t, Userspace); + ErrorOr sys$pread(int fd, Userspace, size_t, off_t); ErrorOr sys$readv(int fd, Userspace iov, int iov_count); ErrorOr sys$write(int fd, Userspace, size_t); - ErrorOr sys$pwritev(int fd, Userspace iov, int iov_count, Userspace); + ErrorOr sys$pwritev(int fd, Userspace iov, int iov_count, off_t); ErrorOr sys$fstat(int fd, Userspace); ErrorOr sys$stat(Userspace); ErrorOr sys$annotate_mapping(Userspace, int flags); ErrorOr sys$lseek(int fd, Userspace, int whence); - ErrorOr sys$ftruncate(int fd, Userspace); + ErrorOr sys$ftruncate(int fd, off_t); ErrorOr sys$futimens(Userspace); - ErrorOr sys$posix_fallocate(int fd, Userspace, Userspace); + ErrorOr sys$posix_fallocate(int fd, off_t, off_t); ErrorOr sys$kill(pid_t pid_or_pgid, int sig); [[noreturn]] void sys$exit(int status); ErrorOr sys$sigreturn(RegisterState& registers); @@ -443,7 +443,7 @@ public: ErrorOr sys$getrandom(Userspace, size_t, unsigned int); ErrorOr sys$getkeymap(Userspace); ErrorOr sys$setkeymap(Userspace); - ErrorOr sys$profiling_enable(pid_t, Userspace); + ErrorOr sys$profiling_enable(pid_t, u64); ErrorOr profiling_enable(pid_t, u64 event_mask); ErrorOr sys$profiling_disable(pid_t); ErrorOr sys$profiling_free_buffer(pid_t); diff --git a/Userland/DevTools/UserspaceEmulator/Emulator.h b/Userland/DevTools/UserspaceEmulator/Emulator.h index adb1fc7e44..0ffef1c154 100644 --- a/Userland/DevTools/UserspaceEmulator/Emulator.h +++ b/Userland/DevTools/UserspaceEmulator/Emulator.h @@ -213,7 +213,7 @@ private: u32 virt$pledge(u32); int virt$poll(FlatPtr); int virt$profiling_disable(pid_t); - int virt$profiling_enable(pid_t); + int virt$profiling_enable(pid_t, u64); int virt$purge(int mode); u32 virt$read(int, FlatPtr, ssize_t); int virt$readlink(FlatPtr); diff --git a/Userland/DevTools/UserspaceEmulator/Emulator_syscalls.cpp b/Userland/DevTools/UserspaceEmulator/Emulator_syscalls.cpp index f92f240167..32217a2335 100644 --- a/Userland/DevTools/UserspaceEmulator/Emulator_syscalls.cpp +++ b/Userland/DevTools/UserspaceEmulator/Emulator_syscalls.cpp @@ -185,7 +185,7 @@ u32 Emulator::virt_syscall(u32 function, u32 arg1, u32 arg2, u32 arg3) case SC_profiling_disable: return virt$profiling_disable(arg1); case SC_profiling_enable: - return virt$profiling_enable(arg1); + return virt$profiling_enable(arg1, arg2); case SC_purge: return virt$purge(arg1); case SC_read: @@ -281,9 +281,9 @@ int Emulator::virt$recvfd(int socket, int options) return syscall(SC_recvfd, socket, options); } -int Emulator::virt$profiling_enable(pid_t pid) +int Emulator::virt$profiling_enable(pid_t pid, u64 mask) { - return syscall(SC_profiling_enable, pid); + return syscall(SC_profiling_enable, pid, mask); } int Emulator::virt$profiling_disable(pid_t pid) @@ -474,11 +474,10 @@ int Emulator::virt$get_stack_bounds(FlatPtr base, FlatPtr size) return 0; } -int Emulator::virt$ftruncate(int fd, FlatPtr length_addr) +int Emulator::virt$ftruncate(int fd, off_t length) { off_t length; - mmu().copy_from_vm(&length, length_addr, sizeof(off_t)); - return syscall(SC_ftruncate, fd, &length); + return syscall(SC_ftruncate, fd, length); } int Emulator::virt$uname(FlatPtr params_addr) diff --git a/Userland/Libraries/LibC/fcntl.cpp b/Userland/Libraries/LibC/fcntl.cpp index 4857ded2d5..4d3a9b4096 100644 --- a/Userland/Libraries/LibC/fcntl.cpp +++ b/Userland/Libraries/LibC/fcntl.cpp @@ -118,7 +118,7 @@ int posix_fadvise(int fd, off_t offset, off_t len, int advice) int posix_fallocate(int fd, off_t offset, off_t len) { // posix_fallocate does not set errno. - return -static_cast(syscall(SC_posix_fallocate, fd, &offset, &len)); + return -static_cast(syscall(SC_posix_fallocate, fd, offset, len)); } // https://pubs.opengroup.org/onlinepubs/9699919799/functions/utimensat.html diff --git a/Userland/Libraries/LibC/serenity.cpp b/Userland/Libraries/LibC/serenity.cpp index 45f1bab522..aae2d5fc98 100644 --- a/Userland/Libraries/LibC/serenity.cpp +++ b/Userland/Libraries/LibC/serenity.cpp @@ -22,7 +22,7 @@ int disown(pid_t pid) int profiling_enable(pid_t pid, uint64_t event_mask) { - int rc = syscall(SC_profiling_enable, pid, &event_mask); + int rc = syscall(SC_profiling_enable, pid, event_mask); __RETURN_WITH_ERRNO(rc, rc, -1); } diff --git a/Userland/Libraries/LibC/sys/uio.cpp b/Userland/Libraries/LibC/sys/uio.cpp index 972abcfd44..8e7d19162d 100644 --- a/Userland/Libraries/LibC/sys/uio.cpp +++ b/Userland/Libraries/LibC/sys/uio.cpp @@ -28,7 +28,7 @@ ssize_t pwritev(int fd, struct iovec const* iov, int iov_count, off_t offset) { __pthread_maybe_cancel(); - int rc = syscall(SC_pwritev, fd, iov, iov_count, &offset); + int rc = syscall(SC_pwritev, fd, iov, iov_count, offset); __RETURN_WITH_ERRNO(rc, rc, -1); } } diff --git a/Userland/Libraries/LibC/unistd.cpp b/Userland/Libraries/LibC/unistd.cpp index beff6b4127..b689e2f02e 100644 --- a/Userland/Libraries/LibC/unistd.cpp +++ b/Userland/Libraries/LibC/unistd.cpp @@ -392,7 +392,7 @@ ssize_t pread(int fd, void* buf, size_t count, off_t offset) { __pthread_maybe_cancel(); - int rc = syscall(SC_pread, fd, buf, count, &offset); + int rc = syscall(SC_pread, fd, buf, count, offset); __RETURN_WITH_ERRNO(rc, rc, -1); } @@ -897,7 +897,7 @@ char* getlogin() // https://pubs.opengroup.org/onlinepubs/9699919799/functions/ftruncate.html int ftruncate(int fd, off_t length) { - int rc = syscall(SC_ftruncate, fd, &length); + int rc = syscall(SC_ftruncate, fd, length); __RETURN_WITH_ERRNO(rc, rc, -1); }