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:
> > 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 <[email protected]>
> > Cc: Mukesh Kumar Chaurasiya (IBM) <[email protected]>
> > Cc: Shrikanth Hegde <[email protected]>
> > Cc: Thomas Gleixner <[email protected]>
> > Cc: [email protected] # v7.3
> > Assisted-by: claude-opus-5-5
> > Signed-off-by: Renzo Davoli <[email protected]>
> > ---
> > 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/[email protected]
> >
> > 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
