[PATCH 1/2] powerpc: Restore all registers after a traced syscall again

[PATCH 1/2] powerpc: Restore all registers after a traced syscall again

[Michal Suchanek]

From: Bernhard M. Wiedemann <bwiedemann@suse.de>

syscall_exit_prepare() asks the exit assembly to reload all GPRs, CTR
and XER from pt_regs only if the thread flags contain any of the flags
in _TIF_SYSCALL_DOTRACE. With the new entry code based GENERIC_ENTRY the
flags are no longer set.

This breaks tracers that modify a stopped syscall, for example ones that
re-execute the syscall instruction by moving the NIP back by 4 at the
syscall-exit stop with the syscall number put back into r0: the sc then
runs with r0 = 0, which is __NR_restart_syscall. dettrace fails like
this with "No filter rule for system call: restart_syscall". A tracer
that writes r4 at the syscall-exit stop of an sc sees the tracee
continue with r4 = 0 instead; with a 7.1 kernel both work.

Check the equivalent syscall_work bits instead, and drop the now unused
_TIF_SYSCALL_DOTRACE.

Fixes: bee25f97ad24 ("powerpc: Enable GENERIC_ENTRY feature")
Cc: stable@vger.kernel.org # 7.2.x: c7585b8e99ad: powerpc: Don't drop _TIF_RESTOREALL on syscall restart
Cc: stable@vger.kernel.org # 7.2.x
Assisted-by: LLM
Signed-off-by: Bernhard M. Wiedemann <bwiedemann@suse.de>
[ms: reword commit message]
Signed-off-by: Michal Suchánek <msuchanek@suse.de>
---
A backport also needs commit c7585b8e99ad ("powerpc: Don't drop
_TIF_RESTOREALL on syscall restart"), without which the flag can still
be lost.

It has been pointed out that the newly defined
PPC_SYSCALL_WORK_RESTOREALL is almost the same as SYSCALL_WORK_ENTER and
the existing define might be used instead. However, it is not clear if
this is expected or if this is merely coincidence, and not guaranteed.

Testing: a small tracer (pokes r4 at the syscall-exit stop of an sc,
and replays a syscall by moving NIP back with a new r0) shows both
writes lost on a ppc64le and both working on 7.1.8.
------------[ cut here ]------------
// SPDX-License-Identifier: GPL-2.0
/*
 * Does a register a tracer writes at the syscall-exit stop of an "sc"
 * reach the tracee? Checks r4, and a replay: NIP moved back onto the sc
 * with r0 set to another syscall number.
 * Build: gcc -O2 -o ppc-exitregs ppc-exitregs.c (powerpc64 only)
 */

/* word offsets in struct pt_regs, arch/powerpc/include/uapi/asm/ptrace.h */

static long do_sc(long nr, long *r4io)
{
	register long r0 __asm__("r0") = nr;
	register long r3 __asm__("r3") = 0;
	register long r4 __asm__("r4") = *r4io;

	__asm__ volatile("sc"
			 : "+r"(r0), "+r"(r3), "+r"(r4)
			 :
			 : "r5", "r6", "r7", "r8", "r9", "r10", "r11", "r12",
			   "cr0", "ctr", "xer", "memory");
	*r4io = r4;
	return r3;
}

static long peek(pid_t pid, int w)
{
	errno = 0;
	long v = ptrace(PTRACE_PEEKUSER, pid, (void *)(w * sizeof(long)), 0);
	if (errno) {
		perror("PEEKUSER");
		exit(2);
	}
	return v;
}

static void poke(pid_t pid, int w, long v)
{
	if (ptrace(PTRACE_POKEUSER, pid, (void *)(w * sizeof(long)), (void *)v)) {
		perror("POKEUSER");
		exit(2);
	}
}

/* Resume to the next syscall stop, which must be one. */
static void next_stop(pid_t pid)
{
	int st;

	ptrace(PTRACE_SYSCALL, pid, 0, 0);
	if (waitpid(pid, &st, 0) != pid || !WIFSTOPPED(st) ||
	    WSTOPSIG(st) != (SIGTRAP | 0x80)) {
		fprintf(stderr, "unexpected wait status %#x\n", st);
		exit(2);
	}
}

int main(void)
{
	pid_t pid = fork();
	int st, fail = 0;

	if (pid == 0) {
		long r4 = 0x1111, r4a, ret;

		ptrace(PTRACE_TRACEME, 0, 0, 0);
		raise(SIGSTOP);
		do_sc(__NR_getpid, &r4);
		r4a = r4;
		ret = do_sc(__NR_getpid, &r4);
		/* bit 0: the tracer's r4 was lost; bit 1: the replay of
		 * the second getpid as getppid returned something else */
		_exit((r4a != 0x2222) | (ret != getppid()) << 1);
	}

	waitpid(pid, &st, 0);
	ptrace(PTRACE_SETOPTIONS, pid, 0, (void *)PTRACE_O_TRACESYSGOOD);

	/* 1: write r4 at the exit stop of getpid */
	next_stop(pid);				/* entry */
	next_stop(pid);				/* exit */
	poke(pid, W_R4, 0x2222);
	next_stop(pid);				/* entry of the second getpid */

	/* 2: replay the second getpid as getppid */
	next_stop(pid);				/* exit */
	poke(pid, W_NIP, peek(pid, W_NIP) - 4);
	poke(pid, W_R0, __NR_getppid);
	next_stop(pid);				/* entry of the replay */
	long nr = peek(pid, W_R0);
	printf("replayed syscall arrives as nr %ld (%s)\n", nr,
	       nr == __NR_getppid ? "getppid, OK" :
	       nr == __NR_restart_syscall ? "restart_syscall, BUG" : "?");
	if (nr != __NR_getppid) {
		fail = 1;
		poke(pid, W_R0, __NR_getppid);	/* entry writes do work */
	}
	next_stop(pid);				/* exit */
	ptrace(PTRACE_CONT, pid, 0, 0);
	waitpid(pid, &st, 0);
	if (!WIFEXITED(st)) {
		printf("child did not exit normally: %#x\n", st);
		return 2;
	}
	printf("r4 written at the exit stop: %s\n",
	       WEXITSTATUS(st) & 1 ? "lost, BUG" : "kept, OK");
	printf("replay result: %s\n",
	       WEXITSTATUS(st) & 2 ? "wrong" : "getppid(), OK");
	return fail || WEXITSTATUS(st);
}
---
 arch/powerpc/include/asm/thread_info.h |  3 ---
 arch/powerpc/kernel/interrupt.c        | 16 ++++++++++++++--
 2 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/arch/powerpc/include/asm/thread_info.h b/arch/powerpc/include/asm/thread_info.h
index 1e069a2e7ce87..0e8dc71c8d3fe 100644
--- a/arch/powerpc/include/asm/thread_info.h
+++ b/arch/powerpc/include/asm/thread_info.h
@@ -142,9 +142,6 @@ void arch_setup_new_exec(void);
 #define _TIF_SYSCALL_TRACEPOINT	(1<<TIF_SYSCALL_TRACEPOINT)
 #define _TIF_EMULATE_STACK_STORE	(1<<TIF_EMULATE_STACK_STORE)
 #define _TIF_SYSCALL_EMU	(1<<TIF_SYSCALL_EMU)
-#define _TIF_SYSCALL_DOTRACE	(_TIF_SYSCALL_TRACE | _TIF_SYSCALL_AUDIT | \
-				 _TIF_SECCOMP | _TIF_SYSCALL_TRACEPOINT | \
-				 _TIF_SYSCALL_EMU)
 
 #define _TIF_USER_WORK_MASK	(_TIF_SIGPENDING | _TIF_NEED_RESCHED | \
 				 _TIF_NEED_RESCHED_LAZY | _TIF_NOTIFY_RESUME | \
