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

["Dmitry V. Levin" <ldv@strace.io>]

On Thu, Jan 09, 2025 at 04:21:39PM +0100, Oleg Nesterov wrote:
> On 01/08, Dmitry V. Levin wrote:
> >
> > +ptrace_set_syscall_info_entry(struct task_struct *child, struct pt_regs *regs,
> > +			      struct ptrace_syscall_info *info)
> > +{
> ...
> > +	syscall_set_nr(child, regs, nr);
> > +	syscall_set_arguments(child, regs, args);
> > +	if (nr == -1) {
> > +		/*
> > +		 * When the syscall number is set to -1, the syscall will be
> > +		 * skipped.  In this case also set the syscall return value to
> > +		 * -ENOSYS, otherwise on some architectures the corresponding
> > +		 * struct pt_regs field will remain unchanged.
> > +		 *
> > +		 * Note that on some architectures syscall_set_return_value()
> > +		 * modifies one of the struct pt_regs fields also modified by
> > +		 * syscall_set_arguments(), so the former should be called
> > +		 * after the latter.
> > +		 */
> > +		syscall_set_return_value(child, regs, -ENOSYS, 0);
> > +	}
> 
> This doesn't look nice to me...
> 
> We don't need this syscall_set_return_value(ENOSYS) on x86, right?

No, we don't need this on x86.

> So perhaps we should move this "if (nr == -1) code  into
> syscall_set_nr/syscall_set_arguments on those "some architectures" which
> actually need it ?

Thanks for the suggestion.  I think the best option is to skip
syscall_set_arguments() invocation in case of nr == -1.  It's not just
pointless, but also it would clobber the syscall return value on those
architectures like arm64 that share the same register both for the first
argument of syscall and its return value.

This is what I'm going to implement for the next iteration of the series:

	syscall_set_nr(child, regs, nr);
	/*
	 * 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.
	 */
	if (nr != -1)
		syscall_set_arguments(child, regs, args);


-- 
ldv