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

[Eugene Syromiatnikov <esyr@redhat.com>]

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).