diff --git a/arch/powerpc/kernel/interrupt.c b/arch/powerpc/kernel/interrupt.c
index 55f9c0c9922ac..524c51104e4c9 100644
--- a/arch/powerpc/kernel/interrupt.c
+++ b/arch/powerpc/kernel/interrupt.c
@@ -75,6 +75,18 @@ static notrace __always_inline bool prep_irq_for_enabled_exit(bool restartable)
 	return true;
 }
 
+/*
+ * Syscall work after which the exit must reload all registers from
+ * pt_regs: a tracer may have changed any of them at a syscall or seccomp
+ * stop, or under PTRACE_SYSEMU. Audit and the tracepoint are included to
+ * keep the pre-GENERIC_ENTRY behaviour.
+ */
+#define PPC_SYSCALL_WORK_RESTOREALL	(SYSCALL_WORK_SECCOMP |		\
+					 SYSCALL_WORK_SYSCALL_TRACEPOINT | \
+					 SYSCALL_WORK_SYSCALL_TRACE |	\
+					 SYSCALL_WORK_SYSCALL_EMU |	\
+					 SYSCALL_WORK_SYSCALL_AUDIT)
+
 /*
  * This should be called after a syscall returns, with r3 the return value
  * from the syscall. If this function returns non-zero, the system call
@@ -117,9 +129,9 @@ notrace unsigned long syscall_exit_prepare(unsigned long r3,
 		regs->gpr[3] = r3;
 	}
 
-	if (unlikely(ti_flags & _TIF_SYSCALL_DOTRACE)) {
+	if (unlikely(READ_ONCE(current_thread_info()->syscall_work) &
+		     PPC_SYSCALL_WORK_RESTOREALL))
 		ret |= _TIF_RESTOREALL;
-	}
 
 	syscall_exit_to_user_mode(regs);
 
-- 
2.51.0

[PATCH 2/2] powerpc: Remove unused thread info flags

[Michal Suchanek]

These are no longer used with the generic entry code

Fixes: 6ed60999d33d4 ("powerpc: Remove unused functions")
Suggested-by: Shrikanth Hegde <sshegde@linux.ibm.com>
Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
 arch/powerpc/include/asm/thread_info.h | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/arch/powerpc/include/asm/thread_info.h b/arch/powerpc/include/asm/thread_info.h
index 0e8dc71c8d3fe..4b7ec1db30349 100644
--- a/arch/powerpc/include/asm/thread_info.h
+++ b/arch/powerpc/include/asm/thread_info.h
@@ -96,22 +96,17 @@ void arch_setup_new_exec(void);
 /*
  * thread information flag bit numbers
  */
