Re: [PATCH 5/6] ptrace: introduce PTRACE_SET_SYSCALL_INFO request

Eugene Syromiatnikov
2025-01-09T15:17:48Z

On Wed, Jan 08, 2025 at 01:04:56AM +0200, Dmitry V. Levin wrote:
> PTRACE_SET_SYSCALL_INFO is a generic ptrace API that complements
> PTRACE_GET_SYSCALL_INFO by letting the ptracer modify details of
> system calls the tracee is blocked in.
>
> This API allows ptracers to obtain and modify system call details
> in a straightforward and architecture-agnostic way.
>
> Current implementation supports changing only those bits of system call
> information that are used by strace, namely, syscall number, syscall
> arguments, and syscall return value.
>
> Support of changing additional details returned by PTRACE_GET_SYSCALL_INFO,
> such as instruction pointer and stack pointer, could be added later
> if needed, by re-using struct ptrace_syscall_info.reserved to specify
> the additional details that should be set. Currently, the reserved
> field of struct ptrace_syscall_info must be initialized with zeroes;
> arch, instruction_pointer, and stack_pointer fields are ignored.
>
> PTRACE_SET_SYSCALL_INFO currently supports only PTRACE_SYSCALL_INFO_ENTRY,
> PTRACE_SYSCALL_INFO_EXIT, and PTRACE_SYSCALL_INFO_SECCOMP operations.
> Other operations could be added later if needed.
>
> Ideally, PTRACE_SET_SYSCALL_INFO should have been introduced along with
> PTRACE_GET_SYSCALL_INFO, but it didn't happen. The last straw that
> convinced me to implement PTRACE_SET_SYSCALL_INFO was apparent failure
> to provide an API of changing the first system call argument on riscv
> architecture.

> index 72c038fc71d0..231b8bf7eeff 100644
> --- a/include/uapi/linux/ptrace.h
> +++ b/include/uapi/linux/ptrace.h
> @@ -74,6 +74,7 @@ struct seccomp_metadata {
> };
>
> #define PTRACE_GET_SYSCALL_INFO 0x420e
> +#define PTRACE_SET_SYSCALL_INFO 0x4212

It seems prudent to also add a comment about 0x4212 being taken right
after "#define PTRACE_GET_SYSCALL_USER_DISPATCH_CONFIG 0x4211", or the
usage of this ptrace request number may be overlooked on the next update.

> struct ptrace_syscall_info {
> __u8 op; /* PTRACE_SYSCALL_INFO_* */
> - __u8 pad[3];
> + __u8 reserved[3];
> __u32 arch;
> __u64 instruction_pointer;
> __u64 stack_pointer;

I would like to suggest adding flags for changing scno and args right
away; while it is possibly of limited use and seems like an unnecessary
overcomplication, at least changing arguments only seems natural to me,
to avoid possible interaction with scno-related shenanigans that might
present/appear on some kernels and/or architectures. Also, it makes
the aforementioned possible extensions of the interface (changing
of ip/sp) more natural (as in those cases users might definitely want
to avoid touching syscall number/arguments).