Re: [PATCH v2 1/2] ptrace: PTRACE_SET_SYSCALL_INFO syscall skipping support

[Oleg Nesterov <oleg@redhat.com>]

Oh... I know nothing about mips.

Add Thomas. Thomas could you help? See the question below.

OK, lets only allow the _SECCOMP -> _EXIT transition for now.
But will it work on MIPS?

grep, grep... So arch/mips/ has

	static inline void syscall_set_return_value(struct task_struct *task,
						    struct pt_regs *regs,
						    int error, long val)
	{
		if (error) {
			regs->regs[2] = -error;
			regs->regs[7] = 1;
		} else {
			regs->regs[2] = val;
			regs->regs[7] = 0;
		}
	}

	static inline void syscall_set_nr(struct task_struct *task,
					  struct pt_regs *regs,
					  int nr)
	{
		/*
		 * New syscall number has to be assigned to regs[2] because
		 * it is loaded from there unconditionally after return from
		 * syscall_trace_enter() invocation.
		 *
		 * Consequently, if the syscall was indirect and nr != __NR_syscall,
		 * then after this assignment the syscall will cease to be indirect.
		 */
		task_thread_info(task)->syscall = regs->regs[2] = nr;
	}

I have no idea. But at least ptrace_set_syscall_info_exit(skip => true)
must do syscall_set_nr(-1) before syscall_set_return_value(), otherwise
the value assigned to regs[2] will be lost.

-------------------------------------------------------------------------------
Now the question. To simplify, suppose we need something like

	void skip_syscall_and_set_return_value(task, regs, retval)
	{
		syscall_set_nr(task, regs, -1);
		syscall_set_return_value(task, regs, 0, retval);
	}

which can be used by debugger when the tracee sleeps in ptrace_report_syscall_entry().

However, arch/mips/kernel/ptrace.c:syscall_trace_enter() does:

	ptrace_report_syscall_entry(regs);

	if (current_thread_info()->syscall < 0)
		syscall_set_return_value(current, regs, -ENOSYS, 0);

and this means that this func won't work on MIPS. Is it possible to make it
work somehow? May be we can abuse regs->regs[7] somehow to detect the case when
syscall_set_return_value() was called by debugger and avoid the unconditional
-ENOSYS ?

Oleg.

On 07/05, Renzo Davoli wrote:
>
> There is a problem on MIPS:
> https://sashiko.dev/#/patchset/20260704142643.692754-1-renzo%40cs.unibo.it
>
> It appears that on MIPS the feature of skipping a system call by setting its
> number to -1 does not work correctly when transitioning from _ENTRY to _EXIT:
> the system call return value is overwritten.
>
> PTRACE_EVENT_SECCOMP, however, has an explicit UAPI specification stating that
> setting the system call number to a negative value suppresses the system call.
>
> Moreover, kernel/ptrace.c contains the following comment:
> /*
>  * If the syscall number is set to -1, setting syscall arguments is not
>  * just pointless, it would also clobber the syscall return value on
>  * those architectures that share the same register both for the first
>  * argument of syscall and its return value.
>  */
> Thus, PTRACE_EVENT_SECCOMP is explicitly designed to preserve the system call
> return value when the system call is skipped.
>
> By contrast, for PTRACE_SYSCALL syscall-entry stops, the man page only states
> that the tracer may modify the system call number. It does not specify that
> assigning a negative value must suppress the system call and preserve the
> return value across all architectures, even though many architectures implement
> exactly this behavior.
>
> At this point I see two possible approaches:
>
> * fix the MIPS implementation (and audit, and possibly fix, the other
>   architectures as well);
>
> * revert to the original proposal and allow the "skip syscall" feature only for
>   PTRACE_EVENT_SECCOMP, i.e. permit PTRACE_SET_SYSCALL_INFO to transform only
>   PTRACE_SYSCALL_INFO_SECCOMP stops into PTRACE_SYSCALL_INFO_EXIT stops.
>
> I would prefer the latter approach. I am concerned that changing the ptrace
> implementation in each architecture may introduce subtle regressions or other
> unintended side effects.
>
> In my opinion, seccomp-based syscall tracing is also the more powerful and
> flexible model compared to the traditional PTRACE_SYSCALL entry/exit mechanism.
>
> Support for system call suppression from PTRACE_SYSCALL_INFO_ENTRY can always
> be added later if and when a real use case arises. That would also provide an
> opportunity to audit the behavior of all supported architectures and, if
> necessary, make the semantics of negative system call numbers consistent across
> architectures.
>
> 	renzo
>