linux-kernel - Re: [PATCH 5/6] ptrace: introduce PTRACE_SET_SYSCALL

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20250111114925.GA8306@strace.io>
Date: Sat, 11 Jan 2025 13:49:25 +0200
From: "Dmitry V. Levin" <ldv@...ace.io>
To: Oleg Nesterov <oleg@...hat.com>
Cc: Eugene Syromyatnikov <evgsyr@...il.com>,
	Mike Frysinger <vapier@...too.org>,
	Renzo Davoli <renzo@...unibo.it>,
	Davide Berardi <berardi.dav@...il.com>,
	strace-devel@...ts.strace.io, linux-kernel@...r.kernel.org,
	linux-api@...r.kernel.org
Subject: Re: [PATCH 5/6] ptrace: introduce PTRACE_SET_SYSCALL_INFO request

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