Re: git: 48a656c588f9 - main - linux: Fix usage of ptrace(PT_GET_SC_ARGS)

On Fri, Jun 13, 2025 at 07:20:14PM +0000, Mark Johnston wrote:
> The branch main has been updated by markj:
> 
> URL: https://cgit.FreeBSD.org/src/commit/?id=48a656c588f9fb995b9c524b57dd5febd9f69168
> 
> commit 48a656c588f9fb995b9c524b57dd5febd9f69168
> Author:     Mark Johnston <markj@FreeBSD.org>
> AuthorDate: 2025-06-13 19:03:03 +0000
> Commit:     Mark Johnston <markj@FreeBSD.org>
> CommitDate: 2025-06-13 19:19:59 +0000
> 
>     linux: Fix usage of ptrace(PT_GET_SC_ARGS)
>     
>     The native handler expects the argument to be a pointer to an array of 8
>     syscall arguments, whereas the emulation provided an array that holds up
>     to 6.
>     
>     Handle this by adding a new range of Linuxulator-specific ptrace
>     commands.  In particular, introduce PTLINUX_GET_SC_ARGS, which always
>     copies exactly six arguments.  This fixes the problem and removes the
>     hack of checking the target thread ABI to decide whether to apply a
>     Linux-specific quirk to PT_GET_SC_ARGS.
>     
>     Reviewed by:    kib
>     MFC after:      2 weeks
>     Sponsored by:   Klara, Inc.
>     Differential Revision:  https://reviews.freebsd.org/D50758
> ---
>  sys/compat/freebsd32/freebsd32_misc.c |  3 +++
>  sys/compat/linux/linux_ptrace.c       | 18 ++++--------------
>  sys/kern/sys_process.c                | 27 +++++++++++++++++++++------
>  sys/sys/ptrace.h                      |  8 ++++++++
>  4 files changed, 36 insertions(+), 20 deletions(-)
> 
> diff --git a/sys/compat/freebsd32/freebsd32_misc.c b/sys/compat/freebsd32/freebsd32_misc.c
> index cc777f2bf830..75fdb1f544ca 100644
> --- a/sys/compat/freebsd32/freebsd32_misc.c
> +++ b/sys/compat/freebsd32/freebsd32_misc.c
> @@ -1177,6 +1177,9 @@ freebsd32_ptrace(struct thread *td, struct freebsd32_ptrace_args *uap)
>  			pscr_args[i] = pscr_args32[i];
>  		r.sr.pscr_args = pscr_args;
>  		break;
> +	case PTLINUX_FIRST ... PTLINUX_LAST:
> +		error = EINVAL;
> +		break;
>  	default:
>  		addr = uap->addr;
>  		break;
> diff --git a/sys/compat/linux/linux_ptrace.c b/sys/compat/linux/linux_ptrace.c
> index 421760eab2a9..0dbfd2177122 100644
> --- a/sys/compat/linux/linux_ptrace.c
> +++ b/sys/compat/linux/linux_ptrace.c
> @@ -386,21 +386,11 @@ linux_ptrace_get_syscall_info(struct thread *td, pid_t pid,
>  	if (lwpinfo.pl_flags & PL_FLAG_SCE) {
>  		si.op = LINUX_PTRACE_SYSCALL_INFO_ENTRY;
>  		si.entry.nr = lwpinfo.pl_syscall_code;
> -		/*
> -		 * The use of PT_GET_SC_ARGS there is special,
> -		 * implementation of PT_GET_SC_ARGS for Linux-ABI
> -		 * callers emulates Linux bug which strace(1) depends
> -		 * on: at initialization it tests whether ptrace works
> -		 * by calling close(2), or some other single-argument
> -		 * syscall, _with six arguments_, and then verifies
> -		 * whether it can fetch them all using this API;
> -		 * otherwise it bails out.
> -		 */
> -		error = kern_ptrace(td, PT_GET_SC_ARGS, pid,
> -		    &si.entry.args, sizeof(si.entry.args));
> +		error = kern_ptrace(td, PTLINUX_GET_SC_ARGS, pid,
> +		    si.entry.args, sizeof(si.entry.args));
>  		if (error != 0) {
> -			linux_msg(td, "PT_GET_SC_ARGS failed with error %d",
> -			    error);
> +			linux_msg(td,
> +			    "PT_LINUX_GET_SC_ARGS failed with error %d", error);
>  			return (error);
>  		}
>  	} else if (lwpinfo.pl_flags & PL_FLAG_SCX) {
> diff --git a/sys/kern/sys_process.c b/sys/kern/sys_process.c
> index 8b382cb3048e..821c537065b3 100644
> --- a/sys/kern/sys_process.c
> +++ b/sys/kern/sys_process.c
> @@ -690,6 +690,9 @@ sys_ptrace(struct thread *td, struct ptrace_args *uap)
>  			break;
>  		r.sr.pscr_args = pscr_args;
>  		break;
> +	case PTLINUX_FIRST ... PTLINUX_LAST:
> +		error = EINVAL;
> +		break;
>  	default:
>  		addr = uap->addr;
>  		break;
> @@ -1166,7 +1169,9 @@ kern_ptrace(struct thread *td, int req, pid_t pid, void *addr, int data)
>  		break;
>  
>  	case PT_GET_SC_ARGS:
> -		CTR1(KTR_PTRACE, "PT_GET_SC_ARGS: pid %d", p->p_pid);
> +	case PTLINUX_GET_SC_ARGS:
> +		CTR2(KTR_PTRACE, "%s: pid %d", req == PT_GET_SC_ARGS ?
> +		    "PT_GET_SC_ARGS" : "PT_LINUX_GET_SC_ARGS", p->p_pid);
>  		if (((td2->td_dbgflags & (TDB_SCE | TDB_SCX)) == 0 &&
>  		     td2->td_sa.code == 0)
>  #ifdef COMPAT_FREEBSD32
> @@ -1176,11 +1181,21 @@ kern_ptrace(struct thread *td, int req, pid_t pid, void *addr, int data)
>  			error = EINVAL;
>  			break;
>  		}
> -		bzero(addr, sizeof(td2->td_sa.args));
> -		/* See the explanation in linux_ptrace_get_syscall_info(). */
> -		bcopy(td2->td_sa.args, addr, SV_PROC_ABI(td->td_proc) ==
> -		    SV_ABI_LINUX ? sizeof(td2->td_sa.args) :
> -		    td2->td_sa.callp->sy_narg * sizeof(syscallarg_t));
> +		if (req == PT_GET_SC_ARGS) {
> +			bzero(addr, sizeof(td2->td_sa.args));
> +			bcopy(td2->td_sa.args, addr, td2->td_sa.callp->sy_narg *
> +			    sizeof(syscallarg_t));
> +		} else {
> +			/*
> +			 * Emulate a Linux bug which which strace(1) depends on:
> +			 * at initialization it tests whether ptrace works by
> +			 * calling close(2), or some other single-argument
> +			 * syscall, _with six arguments_, and then verifies
> +			 * whether it can fetch them all using this API;
> +			 * otherwise it bails out.
> +			 */
> +			bcopy(td2->td_sa.args, addr, 6 * sizeof(syscallarg_t));
> +		}
>  		break;
>  
>  	case PT_GET_SC_RET:
> diff --git a/sys/sys/ptrace.h b/sys/sys/ptrace.h
> index 3a7b87bfc85f..13291cd31cf5 100644
> --- a/sys/sys/ptrace.h
> +++ b/sys/sys/ptrace.h
> @@ -87,8 +87,16 @@
>  #define	PT_SC_REMOTE	44	/* Execute a syscall */
>  
>  #define PT_FIRSTMACH    64	/* for machine-specific requests */
> +#define	PT_LASTMACH     127
>  #include <machine/ptrace.h>	/* machine-specific requests, if any */
>  
> +#ifdef _KERNEL
> +/* Space for Linux ptrace emulation. */
> +#define	PTLINUX_FIRST	128
As an after-thought, I think it is better to name them like
PTINTERNAL_FIRST/LAST.

> +#define	PTLINUX_LAST	191
> +#define	PTLINUX_GET_SC_ARGS	(PTLINUX_FIRST + 0)
> +#endif
> +
>  /* Events used with PT_GET_EVENT_MASK and PT_SET_EVENT_MASK */
>  #define	PTRACE_EXEC	0x0001
>  #define	PTRACE_SCE	0x0002