mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH 1/2] powerpc: Restore all registers after a traced syscall again
@ 2026-10-12 17:38 Michal Suchanek
  2026-10-12 17:38 ` [PATCH 2/2] powerpc: Remove unused thread info flags Michal Suchanek
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Michal Suchanek @ 2026-10-12 17:38 UTC (permalink / raw)
  To: linuxppc-dev
  Cc: Michal Suchanek, Madhavan Srinivasan, Michael Ellerman,
	Nicholas Piggin, Christophe Leroy (CS GROUP),
	Ritesh Harjani (IBM),
	Shrikanth Hegde, Kees Cook, Andy Lutomirski, Will Drewry,
	Mukesh Kumar Chaurasiya (IBM),
	Kevin Brodsky, Alexander Gordeev, Mukesh Kumar Chaurasiya,
	Bernhard M. Wiedemann, linux-kernel, stable

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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 2/2] powerpc: Remove unused thread info flags
  2026-10-12 17:38 [PATCH 1/2] powerpc: Restore all registers after a traced syscall again Michal Suchanek
@ 2026-10-12 17:38 ` Michal Suchanek
  2026-10-13 13:16   ` Mukesh Kumar Chaurasiya
  2026-10-13 14:06   ` Shrikanth Hegde
  2026-10-13 13:13 ` [PATCH 1/2] powerpc: Restore all registers after a traced syscall again Mukesh Kumar Chaurasiya
  2026-10-13 13:51 ` Shrikanth Hegde
  2 siblings, 2 replies; 6+ messages in thread
From: Michal Suchanek @ 2026-10-12 17:38 UTC (permalink / raw)
  To: linuxppc-dev
  Cc: Michal Suchanek, Madhavan Srinivasan, Michael Ellerman,
	Nicholas Piggin, Christophe Leroy (CS GROUP),
	Ritesh Harjani (IBM),
	Shrikanth Hegde, Kees Cook, Andy Lutomirski, Will Drewry,
	Mukesh Kumar Chaurasiya (IBM),
	Kevin Brodsky, Alexander Gordeev, Mukesh Kumar Chaurasiya,
	Bernhard M. Wiedemann, linux-kernel

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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] powerpc: Restore all registers after a traced syscall again
  2026-10-12 17:38 [PATCH 1/2] powerpc: Restore all registers after a traced syscall again Michal Suchanek
  2026-10-12 17:38 ` [PATCH 2/2] powerpc: Remove unused thread info flags Michal Suchanek
@ 2026-10-13 13:13 ` Mukesh Kumar Chaurasiya
  2026-10-13 13:51 ` Shrikanth Hegde
  2 siblings, 0 replies; 6+ messages in thread
From: Mukesh Kumar Chaurasiya @ 2026-10-13 13:13 UTC (permalink / raw)
  To: Michal Suchanek
  Cc: linuxppc-dev, Madhavan Srinivasan, Michael Ellerman,
	Nicholas Piggin, Christophe Leroy (CS GROUP),
	Ritesh Harjani (IBM),
	Shrikanth Hegde, Kees Cook, Andy Lutomirski, Will Drewry,
	Kevin Brodsky, Alexander Gordeev, Mukesh Kumar Chaurasiya,
	Bernhard M. Wiedemann, linux-kernel, stable

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
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] powerpc: Remove unused thread info flags
  2026-10-12 17:38 ` [PATCH 2/2] powerpc: Remove unused thread info flags Michal Suchanek
@ 2026-10-13 13:16   ` Mukesh Kumar Chaurasiya
  2026-10-13 14:06   ` Shrikanth Hegde
  1 sibling, 0 replies; 6+ messages in thread
From: Mukesh Kumar Chaurasiya @ 2026-10-13 13:16 UTC (permalink / raw)
  To: Michal Suchanek
  Cc: linuxppc-dev, Madhavan Srinivasan, Michael Ellerman,
	Nicholas Piggin, Christophe Leroy (CS GROUP),
	Ritesh Harjani (IBM),
	Shrikanth Hegde, Kees Cook, Andy Lutomirski, Will Drewry,
	Kevin Brodsky, Alexander Gordeev, Mukesh Kumar Chaurasiya,
	Bernhard M. Wiedemann, linux-kernel

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>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] powerpc: Restore all registers after a traced syscall again
  2026-10-12 17:38 [PATCH 1/2] powerpc: Restore all registers after a traced syscall again Michal Suchanek
  2026-10-12 17:38 ` [PATCH 2/2] powerpc: Remove unused thread info flags Michal Suchanek
  2026-10-13 13:13 ` [PATCH 1/2] powerpc: Restore all registers after a traced syscall again Mukesh Kumar Chaurasiya
@ 2026-10-13 13:51 ` Shrikanth Hegde
  2 siblings, 0 replies; 6+ messages in thread
From: Shrikanth Hegde @ 2026-10-13 13:51 UTC (permalink / raw)
  To: Michal Suchanek, linuxppc-dev, Mukesh Kumar Chaurasiya
  Cc: Madhavan Srinivasan, Michael Ellerman, Nicholas Piggin,
	Christophe Leroy (CS GROUP), Ritesh Harjani (IBM),
	Kees Cook, Andy Lutomirski, Will Drewry,
	Mukesh Kumar Chaurasiya (IBM),
	Kevin Brodsky, Alexander Gordeev, Bernhard M. Wiedemann,
	linux-kernel, stable

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);
>   

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] powerpc: Remove unused thread info flags
  2026-10-12 17:38 ` [PATCH 2/2] powerpc: Remove unused thread info flags Michal Suchanek
  2026-10-13 13:16   ` Mukesh Kumar Chaurasiya
@ 2026-10-13 14:06   ` Shrikanth Hegde
  1 sibling, 0 replies; 6+ messages in thread
From: Shrikanth Hegde @ 2026-10-13 14:06 UTC (permalink / raw)
  To: Michal Suchanek, linuxppc-dev
  Cc: Madhavan Srinivasan, Michael Ellerman, Nicholas Piggin,
	Christophe Leroy (CS GROUP), Ritesh Harjani (IBM),
	Kees Cook, Andy Lutomirski, Will Drewry,
	Mukesh Kumar Chaurasiya (IBM),
	Kevin Brodsky, Alexander Gordeev, Mukesh Kumar Chaurasiya,
	Bernhard M. Wiedemann, linux-kernel



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>

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-10-13 14:06 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-12 17:38 [PATCH 1/2] powerpc: Restore all registers after a traced syscall again Michal Suchanek
2026-10-12 17:38 ` [PATCH 2/2] powerpc: Remove unused thread info flags Michal Suchanek
2026-10-13 13:16   ` Mukesh Kumar Chaurasiya
2026-10-13 14:06   ` Shrikanth Hegde
2026-10-13 13:13 ` [PATCH 1/2] powerpc: Restore all registers after a traced syscall again Mukesh Kumar Chaurasiya
2026-10-13 13:51 ` Shrikanth Hegde

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®