-#define TIF_SYSCALL_TRACE	0	/* syscall trace active */
 #define TIF_SIGPENDING		1	/* signal pending */
 #define TIF_NEED_RESCHED	2	/* rescheduling necessary */
 #define TIF_NOTIFY_SIGNAL	3	/* signal notifications exist */
-#define TIF_SYSCALL_EMU		4	/* syscall emulation active */
 #define TIF_RESTORE_TM		5	/* need to restore TM FP/VEC/VSX */
 #define TIF_PATCH_PENDING	6	/* pending live patching update */
-#define TIF_SYSCALL_AUDIT	7	/* syscall auditing active */
 #define TIF_SINGLESTEP		8	/* singlestepping active */
 #define TIF_NEED_RESCHED_LAZY	9       /* Scheduler driven lazy preemption */
-#define TIF_SECCOMP		10	/* secure computing */
 #define TIF_RESTOREALL		11	/* Restore all regs (implies NOERROR) */
 #define TIF_NOERROR		12	/* Force successful syscall return */
 #define TIF_NOTIFY_RESUME	13	/* callback before returning to user */
 #define TIF_UPROBE		14	/* breakpointed or single-stepping */
-#define TIF_SYSCALL_TRACEPOINT	15	/* syscall tracepoint instrumentation */
 #define TIF_EMULATE_STACK_STORE	16	/* Is an instruction emulation
 						for stack store? */
 #define TIF_MEMDIE		17	/* is terminating due to OOM killer */
@@ -123,7 +118,6 @@ void arch_setup_new_exec(void);
 #define TIF_SYSCALL_RET		21	/* syscall error value set */
 
 /* as above, but as bit values */
-#define _TIF_SYSCALL_TRACE	(1<<TIF_SYSCALL_TRACE)
 #define _TIF_SIGPENDING		(1<<TIF_SIGPENDING)
 #define _TIF_NEED_RESCHED	(1<<TIF_NEED_RESCHED)
 #define _TIF_NEED_RESCHED_LAZY	(1<<TIF_NEED_RESCHED_LAZY)
@@ -132,16 +126,12 @@ void arch_setup_new_exec(void);
 #define _TIF_32BIT		(1<<TIF_32BIT)
 #define _TIF_RESTORE_TM		(1<<TIF_RESTORE_TM)
 #define _TIF_PATCH_PENDING	(1<<TIF_PATCH_PENDING)
-#define _TIF_SYSCALL_AUDIT	(1<<TIF_SYSCALL_AUDIT)
 #define _TIF_SINGLESTEP		(1<<TIF_SINGLESTEP)
-#define _TIF_SECCOMP		(1<<TIF_SECCOMP)
 #define _TIF_RESTOREALL		(1<<TIF_RESTOREALL)
 #define _TIF_NOERROR		(1<<TIF_NOERROR)
 #define _TIF_NOTIFY_RESUME	(1<<TIF_NOTIFY_RESUME)
 #define _TIF_UPROBE		(1<<TIF_UPROBE)
-#define _TIF_SYSCALL_TRACEPOINT	(1<<TIF_SYSCALL_TRACEPOINT)
 #define _TIF_EMULATE_STACK_STORE	(1<<TIF_EMULATE_STACK_STORE)
