From 3e223185b3b6bf96b0b86a3be0ad626193ef85d4 Mon Sep 17 00:00:00 2001 From: Ben Wiederhake Date: Thu, 25 Nov 2021 20:15:02 +0100 Subject: [PATCH] Kernel+strace: Remove unnecessary indirection for PEEK Also, remove incomplete, superfluous check. Incomplete, because only the byte at the provided address was checked; this misses the last bytes of the "jerk page". Superfluous, because it is already correctly checked by peek_user_data (which calls copy_from_user). The caller/tracer should not typically attempt to read non-userspace addresses, we don't need to "hot-path" it either. --- Kernel/API/Syscall.h | 5 ----- Kernel/Syscalls/ptrace.cpp | 16 ++++------------ Userland/Libraries/LibC/sys/ptrace.cpp | 5 +---- 3 files changed, 5 insertions(+), 21 deletions(-) diff --git a/Kernel/API/Syscall.h b/Kernel/API/Syscall.h index 019a471db9..63d3050e2f 100644 --- a/Kernel/API/Syscall.h +++ b/Kernel/API/Syscall.h @@ -465,11 +465,6 @@ struct SC_ptrace_params { FlatPtr data; }; -struct SC_ptrace_peek_params { - const void* address; - FlatPtr* out_data; -}; - struct SC_set_coredump_metadata_params { StringArgument key; StringArgument value; diff --git a/Kernel/Syscalls/ptrace.cpp b/Kernel/Syscalls/ptrace.cpp index adfcef8f70..9547725a33 100644 --- a/Kernel/Syscalls/ptrace.cpp +++ b/Kernel/Syscalls/ptrace.cpp @@ -114,26 +114,18 @@ static ErrorOr handle_ptrace(const Kernel::Syscall::SC_ptrace_params& p } case PT_PEEK: { - Kernel::Syscall::SC_ptrace_peek_params peek_params {}; - TRY(copy_from_user(&peek_params, reinterpret_cast(params.addr))); - if (!Memory::is_user_address(VirtualAddress { peek_params.address })) - return EFAULT; - auto data = TRY(peer->process().peek_user_data(Userspace { (FlatPtr)peek_params.address })); - TRY(copy_to_user(peek_params.out_data, &data)); + auto data = TRY(peer->process().peek_user_data(Userspace { (FlatPtr)params.addr })); + TRY(copy_to_user((FlatPtr*)params.data, &data)); break; } case PT_POKE: - if (!Memory::is_user_address(VirtualAddress { params.addr })) - return EFAULT; TRY(peer->process().poke_user_data(Userspace { (FlatPtr)params.addr }, params.data)); return 0; case PT_PEEKDEBUG: { - Kernel::Syscall::SC_ptrace_peek_params peek_params {}; - TRY(copy_from_user(&peek_params, reinterpret_cast(params.addr))); - auto data = TRY(peer->peek_debug_register(reinterpret_cast(peek_params.address))); - TRY(copy_to_user(peek_params.out_data, &data)); + auto data = TRY(peer->peek_debug_register(reinterpret_cast(params.addr))); + TRY(copy_to_user((FlatPtr*)params.data, &data)); break; } case PT_POKEDEBUG: diff --git a/Userland/Libraries/LibC/sys/ptrace.cpp b/Userland/Libraries/LibC/sys/ptrace.cpp index e20f4ce1eb..61c3e41e12 100644 --- a/Userland/Libraries/LibC/sys/ptrace.cpp +++ b/Userland/Libraries/LibC/sys/ptrace.cpp @@ -18,12 +18,9 @@ long ptrace(int request, pid_t tid, void* addr, void* data) // by looking at errno rather than the return value. FlatPtr out_data; - Syscall::SC_ptrace_peek_params peek_params; auto is_peek_type = request == PT_PEEK || request == PT_PEEKDEBUG; if (is_peek_type) { - peek_params.address = reinterpret_cast(addr); - peek_params.out_data = &out_data; - addr = &peek_params; + data = &out_data; } Syscall::SC_ptrace_params params {