All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Michal Suchánek" <msuchanek@suse.de>
To: Oleg Nesterov <oleg@redhat.com>
Cc: Renzo Davoli <renzo@cs.unibo.it>,
	linux-kernel@vger.kernel.org,
	Andrew Morton <akpm@linux-foundation.org>,
	Shuah Khan <shuah@kernel.org>, Alexey Gladkov <legion@kernel.org>,
	Eugene Syromyatnikov <evgsyr@gmail.com>,
	Davide Berardi <berardi.dav@gmail.com>,
	strace-devel@lists.strace.io, "Dmitry V . Levin" <ldv@strace.io>,
	Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Subject: Re: [PATCH v7 1/2] ptrace: add PTRACE_SET_SYSCALL_INFO syscall skipping support
Date: Mon, 31 Aug 2026 15:43:58 +0200	[thread overview]
Message-ID: <apWFHkZnCMfxQCEk@kunlun.suse.cz> (raw)
In-Reply-To: <apV_j5GvklbgmjAk@redhat.com>

Hello,

on some architectures the syscall return value overlaps with the syscall
number or syscall arguments - see
https://www.man7.org/linux/man-pages/man2/syscall.2.html

When calling a seccomp filter the filter is able to return a value
which then tells the kernel if the syscall state should be interpreted
as entry (syscall nr + arguments) or exit (syscall skipped, registers
have return value).

ptrace has no way to return a value, and from the register state alone
it is not possible to tell if the syscall state is entry or exit.

Some architectures affected by this overlap problem implement an
architecture-specific thread info flag that tells the kernel that the
syscall is alreaady dispatched. However, this flag is
architecture-specific, and you would need to set it in architecture
independent code.

You could make this flag architecture independent (and even set it to 0
on architectures that are not affected) which would then allow this to
work in general.

Otherwise this API is non-protable, and broken from the start.