-#define _TIF_SYSCALL_EMU	(1<<TIF_SYSCALL_EMU)
 
 #define _TIF_USER_WORK_MASK	(_TIF_SIGPENDING | _TIF_NEED_RESCHED | \
 				 _TIF_NEED_RESCHED_LAZY | _TIF_NOTIFY_RESUME | \
-- 
2.51.0

Re: [PATCH 2/2] powerpc: Remove unused thread info flags

[Mukesh Kumar Chaurasiya]

On Mon, Oct 12, 2026 at 07:38:29PM +0200, Michal Suchanek wrote:
> These are no longer used with the generic entry code
> 
> Fixes: 6ed60999d33d4 ("powerpc: Remove unused functions")
> Suggested-by: Shrikanth Hegde <sshegde@linux.ibm.com>
> Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> ---
>  arch/powerpc/include/asm/thread_info.h | 10 ----------
>  1 file changed, 10 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/thread_info.h b/arch/powerpc/include/asm/thread_info.h
> index 0e8dc71c8d3fe..4b7ec1db30349 100644
> --- a/arch/powerpc/include/asm/thread_info.h
> +++ b/arch/powerpc/include/asm/thread_info.h
> @@ -96,22 +96,17 @@ void arch_setup_new_exec(void);
>  /*
>   * thread information flag bit numbers
>   */
> -#define TIF_SYSCALL_TRACE	0	/* syscall trace active */
>  #define TIF_SIGPENDING		1	/* signal pending */
>  #define TIF_NEED_RESCHED	2	/* rescheduling necessary */
>  #define TIF_NOTIFY_SIGNAL	3	/* signal notifications exist */
> -#define TIF_SYSCALL_EMU		4	/* syscall emulation active */
>  #define TIF_RESTORE_TM		5	/* need to restore TM FP/VEC/VSX */
>  #define TIF_PATCH_PENDING	6	/* pending live patching update */
> -#define TIF_SYSCALL_AUDIT	7	/* syscall auditing active */
>  #define TIF_SINGLESTEP		8	/* singlestepping active */
>  #define TIF_NEED_RESCHED_LAZY	9       /* Scheduler driven lazy preemption */
> -#define TIF_SECCOMP		10	/* secure computing */
>  #define TIF_RESTOREALL		11	/* Restore all regs (implies NOERROR) */
>  #define TIF_NOERROR		12	/* Force successful syscall return */
>  #define TIF_NOTIFY_RESUME	13	/* callback before returning to user */
>  #define TIF_UPROBE		14	/* breakpointed or single-stepping */
> -#define TIF_SYSCALL_TRACEPOINT	15	/* syscall tracepoint instrumentation */
>  #define TIF_EMULATE_STACK_STORE	16	/* Is an instruction emulation
>  						for stack store? */
>  #define TIF_MEMDIE		17	/* is terminating due to OOM killer */
> @@ -123,7 +118,6 @@ void arch_setup_new_exec(void);
>  #define TIF_SYSCALL_RET		21	/* syscall error value set */
>  
>  /* as above, but as bit values */
> -#define _TIF_SYSCALL_TRACE	(1<<TIF_SYSCALL_TRACE)
>  #define _TIF_SIGPENDING		(1<<TIF_SIGPENDING)
>  #define _TIF_NEED_RESCHED	(1<<TIF_NEED_RESCHED)
>  #define _TIF_NEED_RESCHED_LAZY	(1<<TIF_NEED_RESCHED_LAZY)
> @@ -132,16 +126,12 @@ void arch_setup_new_exec(void);
>  #define _TIF_32BIT		(1<<TIF_32BIT)
>  #define _TIF_RESTORE_TM		(1<<TIF_RESTORE_TM)
>  #define _TIF_PATCH_PENDING	(1<<TIF_PATCH_PENDING)
> -#define _TIF_SYSCALL_AUDIT	(1<<TIF_SYSCALL_AUDIT)
>  #define _TIF_SINGLESTEP		(1<<TIF_SINGLESTEP)
> -#define _TIF_SECCOMP		(1<<TIF_SECCOMP)
>  #define _TIF_RESTOREALL		(1<<TIF_RESTOREALL)
>  #define _TIF_NOERROR		(1<<TIF_NOERROR)
>  #define _TIF_NOTIFY_RESUME	(1<<TIF_NOTIFY_RESUME)
>  #define _TIF_UPROBE		(1<<TIF_UPROBE)
> -#define _TIF_SYSCALL_TRACEPOINT	(1<<TIF_SYSCALL_TRACEPOINT)
>  #define _TIF_EMULATE_STACK_STORE	(1<<TIF_EMULATE_STACK_STORE)
> -#define _TIF_SYSCALL_EMU	(1<<TIF_SYSCALL_EMU)
>  
>  #define _TIF_USER_WORK_MASK	(_TIF_SIGPENDING | _TIF_NEED_RESCHED | \
>  				 _TIF_NEED_RESCHED_LAZY | _TIF_NOTIFY_RESUME | \
> -- 
> 2.51.0
> 
LGTM
Reviewed-by: Mukesh Kumar Chaurasiya (IBM) <mkchauras@gmail.com>

