Re: [PATCH v2 1/1] powerpc/syscall: Return the preset value of skipped syscalls

On Tue, Oct 27, 2026 at 11:19:42AM +0100, Michal Suchánek wrote:

Hello,

On Fri, Oct 23, 2026 at 08:07:32PM +0200, Renzo Davoli wrote:

quoted

When ptrace or seccomp skip a system call, the return value they set in
regs->gpr[3] gets lost or the tracer is affected, since two recent
changes:

1. Commit 05c033db7e9a ("entry, treewide: Make
   syscall_enter_from_user_mode[_work]() indicate syscall execution")
   made system_call_exception() return syscall_get_error() for a skipped
   system call.  syscall_get_error() is 0 for any non-error value, so a
   success value set by a tracer at a PTRACE_EVENT_SECCOMP stop (e.g.
   with PTRACE_POKEUSER), or by a SECCOMP_RET_USER_NOTIF listener, is
   replaced with 0.

2. Commit 69cb2be898be ("powerpc/syscall: Fix syscall skip handling for
   seccomp and ptrace") made syscall_set_return_value() set
   TIF_SYSCALL_RET with set_thread_flag(), i.e. on current rather than on
   the task passed in.  When the caller is a tracer, as in
   PTRACE_SET_SYSCALL_INFO with op == PTRACE_SYSCALL_INFO_EXIT, the flag
   ends up on the tracer, and the next system call of the tracer is
   silently not executed.  Setting it on the right task would not be
   enough either: at a syscall-exit stop the flag would then skip the
   next system call of the tracee.

TIF_SYSCALL_RET is not needed anymore: after the change described in 1.
the generic entry code tells whether ptrace or seccomp skipped the
system call, and every in-kernel caller of syscall_set_return_value() on
the syscall entry path (SECCOMP_RET_ERRNO, SECCOMP_RET_TRACE without a
tracer, SECCOMP_RET_USER_NOTIF) also makes it return false.  A user
calling syscall(-1) without tracer or seccomp still gets -ENOSYS.

Remove the flag and return regs->gpr[3] for a skipped system call, as
done before the conversion to GENERIC_ENTRY.  The syscall exit path
already converts a negative error value to the "sc" ABI, and
syscall_set_return_value() keeps CR0.SO in sync for positive error
values.

Fixes: 69cb2be898be ("powerpc/syscall: Fix syscall skip handling for seccomp and ptrace")
Fixes: 05c033db7e9a ("entry, treewide: Make syscall_enter_from_user_mode[_work]() indicate syscall execution")
Cc: Michal Suchánek <redacted>
Cc: Mukesh Kumar Chaurasiya (IBM) <redacted>
Cc: Shrikanth Hegde <sshegde@linux.ibm.com>
Cc: Thomas Gleixner <tglx@kernel.org>
Cc: <redacted> # v7.3
Assisted-by: claude-opus-5-5
Signed-off-by: Renzo Davoli <redacted>
---
How this was found and tested: QEMU pseries (TCG, -cpu power9), ppc64le
guest config, v7.3-rc6 (7b63ef2d55f2), with a small reproducer
(ppc_skip_repro.c, available on request) which

 - sets the getpid() return value at a syscall-exit stop with
   PTRACE_SET_SYSCALL_INFO, then makes a syscall in the tracer;
 - skips getpid() at a PTRACE_EVENT_SECCOMP stop with PTRACE_POKEUSER
   (r0 = -1, r3/CR0.SO = return value), with both "sc" and "scv 0".

                                             v7.3-rc6      with this patch
  tracer syscall after SET_SYSCALL_INFO      returns 0     correct
  skipped syscall, success value, sc/scv     returns 0     correct
  skipped syscall, error value, sc/scv       correct       correct
  selftests/ptrace set_syscall_info test    FAIL          pass
  selftests/seccomp/seccomp_bpf              11 failures   all pass (*)

  failing in v7.3-rc6: user_notification_{basic,child_pid_ns,
  sibling_pid_ns,fault_recv,addfd,addfd_rlimit,sync,fifo,wait_killable},
  TRACE_syscall.seccomp.syscall_faked, plus the timing-only one below.

(*) user_notification_wait_killable_after_reply (100000 notifications
    with a 1 ms itimer) can hit the 30 s harness timeout under emulation,
    with and without this patch.

Build-tested with W=1: ppc64le guest config, ppc64_defconfig (BE),
pmac32_defconfig.

Changes in v2:
- Rebased on current mainline (734bef548a96), which now contains
  commit 66236558dfac ("powerpc/syscall: Avoid atomic
  test_and_clear_thread_flag on fast path").  That commit folded the
  TIF_SYSCALL_RET check into the skip condition, so the v1 hunk in
  system_call_exception() no longer applied.  The resulting code is the
  same as with v1.
- Link to v1:
  https://lore.kernel.org/lkml/20261018151539.440213-1-renzo@cs.unibo.it (local)

 arch/powerpc/include/asm/syscall.h     |  6 ------
 arch/powerpc/include/asm/thread_info.h |  1 -
 arch/powerpc/kernel/syscall.c          | 13 ++++++++-----
 3 files changed, 8 insertions(+), 12 deletions(-)

diff --git a/arch/powerpc/include/asm/syscall.h b/arch/powerpc/include/asm/syscall.h
index 32be22b995cc..4b3c52ed6e9d 100644
--- a/arch/powerpc/include/asm/syscall.h
+++ b/arch/powerpc/include/asm/syscall.h

@@ -98,12 +98,6 @@ static inline void syscall_set_return_value(struct task_struct *task,
 			regs->gpr[3] = val;
 		}
 	}
-	/*
-	 * Mark that a return value has been explicitly set by seccomp or
-	 * ptrace so that system_call_exception() can skip the syscall
-	 * unconditionally, even when the user requested syscall(-1).
-	 */
-	set_thread_flag(TIF_SYSCALL_RET);
 }
 
 static inline void syscall_get_arguments(struct task_struct *task,

diff --git a/arch/powerpc/include/asm/thread_info.h b/arch/powerpc/include/asm/thread_info.h
index 1e069a2e7ce8..0487e94d3416 100644
--- a/arch/powerpc/include/asm/thread_info.h
+++ b/arch/powerpc/include/asm/thread_info.h

@@ -120,7 +120,6 @@ void arch_setup_new_exec(void);
 #endif
 #define TIF_POLLING_NRFLAG	19	/* true if poll_idle() is polling TIF_NEED_RESCHED */
 #define TIF_32BIT		20	/* 32 bit binary */
-#define TIF_SYSCALL_RET		21	/* syscall error value set */
 
 /* as above, but as bit values */
 #define _TIF_SYSCALL_TRACE	(1<<TIF_SYSCALL_TRACE)

diff --git a/arch/powerpc/kernel/syscall.c b/arch/powerpc/kernel/syscall.c
index 8529827d697f..52ceff4ef41b 100644
--- a/arch/powerpc/kernel/syscall.c
+++ b/arch/powerpc/kernel/syscall.c

@@ -18,11 +18,14 @@ notrace long system_call_exception(struct pt_regs *regs, unsigned long r0)
 	long ret;
 	syscall_fn f;
 
-	if (unlikely(!syscall_enter_from_user_mode_randomize_stack(regs, &r0) ||
-			test_thread_flag(TIF_SYSCALL_RET))) {
-		clear_thread_flag(TIF_SYSCALL_RET);
-		return syscall_get_error(current, regs);
-	}
+	/*
+	 * If ptrace or seccomp decided to skip the syscall, regs->gpr[3] holds
+	 * the return value they set (or the unmodified first argument, e.g.
+	 * for PTRACE_SYSEMU). Return it as is, the exit path takes care of
+	 * the error encoding for the "sc" ABI.
+	 */
+	if (unlikely(!syscall_enter_from_user_mode_randomize_stack(regs, &r0)))
+		return regs->gpr[3];
 
 	if (unlikely(r0 >= NR_syscalls)) {
 		if (unlikely(trap_is_unsupported_scv(regs))) {

This does two things that should be addressed separately:

 - eliminating the TIF_SYSCALL_RET flag

The previous refactor aimed at removing this flag. If the kernel still
builds when it's removed removing the flag and the branch that tests it
is merely dead code eliminaation unless there is something creative
going on somewhere.

 - replacing syscall_get_error() with regs->gpr[3]

This is a very dubious change. If this does fix something this is likely
papering over some place that sets the return value incorrectly.
Instead of this change the place where the incorrect setting of the
return value happens should be fixed.

Thanks

Michal

I agree with Michal here.

what do you think about this diff

diff --git a/arch/powerpc/include/asm/syscall.h b/arch/powerpc/include/asm/syscall.h
index 32be22b995cc..4b3c52ed6e9d 100644
--- a/arch/powerpc/include/asm/syscall.h
+++ b/arch/powerpc/include/asm/syscall.h

@@ -98,12 +98,6 @@ static inline void syscall_set_return_value(struct task_struct *task,
 			regs->gpr[3] = val;
 		}
 	}
-	/*
-	 * Mark that a return value has been explicitly set by seccomp or
-	 * ptrace so that system_call_exception() can skip the syscall
-	 * unconditionally, even when the user requested syscall(-1).
-	 */
-	set_thread_flag(TIF_SYSCALL_RET);
 }
 
 static inline void syscall_get_arguments(struct task_struct *task,

diff --git a/arch/powerpc/include/asm/thread_info.h b/arch/powerpc/include/asm/thread_info.h
index 4df3f4b5bd56..f4f35314a901 100644
--- a/arch/powerpc/include/asm/thread_info.h
+++ b/arch/powerpc/include/asm/thread_info.h

@@ -110,7 +110,6 @@ void arch_setup_new_exec(void);
 #endif
 #define TIF_POLLING_NRFLAG	22	/* true if poll_idle() is polling TIF_NEED_RESCHED */
 #define TIF_32BIT		23	/* 32 bit binary */
-#define TIF_SYSCALL_RET		24	/* syscall error value set */
 
 /* as above, but as bit values */
 #define _TIF_POLLING_NRFLAG	(1<<TIF_POLLING_NRFLAG)

diff --git a/arch/powerpc/kernel/syscall.c b/arch/powerpc/kernel/syscall.c
index fbefe1927b10..ec81046e9eb5 100644
--- a/arch/powerpc/kernel/syscall.c
+++ b/arch/powerpc/kernel/syscall.c

@@ -19,13 +19,17 @@ notrace long system_call_exception(struct pt_regs *regs, unsigned long r0)
 	syscall_fn f;
 
 	if (unlikely(!syscall_enter_from_user_mode_randomize_stack(regs, &r0))) {
-		clear_thread_flag(TIF_SYSCALL_RET);
-		return syscall_get_error(current, regs);
+		/*
+		 * The syscall was skipped by seccomp or ptrace, which have
+		 * already stored the result in the user ABI form via
+		 * syscall_set_return_value(). Hand it back in the form
+		 * syscall_exit_prepare() expects: -errno on failure, else the
+		 * value in r3. Using syscall_get_error() alone would drop a
+		 * successful value, e.g. from SECCOMP_USER_NOTIF.
+		 */
+		return syscall_get_error(current, regs) ?: regs->gpr[3];
 	}
 
-	if (unlikely(test_and_clear_thread_flag(TIF_SYSCALL_RET)))
-		return syscall_get_error(current, regs);
-
 	if (unlikely(r0 >= NR_syscalls)) {
 		if (unlikely(trap_is_unsupported_scv(regs))) {
 			/* Unsupported scv vector */

Regards,
Mukesh