Skip to contentSkip to conversation

powerpc/64s/syscall: Use pt_regs.trap to distinguish syscall ABI difference between sc and scv syscalls

PATCH Applied Nicholas Piggin 3 participants 5 messages May 20, 2021 – May 21, 2021

Subsystems (5)
Documentation PowerPC Kselftest ptrace support +1 more subsystems
lore
Share & tools
lore

Nicholas Piggin

author reviewer
#0 lore raw

PATCH 1/2 powerpc/64s/syscall: Use pt_regs.trap to distinguish syscall ABI difference between sc and scv syscalls

The sc and scv 0 system calls have different ABI conventions, and ptracers need to know which system call type is being used if it wants to look at the syscall registers.

Document that pt_regs.trap can be used for this, and fix one in-tree user to work with scv 0 syscalls.

Fixes 7fa95f9adaee ("powerpc/64s: system call support for scv/rfscv instructions")
Reported-by "Dmitry V. Levin" <[email protected]>
Suggested-by "Dmitry V. Levin" <[email protected]>
Signed-off-by Nicholas Piggin <[email protected]>
---
 Documentation/powerpc/syscall64-abi.rst       | 10 +++++++
 tools/testing/selftests/seccomp/seccomp_bpf.c | 27 ++++++++++++-------
 2 files changed, 28 insertions(+), 9 deletions(-)
Documentation/powerpc/syscall64-abi.rst +10 -0
index dabee3729e5a..56490c4c0c07 100644
@@ -109,6 +109,16 @@ auxiliary vector.
 
 scv 0 syscalls will always behave as PPC_FEATURE2_HTM_NOSC.
 
+ptrace
+------
+When ptracing system calls (PTRACE_SYSCALL), the pt_regs.trap value contains
+the system call type that can be used to distinguish between sc and scv 0
+system calls, and the different register conventions can be accounted for.
+
+If the value of (pt_regs.trap & 0xfff0) is 0xc00 then the system call was
+performed with the sc instruction, if it is 0x3000 then the system call was
+performed with the scv 0 instruction.
+
 vsyscall
 ========
tools/testing/selftests/seccomp/seccomp_bpf.c +18 -9
index 98c3b647f54d..e3d5c77a8612 100644
@@ -1753,16 +1753,25 @@ TEST_F(TRACE_poke, getpid_runs_normally)
 # define SYSCALL_RET_SET(_regs, _val)				\
 	do {							\
 		typeof(_val) _result = (_val);			\
-		/*						\
-		 * A syscall error is signaled by CR0 SO bit	\
-		 * and the code is stored as a positive value.	\
-		 */						\
-		if (_result < 0) {				\
-			SYSCALL_RET(_regs) = -_result;		\
-			(_regs).ccr |= 0x10000000;		\
-		} else {					\
+		if ((_regs.trap & 0xfff0) == 0x3000) {		\
+			/*					\
+			 * scv 0 system call uses -ve result	\
+			 * for error, so no need to adjust.	\
+			 */					\
 			SYSCALL_RET(_regs) = _result;		\
-			(_regs).ccr &= ~0x10000000;		\
+		} else {					\
+			/*					\
+			 * A syscall error is signaled by the	\
+			 * CR0 SO bit and the code is stored as	\
+			 * a positive value.			\
+			 */					\
+			if (_result < 0) {			\
+				SYSCALL_RET(_regs) = -_result;	\
+				(_regs).ccr |= 0x10000000;	\
+			} else {				\
+				SYSCALL_RET(_regs) = _result;	\
+				(_regs).ccr &= ~0x10000000;	\
+			}					\
 		}						\
 	} while (0)
 # define SYSCALL_RET_SET_ON_PTRACE_EXIT
Signature · 1 line
2.23.0
author reviewer
#1 lore raw

PATCH 2/2 powerpc/64s/syscall: Fix ptrace syscall info with scv syscalls

The scv implementation missed updating syscall return value and error value get/set functions to deal with the changed register ABI. This broke ptrace PTRACE_GET_SYSCALL_INFO as well as some kernel auditing and tracing functions.

Fix. tools/testing/selftests/ptrace/get_syscall_info now passes when scv is used.

Fixes 7fa95f9adaee ("powerpc/64s: system call support for scv/rfscv instructions")
Reported-by "Dmitry V. Levin" <[email protected]>
Signed-off-by Nicholas Piggin <[email protected]>
---
 arch/powerpc/include/asm/ptrace.h  | 45 +++++++++++++++++-------------
 arch/powerpc/include/asm/syscall.h | 42 +++++++++++++++++-----------
 2 files changed, 52 insertions(+), 35 deletions(-)
arch/powerpc/include/asm/ptrace.h +26 -19
index 9c9ab2746168..b476a685f066 100644
@@ -19,6 +19,7 @@
 #ifndef _ASM_POWERPC_PTRACE_H
 #define _ASM_POWERPC_PTRACE_H
 
+#include <linux/err.h>
 #include <uapi/asm/ptrace.h>
 #include <asm/asm-const.h>
 

@@ -152,25 +153,6 @@ extern unsigned long profile_pc(struct pt_regs *regs);
 long do_syscall_trace_enter(struct pt_regs *regs);
 void do_syscall_trace_leave(struct pt_regs *regs);
 
-#define kernel_stack_pointer(regs) ((regs)->gpr[1])
-static inline int is_syscall_success(struct pt_regs *regs)
-{
-	return !(regs->ccr & 0x10000000);
-}
-
-static inline long regs_return_value(struct pt_regs *regs)
-{
-	if (is_syscall_success(regs))
-		return regs->gpr[3];
-	else
-		return -regs->gpr[3];
-}
-
-static inline void regs_set_return_value(struct pt_regs *regs, unsigned long rc)
-{
-	regs->gpr[3] = rc;
-}
-
 #ifdef __powerpc64__
 #define user_mode(regs) ((((regs)->msr) >> MSR_PR_LG) & 0x1)
 #else

@@ -235,6 +217,31 @@ static __always_inline void set_trap_norestart(struct pt_regs *regs)
 	regs->trap |= 0x1;
 }
 