Re: [PATCH 2/2] powerpc: Remove unused thread info flags

[Shrikanth Hegde]

On 10/12/26 11:08 PM, Michal Suchanek wrote:
> These are no longer used with the generic entry code
> 
> Fixes: 6ed60999d33d4 ("powerpc: Remove unused functions")
> Suggested-by: Shrikanth Hegde <sshegde@linux.ibm.com>
> Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> ---
>   arch/powerpc/include/asm/thread_info.h | 10 ----------
>   1 file changed, 10 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/thread_info.h b/arch/powerpc/include/asm/thread_info.h
> index 0e8dc71c8d3fe..4b7ec1db30349 100644
> --- a/arch/powerpc/include/asm/thread_info.h
> +++ b/arch/powerpc/include/asm/thread_info.h
> @@ -96,22 +96,17 @@ void arch_setup_new_exec(void);
>   /*
>    * thread information flag bit numbers
>    */
> -#define TIF_SYSCALL_TRACE	0	/* syscall trace active */
>   #define TIF_SIGPENDING		1	/* signal pending */
>   #define TIF_NEED_RESCHED	2	/* rescheduling necessary */
>   #define TIF_NOTIFY_SIGNAL	3	/* signal notifications exist */
> -#define TIF_SYSCALL_EMU		4	/* syscall emulation active */
>   #define TIF_RESTORE_TM		5	/* need to restore TM FP/VEC/VSX */
>   #define TIF_PATCH_PENDING	6	/* pending live patching update */
> -#define TIF_SYSCALL_AUDIT	7	/* syscall auditing active */
>   #define TIF_SINGLESTEP		8	/* singlestepping active */
>   #define TIF_NEED_RESCHED_LAZY	9       /* Scheduler driven lazy preemption */
> -#define TIF_SECCOMP		10	/* secure computing */
>   #define TIF_RESTOREALL		11	/* Restore all regs (implies NOERROR) */
>   #define TIF_NOERROR		12	/* Force successful syscall return */
>   #define TIF_NOTIFY_RESUME	13	/* callback before returning to user */
>   #define TIF_UPROBE		14	/* breakpointed or single-stepping */
> -#define TIF_SYSCALL_TRACEPOINT	15	/* syscall tracepoint instrumentation */
>   #define TIF_EMULATE_STACK_STORE	16	/* Is an instruction emulation
>   						for stack store? */
>   #define TIF_MEMDIE		17	/* is terminating due to OOM killer */
> @@ -123,7 +118,6 @@ void arch_setup_new_exec(void);
>   #define TIF_SYSCALL_RET		21	/* syscall error value set */
>   
>   /* as above, but as bit values */
> -#define _TIF_SYSCALL_TRACE	(1<<TIF_SYSCALL_TRACE)
>   #define _TIF_SIGPENDING		(1<<TIF_SIGPENDING)
>   #define _TIF_NEED_RESCHED	(1<<TIF_NEED_RESCHED)
>   #define _TIF_NEED_RESCHED_LAZY	(1<<TIF_NEED_RESCHED_LAZY)
> @@ -132,16 +126,12 @@ void arch_setup_new_exec(void);
>   #define _TIF_32BIT		(1<<TIF_32BIT)
>   #define _TIF_RESTORE_TM		(1<<TIF_RESTORE_TM)
>   #define _TIF_PATCH_PENDING	(1<<TIF_PATCH_PENDING)
> -#define _TIF_SYSCALL_AUDIT	(1<<TIF_SYSCALL_AUDIT)
>   #define _TIF_SINGLESTEP		(1<<TIF_SINGLESTEP)
> -#define _TIF_SECCOMP		(1<<TIF_SECCOMP)
>   #define _TIF_RESTOREALL		(1<<TIF_RESTOREALL)
>   #define _TIF_NOERROR		(1<<TIF_NOERROR)
>   #define _TIF_NOTIFY_RESUME	(1<<TIF_NOTIFY_RESUME)
>   #define _TIF_UPROBE		(1<<TIF_UPROBE)
> -#define _TIF_SYSCALL_TRACEPOINT	(1<<TIF_SYSCALL_TRACEPOINT)
>   #define _TIF_EMULATE_STACK_STORE	(1<<TIF_EMULATE_STACK_STORE)
> -#define _TIF_SYSCALL_EMU	(1<<TIF_SYSCALL_EMU)
>   
>   #define _TIF_USER_WORK_MASK	(_TIF_SIGPENDING | _TIF_NEED_RESCHED | \
>   				 _TIF_NEED_RESCHED_LAZY | _TIF_NOTIFY_RESUME | \