On Mon, Aug 31, 2026 at 03:20:15PM +0200, Oleg Nesterov wrote:
> Well, still looks good to me...
> 
> But I forgot (sorry, actually didn't really read) the previous discussions
> and IIRC Michal had some concerns about this patch. And you don't CC him.
> 
> Michal, can you take a look? Do you see anything wrong?
> 
> Oleg.
> 
> On 08/31, Renzo Davoli wrote:
> >
> > Extend PTRACE_SET_SYSCALL_INFO to support skipping a system call triggered
> > via seccomp.
> >
> > When a tracer retrieves a ptrace_syscall_info structure with 'op' set to
> > PTRACE_SYSCALL_INFO_SECCOMP, it can now choose to skip the system call.
> > To do this, the tracer changes 'op' to PTRACE_SYSCALL_INFO_EXIT and
> > populates the exit union fields (rval and is_error) to define the return
> > value and error status for the tracee.
> >
> > System call suppression via PTRACE_SYSCALL_INFO_ENTRY is currently not
> > implemented.  On some architectures (e.g. MIPS), when a system call is
> > skipped by setting the syscall number to -1 at the entry stop, the
> > architecture entry path unconditionally overwrites the return value
> > register with -ENOSYS, clobbering any custom return value set by the
> > tracer at the entry stop.
> >
> > Signed-off-by: Renzo Davoli <renzo@cs.unibo.it>
> > Reviewed-by: Oleg Nesterov <oleg@redhat.com>
> > Reviewed-by: Dmitry V. Levin <ldv@strace.io>
> > ---
> >  kernel/ptrace.c | 27 ++++++++++++++++++++++-----
> >  1 file changed, 22 insertions(+), 5 deletions(-)
> >
> > diff --git a/kernel/ptrace.c b/kernel/ptrace.c
> > index d041645d9d17..64fd1b455297 100644
> > --- a/kernel/ptrace.c
> > +++ b/kernel/ptrace.c
> > @@ -1099,7 +1099,7 @@ ptrace_set_syscall_info_seccomp(struct task_struct *child, struct pt_regs *regs,
> >
> >  static int
> >  ptrace_set_syscall_info_exit(struct task_struct *child, struct pt_regs *regs,
> > -			     struct ptrace_syscall_info *info)
> > +			     struct ptrace_syscall_info *info, bool skip_syscall)
> >  {
> >  	long rval = info->exit.rval;
> >
> > @@ -1111,6 +1111,9 @@ ptrace_set_syscall_info_exit(struct task_struct *child, struct pt_regs *regs,
> >  	if (rval != info->exit.rval)
> >  		return -ERANGE;
> >
> > +	if (skip_syscall)
> > +		syscall_set_nr(child, regs, -1);
> > +
> >  	if (info->exit.is_error)
> >  		syscall_set_return_value(child, regs, rval, 0);
> >  	else
> > @@ -1125,6 +1128,8 @@ ptrace_set_syscall_info(struct task_struct *child, unsigned long user_size,
> >  {
> >  	struct pt_regs *regs = task_pt_regs(child);
> >  	struct ptrace_syscall_info info;
> > +	int op;
> > +	bool skip_syscall = false;
> >
> >  	if (user_size < sizeof(info))
> >  		return -EINVAL;
> > @@ -1141,15 +1146,27 @@ ptrace_set_syscall_info(struct task_struct *child, unsigned long user_size,
> >  	if (info.flags || info.reserved)
> >  		return -EINVAL;
> >
> > -	/* Changing the type of the system call stop is not supported yet. */
> > -	if (ptrace_get_syscall_info_op(child) != info.op)
> > -		return -EINVAL;
> > +	/*
> > +	 * Changing the type of the system call stop is not allowed, with the
> > +	 * following exception:
> > +	 * PTRACE_SYSCALL_INFO_SECCOMP can be changed to PTRACE_SYSCALL_INFO_EXIT
> > +	 * to skip the system call
> > +	 */
> > +
> > +	op = ptrace_get_syscall_info_op(child);
> > +	if (op != info.op) {
> > +		if (info.op == PTRACE_SYSCALL_INFO_EXIT &&
> > +				op == PTRACE_SYSCALL_INFO_SECCOMP)
> > +			skip_syscall = true;
> > +		else
> > +			return -EINVAL;
> > +	}
> >
> >  	switch (info.op) {
> >  	case PTRACE_SYSCALL_INFO_ENTRY:
> >  		return ptrace_set_syscall_info_entry(child, regs, &info);
> >  	case PTRACE_SYSCALL_INFO_EXIT:
> > -		return ptrace_set_syscall_info_exit(child, regs, &info);
> > +		return ptrace_set_syscall_info_exit(child, regs, &info, skip_syscall);

Here it might be preferable to define a new function that does the
skip=true part and calls ptrace_set_syscall_info_exit.

Looking at the code it would not be the first.

Thanks

Michal

> >  	case PTRACE_SYSCALL_INFO_SECCOMP:
> >  		return ptrace_set_syscall_info_seccomp(child, regs, &info);
> >  	default:
> > --
> > 2.55.0
> >
> 

  reply	other threads:[~2026-08-31 13:44 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-31  8:27 [PATCH v7 0/2] PTRACE_SET_SYSCALL_INFO: add support for seccomp syscall skipping Renzo Davoli
2026-08-31  8:27 ` [PATCH v7 1/2] ptrace: add PTRACE_SET_SYSCALL_INFO syscall skipping support Renzo Davoli
2026-08-31 13:20   ` Oleg Nesterov
2026-08-31 13:43     ` Michal Suchánek [this message]
2026-08-31  8:27 ` [PATCH v7 2/2] selftests/ptrace: add a test case for PTRACE_SET_SYSCALL_INFO syscall skipping Renzo Davoli

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=apWFHkZnCMfxQCEk@kunlun.suse.cz \
    --to=msuchanek@suse.de \
    --cc=akpm@linux-foundation.org \
    --cc=berardi.dav@gmail.com \
    --cc=evgsyr@gmail.com \
    --cc=ldv@strace.io \
    --cc=legion@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=oleg@redhat.com \
    --cc=renzo@cs.unibo.it \
    --cc=shuah@kernel.org \
    --cc=strace-devel@lists.strace.io \
    --cc=tsbogend@alpha.franken.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.