mirror of https://lore.kernel.org/lkml/
 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®