Thanks for the cleanup. It maybe possible to re-org the TIF flags. But that is
out of scope here.

Reviewed-by: Shrikanth Hegde <sshegde@linux.ibm.com>

Re: [PATCH 1/2] powerpc: Restore all registers after a traced syscall again

[Mukesh Kumar Chaurasiya]

On Mon, Oct 12, 2026 at 07:38:28PM +0200, Michal Suchanek wrote:
> From: Bernhard M. Wiedemann <bwiedemann@suse.de>
> 
[...]
> +/*
> + * Syscall work after which the exit must reload all registers from
> + * pt_regs: a tracer may have changed any of them at a syscall or seccomp
> + * stop, or under PTRACE_SYSEMU. Audit and the tracepoint are included to
> + * keep the pre-GENERIC_ENTRY behaviour.
> + */
> +#define PPC_SYSCALL_WORK_RESTOREALL	(SYSCALL_WORK_SECCOMP |		\
> +					 SYSCALL_WORK_SYSCALL_TRACEPOINT | \
> +					 SYSCALL_WORK_SYSCALL_TRACE |	\
> +					 SYSCALL_WORK_SYSCALL_EMU |	\
> +					 SYSCALL_WORK_SYSCALL_AUDIT)
> +
Hey Michal,

Thanks for this.
I would prefer this to be in thread_info.h if it's not creating any
cyclic dependency or any other issue.