+#define kernel_stack_pointer(regs) ((regs)->gpr[1])
+static inline int is_syscall_success(struct pt_regs *regs)
+{
+	if (trap_is_scv(regs))
+		return !IS_ERR_VALUE((unsigned long)regs->gpr[3]);
+	else
+		return !(regs->ccr & 0x10000000);
+}
+
+static inline long regs_return_value(struct pt_regs *regs)
+{
+	if (trap_is_scv(regs))
+		return regs->gpr[3];
+
+	if (is_syscall_success(regs))
+		return regs->gpr[3];
+	else
+		return -regs->gpr[3];
+}
+
+static inline void regs_set_return_value(struct pt_regs *regs, unsigned long rc)
+{
+	regs->gpr[3] = rc;
+}
+
 #define arch_has_single_step()	(1)
 #define arch_has_block_step()	(true)
 #define ARCH_HAS_USER_SINGLE_STEP_REPORT
arch/powerpc/include/asm/syscall.h +26 -16

55 lines not on this page · raw message

Signature · 1 line
2.23.0
Reviewed-by on v1 2/2
#2 lore raw

Re: [PATCH 2/2] powerpc/64s/syscall: Fix ptrace syscall info with scv syscalls

Quoted 11 lines · Nicholas Piggin

The scv implementation missed updating syscall return value and error value get/set functions to deal with the changed register ABI. This broke ptrace PTRACE_GET_SYSCALL_INFO as well as some kernel auditing and tracing functions.

Fix. tools/testing/selftests/ptrace/get_syscall_info now passes when scv is used.

Fixes 7fa95f9adaee ("powerpc/64s: system call support for scv/rfscv instructions")
Reported-by "Dmitry V. Levin" <[email protected]>
Signed-off-by Nicholas Piggin <[email protected]>

Thanks, feel free to add

Reviewed-by Dmitry V. Levin <[email protected]>

Also consider adding
Cc: [email protected] # 5.9+

Signature · 1 line
ldv
#3 lore raw
In reply to Nicholas Piggin
Quoted 3 lines · Nicholas Piggin

The sc and scv 0 system calls have different ABI conventions, and ptracers need to know which system call type is being used if it wants to look at the syscall registers.

typo: s/if it wants/if they want/

Quoted 7 lines

Document that pt_regs.trap can be used for this, and fix one in-tree user to work with scv 0 syscalls.

Fixes 7fa95f9adaee ("powerpc/64s: system call support for scv/rfscv instructions")
Reported-by "Dmitry V. Levin" <[email protected]>
Suggested-by "Dmitry V. Levin" <[email protected]>
Signed-off-by Nicholas Piggin <[email protected]>

Also consider adding
Cc: [email protected] # 5.9+

Besides that, looks good, thanks!

Signature · 1 line
ldv
Applied
#4 lore raw
In reply to Nicholas Piggin
Quoted 6 lines · Nicholas Piggin

The sc and scv 0 system calls have different ABI conventions, and ptracers need to know which system call type is being used if it wants to look at the syscall registers.

Document that pt_regs.trap can be used for this, and fix one in-tree user to work with scv 0 syscalls.

Applied to powerpc/fixes.

[1/2] powerpc/64s/syscall: Use pt_regs.trap to distinguish syscall ABI difference between sc and scv syscalls

      https://git.kernel.org/powerpc/c/5665bc35c1ed917ac8fd06cb651317bb47a65b10
[2/2] powerpc/64s/syscall: Fix ptrace syscall info with scv syscalls
      https://git.kernel.org/powerpc/c/d72500f992849d31ebae8f821a023660ddd0dcc2

cheers