Regards,
Mukesh
>  /*
>   * This should be called after a syscall returns, with r3 the return value
>   * from the syscall. If this function returns non-zero, the system call
> @@ -117,9 +129,9 @@ notrace unsigned long syscall_exit_prepare(unsigned long r3,
>  		regs->gpr[3] = r3;
>  	}
>  
> -	if (unlikely(ti_flags & _TIF_SYSCALL_DOTRACE)) {
> +	if (unlikely(READ_ONCE(current_thread_info()->syscall_work) &
> +		     PPC_SYSCALL_WORK_RESTOREALL))
>  		ret |= _TIF_RESTOREALL;
> -	}
>  
>  	syscall_exit_to_user_mode(regs);
>  
> -- 
> 2.51.0
> 

Re: [PATCH 1/2] powerpc: Restore all registers after a traced syscall again

[Shrikanth Hegde]

Hi Michal,

On 10/12/26 11:08 PM, Michal Suchanek wrote:
> From: Bernhard M. Wiedemann <bwiedemann@suse.de>
> 
> syscall_exit_prepare() asks the exit assembly to reload all GPRs, CTR
> and XER from pt_regs only if the thread flags contain any of the flags
> in _TIF_SYSCALL_DOTRACE. With the new entry code based GENERIC_ENTRY the
> flags are no longer set.
> 
> This breaks tracers that modify a stopped syscall, for example ones that
> re-execute the syscall instruction by moving the NIP back by 4 at the
> syscall-exit stop with the syscall number put back into r0: the sc then
> runs with r0 = 0, which is __NR_restart_syscall. dettrace fails like
> this with "No filter rule for system call: restart_syscall". A tracer
> that writes r4 at the syscall-exit stop of an sc sees the tracee
> continue with r4 = 0 instead; with a 7.1 kernel both work.
> 
> Check the equivalent syscall_work bits instead, and drop the now unused
> _TIF_SYSCALL_DOTRACE.
> 
> Fixes: bee25f97ad24 ("powerpc: Enable GENERIC_ENTRY feature")
> Cc: stable@vger.kernel.org # 7.2.x: c7585b8e99ad: powerpc: Don't drop _TIF_RESTOREALL on syscall restart
> Cc: stable@vger.kernel.org # 7.2.x
> Assisted-by: LLM
> Signed-off-by: Bernhard M. Wiedemann <bwiedemann@suse.de>
> [ms: reword commit message]
> Signed-off-by: Michal Suchánek <msuchanek@suse.de>
> ---
> A backport also needs commit c7585b8e99ad ("powerpc: Don't drop
> _TIF_RESTOREALL on syscall restart"), without which the flag can still
> be lost.
> 
> It has been pointed out that the newly defined
> PPC_SYSCALL_WORK_RESTOREALL is almost the same as SYSCALL_WORK_ENTER and
> the existing define might be used instead. However, it is not clear if
> this is expected or if this is merely coincidence, and not guaranteed.
> 

They seems to semantically different. So for now, we can keep it separate.

> Testing: a small tracer (pokes r4 at the syscall-exit stop of an sc,
> and replays a syscall by moving NIP back with a new r0) shows both
> writes lost on a ppc64le and both working on 7.1.8.
> ------------[ cut here ]------------
> // SPDX-License-Identifier: GPL-2.0
> /*
>   * Does a register a tracer writes at the syscall-exit stop of an "sc"
>   * reach the tracee? Checks r4, and a replay: NIP moved back onto the sc
>   * with r0 set to another syscall number.
>   * Build: gcc -O2 -o ppc-exitregs ppc-exitregs.c (powerpc64 only)
>   */
> 
> /* word offsets in struct pt_regs, arch/powerpc/include/uapi/asm/ptrace.h */
> 
> static long do_sc(long nr, long *r4io)
> {
> 	register long r0 __asm__("r0") = nr;
> 	register long r3 __asm__("r3") = 0;
> 	register long r4 __asm__("r4") = *r4io;
> 
> 	__asm__ volatile("sc"
> 			 : "+r"(r0), "+r"(r3), "+r"(r4)
> 			 :
> 			 : "r5", "r6", "r7", "r8", "r9", "r10", "r11", "r12",
> 			   "cr0", "ctr", "xer", "memory");
> 	*r4io = r4;
> 	return r3;
> }
> 
> static long peek(pid_t pid, int w)
> {
> 	errno = 0;
> 	long v = ptrace(PTRACE_PEEKUSER, pid, (void *)(w * sizeof(long)), 0);
> 	if (errno) {
> 		perror("PEEKUSER");
> 		exit(2);
> 	}
> 	return v;
> }
> 
> static void poke(pid_t pid, int w, long v)
> {
> 	if (ptrace(PTRACE_POKEUSER, pid, (void *)(w * sizeof(long)), (void *)v)) {
> 		perror("POKEUSER");
> 		exit(2);
> 	}
> }
> 
> /* Resume to the next syscall stop, which must be one. */
> static void next_stop(pid_t pid)
> {
> 	int st;
> 
> 	ptrace(PTRACE_SYSCALL, pid, 0, 0);
> 	if (waitpid(pid, &st, 0) != pid || !WIFSTOPPED(st) ||
> 	    WSTOPSIG(st) != (SIGTRAP | 0x80)) {
> 		fprintf(stderr, "unexpected wait status %#x\n", st);
> 		exit(2);
> 	}
> }
> 
> int main(void)
> {
> 	pid_t pid = fork();
> 	int st, fail = 0;
> 
> 	if (pid == 0) {
> 		long r4 = 0x1111, r4a, ret;
> 
> 		ptrace(PTRACE_TRACEME, 0, 0, 0);
> 		raise(SIGSTOP);
> 		do_sc(__NR_getpid, &r4);
> 		r4a = r4;
> 		ret = do_sc(__NR_getpid, &r4);
> 		/* bit 0: the tracer's r4 was lost; bit 1: the replay of
> 		 * the second getpid as getppid returned something else */
> 		_exit((r4a != 0x2222) | (ret != getppid()) << 1);
> 	}
> 
> 	waitpid(pid, &st, 0);
> 	ptrace(PTRACE_SETOPTIONS, pid, 0, (void *)PTRACE_O_TRACESYSGOOD);
> 
> 	/* 1: write r4 at the exit stop of getpid */
> 	next_stop(pid);				/* entry */
> 	next_stop(pid);				/* exit */
> 	poke(pid, W_R4, 0x2222);
> 	next_stop(pid);				/* entry of the second getpid */
> 
> 	/* 2: replay the second getpid as getppid */
> 	next_stop(pid);				/* exit */
> 	poke(pid, W_NIP, peek(pid, W_NIP) - 4);
> 	poke(pid, W_R0, __NR_getppid);
> 	next_stop(pid);				/* entry of the replay */
> 	long nr = peek(pid, W_R0);
> 	printf("replayed syscall arrives as nr %ld (%s)\n", nr,
> 	       nr == __NR_getppid ? "getppid, OK" :
> 	       nr == __NR_restart_syscall ? "restart_syscall, BUG" : "?");
> 	if (nr != __NR_getppid) {
> 		fail = 1;
> 		poke(pid, W_R0, __NR_getppid);	/* entry writes do work */
> 	}
> 	next_stop(pid);				/* exit */
> 	ptrace(PTRACE_CONT, pid, 0, 0);
> 	waitpid(pid, &st, 0);
> 	if (!WIFEXITED(st)) {
> 		printf("child did not exit normally: %#x\n", st);
> 		return 2;
> 	}
> 	printf("r4 written at the exit stop: %s\n",
> 	       WEXITSTATUS(st) & 1 ? "lost, BUG" : "kept, OK");
> 	printf("replay result: %s\n",
> 	       WEXITSTATUS(st) & 2 ? "wrong" : "getppid(), OK");
> 	return fail || WEXITSTATUS(st);
> }
> ---
>   arch/powerpc/include/asm/thread_info.h |  3 ---
>   arch/powerpc/kernel/interrupt.c        | 16 ++++++++++++++--
>   2 files changed, 14 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/thread_info.h b/arch/powerpc/include/asm/thread_info.h
> index 1e069a2e7ce87..0e8dc71c8d3fe 100644
> --- a/arch/powerpc/include/asm/thread_info.h
> +++ b/arch/powerpc/include/asm/thread_info.h
> @@ -142,9 +142,6 @@ void arch_setup_new_exec(void);
>   #define _TIF_SYSCALL_TRACEPOINT	(1<<TIF_SYSCALL_TRACEPOINT)
>   #define _TIF_EMULATE_STACK_STORE	(1<<TIF_EMULATE_STACK_STORE)
>   #define _TIF_SYSCALL_EMU	(1<<TIF_SYSCALL_EMU)
> -#define _TIF_SYSCALL_DOTRACE	(_TIF_SYSCALL_TRACE | _TIF_SYSCALL_AUDIT | \
> -				 _TIF_SECCOMP | _TIF_SYSCALL_TRACEPOINT | \
> -				 _TIF_SYSCALL_EMU)
>   
>   #define _TIF_USER_WORK_MASK	(_TIF_SIGPENDING | _TIF_NEED_RESCHED | \
>   				 _TIF_NEED_RESCHED_LAZY | _TIF_NOTIFY_RESUME | \
> diff --git a/arch/powerpc/kernel/interrupt.c b/arch/powerpc/kernel/interrupt.c
> index 55f9c0c9922ac..524c51104e4c9 100644
> --- a/arch/powerpc/kernel/interrupt.c
> +++ b/arch/powerpc/kernel/interrupt.c
> @@ -75,6 +75,18 @@ static notrace __always_inline bool prep_irq_for_enabled_exit(bool restartable)
>   	return true;
>   }
>   
> +/*
> + * Syscall work after which the exit must reload all registers from
> + * pt_regs: a tracer may have changed any of them at a syscall or seccomp
> + * stop, or under PTRACE_SYSEMU. Audit and the tracepoint are included to
> + * keep the pre-GENERIC_ENTRY behaviour.
> + */
> +#define PPC_SYSCALL_WORK_RESTOREALL	(SYSCALL_WORK_SECCOMP |		\
> +					 SYSCALL_WORK_SYSCALL_TRACEPOINT | \
> +					 SYSCALL_WORK_SYSCALL_TRACE |	\
> +					 SYSCALL_WORK_SYSCALL_EMU |	\
> +					 SYSCALL_WORK_SYSCALL_AUDIT)
> +

nit: Apart from mukesh's comment,
can we keep the name as SYSCALL_WORK_RESTOREALL instead? It is already in PPC folder.

>   /*
>    * This should be called after a syscall returns, with r3 the return value
>    * from the syscall. If this function returns non-zero, the system call
> @@ -117,9 +129,9 @@ notrace unsigned long syscall_exit_prepare(unsigned long r3,
>   		regs->gpr[3] = r3;
>   	}
>   
> -	if (unlikely(ti_flags & _TIF_SYSCALL_DOTRACE)) {
> +	if (unlikely(READ_ONCE(current_thread_info()->syscall_work) &
> +		     PPC_SYSCALL_WORK_RESTOREALL))
>   		ret |= _TIF_RESTOREALL;
> -	}
>   
>   	syscall_exit_to_user_mode(regs);
>