[PATCH 1/3] powerpc: Redefine TIF_32BITS thread flag

[PATCH 1/3] powerpc: Redefine TIF_32BITS thread flag

[Breno Leitao]

Moving TIF_32BIT to use bit 20 instead of 4 in the task flag field.

This change is making room for an upcoming new task macro
(_TIF_SYSCALL_EMU) which is preferred to set a bit in the lower 16-bits
part of the word.

This upcoming flag macro will take part in a composed macro
(_TIF_SYSCALL_DOTRACE) which will contain other flags as well, and it is
preferred that the whole _TIF_SYSCALL_DOTRACE macro only sets the lower 16
bits of a word, so, it could be handled using immediate operations (as load
immediate, add immediate, ...) where the immediate operand (SI) is limited
to 16-bits.

Another possible solution would be using the LOAD_REG_IMMEDIATE() macro
to load a full 64-bits word immediate, but it takes 5 operations instead of
one.

Having TIF_32BITS being redefined to use an upper bit is not a problem
since there is only one place in the assembly code where TIF_32BIT is being
used, and it could be replaced with an operation with right shift (addis),
since it is used alone, i.e. not being part of a composed macro, which has
different bits set, and would require LOAD_REG_IMMEDIATE().

Tested on a 64 bits Big Endian machine running a 32 bits task.

Signed-off-by: Breno Leitao <leitao@debian.org>
---
 arch/powerpc/include/asm/thread_info.h | 2 +-
 arch/powerpc/kernel/entry_64.S         | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/include/asm/thread_info.h b/arch/powerpc/include/asm/thread_info.h
index 3c0002044bc9..1a52e14ec3ee 100644
--- a/arch/powerpc/include/asm/thread_info.h
+++ b/arch/powerpc/include/asm/thread_info.h
@@ -81,7 +81,6 @@ extern int arch_dup_task_struct(struct task_struct *dst, struct task_struct *src
 #define TIF_SIGPENDING		1	/* signal pending */
 #define TIF_NEED_RESCHED	2	/* rescheduling necessary */
 #define TIF_FSCHECK		3	/* Check FS is USER_DS on return */
-#define TIF_32BIT		4	/* 32 bit binary */
 #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 */
@@ -100,6 +99,7 @@ extern int arch_dup_task_struct(struct task_struct *dst, struct task_struct *src
 #define TIF_ELF2ABI		18	/* function descriptors must die! */
 #endif
 #define TIF_POLLING_NRFLAG	19	/* true if poll_idle() is polling TIF_NEED_RESCHED */
+#define TIF_32BIT		20	/* 32 bit binary */
 
 /* as above, but as bit values */
 #define _TIF_SYSCALL_TRACE	(1<<TIF_SYSCALL_TRACE)
diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
index 2206912ea4f0..56f7b11d76ec 100644
--- a/arch/powerpc/kernel/entry_64.S
+++ b/arch/powerpc/kernel/entry_64.S
@@ -171,7 +171,7 @@ system_call:			/* label this so stack traces look sane */
  * based on caller's run-mode / personality.
  */
 	ld	r11,SYS_CALL_TABLE@toc(2)
-	andi.	r10,r10,_TIF_32BIT
+	andis.	r10,r10,_TIF_32BIT@h
 	beq	15f
 	addi	r11,r11,8	/* use 32-bit syscall entries */
 	clrldi	r3,r3,32
-- 
2.19.0

[PATCH 2/3] powerpc/ptrace: Add support for PTRACE_SYSEMU

[Breno Leitao]

This is a patch that adds support for PTRACE_SYSEMU ptrace request in
PowerPC architecture.

When ptrace(PTRACE_SYSEMU, ...) request is called, it will be handled by
the arch independent function ptrace_resume(), which will tag the task with
the TIF_SYSCALL_EMU flag. This flag needs to be handled from a platform
dependent point of view, which is what this patch does.

This patch adds this task's flag as part of the _TIF_SYSCALL_DOTRACE, which
is the MACRO that is used to trace syscalls at entrance/exit.

Since TIF_SYSCALL_EMU is now part of _TIF_SYSCALL_DOTRACE, if the task has
_TIF_SYSCALL_DOTRACE set, it will hit do_syscall_trace_enter() at syscall
entrance and do_syscall_trace_leave() at syscall leave.
do_syscall_trace_enter() needs to handle the TIF_SYSCALL_EMU flag properly,
which will interrupt the syscall executing if TIF_SYSCALL_EMU is set. The
output values should not be changed, i.e. the return value (r3) should
contain the original syscall argument on exit.

With this flag set, the syscall is not executed fundamentally, because
do_syscall_trace_enter() is returning -1 which is bigger than NR_syscall,
thus, skipping the syscall execution and exiting userspace.

Signed-off-by: Breno Leitao <leitao@debian.org>
---
 arch/powerpc/include/asm/thread_info.h |  4 +++-
 arch/powerpc/include/uapi/asm/ptrace.h |  4 ++++
 arch/powerpc/kernel/ptrace.c           | 11 +++++++++++
 3 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/include/asm/thread_info.h b/arch/powerpc/include/asm/thread_info.h
index 1a52e14ec3ee..3185f8ac1182 100644
--- a/arch/powerpc/include/asm/thread_info.h
+++ b/arch/powerpc/include/asm/thread_info.h
@@ -81,6 +81,7 @@ extern int arch_dup_task_struct(struct task_struct *dst, struct task_struct *src
 #define TIF_SIGPENDING		1	/* signal pending */
 #define TIF_NEED_RESCHED	2	/* rescheduling necessary */
 #define TIF_FSCHECK		3	/* Check FS is USER_DS on return */
+#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 */
@@ -120,9 +121,10 @@ extern int arch_dup_task_struct(struct task_struct *dst, struct task_struct *src
 #define _TIF_EMULATE_STACK_STORE	(1<<TIF_EMULATE_STACK_STORE)
 #define _TIF_NOHZ		(1<<TIF_NOHZ)
 #define _TIF_FSCHECK		(1<<TIF_FSCHECK)
+#define _TIF_SYSCALL_EMU	(1<<TIF_SYSCALL_EMU)
 #define _TIF_SYSCALL_DOTRACE	(_TIF_SYSCALL_TRACE | _TIF_SYSCALL_AUDIT | \
 				 _TIF_SECCOMP | _TIF_SYSCALL_TRACEPOINT | \
-				 _TIF_NOHZ)
+				 _TIF_NOHZ | _TIF_SYSCALL_EMU)
 
 #define _TIF_USER_WORK_MASK	(_TIF_SIGPENDING | _TIF_NEED_RESCHED | \
 				 _TIF_NOTIFY_RESUME | _TIF_UPROBE | \
diff --git a/arch/powerpc/include/uapi/asm/ptrace.h b/arch/powerpc/include/uapi/asm/ptrace.h
index 5e3edc2a7634..55c7a131d2ab 100644
--- a/arch/powerpc/include/uapi/asm/ptrace.h
+++ b/arch/powerpc/include/uapi/asm/ptrace.h
@@ -160,6 +160,10 @@ struct pt_regs {
 #define PTRACE_GETVSRREGS	0x1b
 #define PTRACE_SETVSRREGS	0x1c
 
+/* Syscall emulation defines */
+#define PTRACE_SYSEMU			0x1d
+#define PTRACE_SYSEMU_SINGLESTEP	0x1e
+
 /*
  * Get or set a debug register. The first 16 are DABR registers and the
  * second 16 are IABR registers.
diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c
index 9667666eb18e..4e372f54088f 100644
--- a/arch/powerpc/kernel/ptrace.c
+++ b/arch/powerpc/kernel/ptrace.c
@@ -2508,6 +2508,7 @@ void ptrace_disable(struct task_struct *child)
 {
 	/* make sure the single step bit is not set. */
 	user_disable_single_step(child);
+	clear_tsk_thread_flag(child, TIF_SYSCALL_EMU);
 }
 
 #ifdef CONFIG_PPC_ADV_DEBUG_REGS
@@ -3264,6 +3265,16 @@ long do_syscall_trace_enter(struct pt_regs *regs)
 {
 	user_exit();
 
+	if (test_thread_flag(TIF_SYSCALL_EMU)) {
+		ptrace_report_syscall(regs);
+		/*
+		 * Returning -1 will skip the syscall execution. We want to
+		 * avoid clobbering any register also, thus, not 'gotoing'
+		 * skip label.
+		 */
+		return -1;
+	}
+
 	/*
 	 * The tracer may decide to abort the syscall, if so tracehook
 	 * will return !0. Note that the tracer may also just change
-- 
2.19.0

[PATCH 3/3] selftests/powerpc: New PTRACE_SYSEMU test

[Breno Leitao]

This patch adds a new test for the new PTRACE_SYSEMU ptrace request.

This test also relies on PTRACE_GETREGS and PTRACE_SETREGS requests to
run properly, since the trace instruction (gettid() syscall) is being
modified at run-time (by PTRACE_SETREGS) and re-executed three times.
PTRACE_GETREGS is being used to check that the registers are still
sane.

This test basically creates a child process that executes syscalls
and the parent process check if it is being traced appropriately.  The
parent process guarantees that the SYSCALLs are being traced, with
PTRACE_SYSEMU, and ptrace stops the child application before a syscall is
executed. The way the tests validates it, is by guaranteeing that the
system calls arguments, as argv[0] (r3) which is the same register that
will have the syscall return value on powerpc, are not being corrupted on
PTRACE_SYSEMU with a return value, i.e, it continues to have the current
arguments instead, meaning that the registers where not clobbered.

This test is basically the same test for x86 located at
tools/testing/selftests/x86/ptrace_syscall.c, limited to test PTRACE_SYSEMU
request, and ported to PowerPC.

Signed-off-by: Breno Leitao <leitao@debian.org>
---
 .../testing/selftests/powerpc/ptrace/Makefile |   2 +-
 .../selftests/powerpc/ptrace/ptrace-syscall.c | 228 ++++++++++++++++++
 2 files changed, 229 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/powerpc/ptrace/ptrace-syscall.c

diff --git a/tools/testing/selftests/powerpc/ptrace/Makefile b/tools/testing/selftests/powerpc/ptrace/Makefile
index 28f5b781a553..1ee59978508d 100644
--- a/tools/testing/selftests/powerpc/ptrace/Makefile
+++ b/tools/testing/selftests/powerpc/ptrace/Makefile
@@ -2,7 +2,7 @@
 TEST_PROGS := ptrace-gpr ptrace-tm-gpr ptrace-tm-spd-gpr \
               ptrace-tar ptrace-tm-tar ptrace-tm-spd-tar ptrace-vsx ptrace-tm-vsx \
               ptrace-tm-spd-vsx ptrace-tm-spr ptrace-hwbreak ptrace-pkey core-pkey \
-              perf-hwbreak
+              perf-hwbreak ptrace-syscall
 
 include ../../lib.mk
 
diff --git a/tools/testing/selftests/powerpc/ptrace/ptrace-syscall.c b/tools/testing/selftests/powerpc/ptrace/ptrace-syscall.c
new file mode 100644
index 000000000000..3353210dcdbd
--- /dev/null
+++ b/tools/testing/selftests/powerpc/ptrace/ptrace-syscall.c
@@ -0,0 +1,228 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * A ptrace test for testing PTRACE_SYSEMU, PTRACE_SETREGS and
+ * PTRACE_GETREG.  This test basically create a child process that executes
+ * syscalls and the parent process check if it is being traced appropriated.
+ *
+ * This test is heavily based on tools/testing/selftests/x86/ptrace_syscall.c
+ * test, and it was adapted to run on Powerpc by
+ * Breno Leitao <leitao@debian.org>
+ */
+#define _GNU_SOURCE
+
+#include <sys/ptrace.h>
+#include <sys/types.h>
+#include <sys/wait.h>
+#include <sys/syscall.h>
+#include <sys/user.h>
+#include <unistd.h>
+#include <errno.h>
+#include <stddef.h>
+#include <stdio.h>
+#include <err.h>
+#include <string.h>
+#include <sys/auxv.h>
+#include "utils.h"
+
+/* Bitness-agnostic defines for user_regs_struct fields. */
+#define user_syscall_nr	gpr[0]
+#define user_arg0		gpr[3]
+#define user_arg1		gpr[4]
+#define user_arg2		gpr[5]
+#define user_arg3		gpr[6]
+#define user_arg4		gpr[7]
+#define user_arg5		gpr[8]
+#define user_ip		nip
+
+#define PTRACE_SYSEMU		0x1d
+
+static int nerrs;
+
+static void wait_trap(pid_t chld)
+{
+	siginfo_t si;
+
+	if (waitid(P_PID, chld, &si, WEXITED|WSTOPPED) != 0)
+		err(1, "waitid");
+	if (si.si_pid != chld)
+		errx(1, "got unexpected pid in event\n");
+	if (si.si_code != CLD_TRAPPED)
+		errx(1, "got unexpected event type %d\n", si.si_code);
+}
+
+static void test_ptrace_syscall_restart(void)
+{
+	int status;
+	struct pt_regs regs;
+	pid_t chld;
+
+	printf("[RUN]\tptrace-induced syscall restart\n");
+
+	chld = fork();
+	if (chld < 0)
+		err(1, "fork");
+
+	/*
+	 * Child process is running 4 syscalls after ptrace.
+	 *
+	 * 1) getpid()
+	 * 2) gettid()
+	 * 3) tgkill() -> Send SIGSTOP
+	 * 4) gettid() -> Where the tests will happen essentially
+	 */
+	if (chld == 0) {
+		if (ptrace(PTRACE_TRACEME, 0, 0, 0) != 0)
+			err(1, "PTRACE_TRACEME");
+
+		pid_t pid = getpid(), tid = syscall(SYS_gettid);
+
+		printf("\tChild will make one syscall\n");
+		syscall(SYS_tgkill, pid, tid, SIGSTOP);
+
+		syscall(SYS_gettid, 10, 11, 12, 13, 14, 15);
+		_exit(0);
+	}
+	/* Parent process below */
+
+	/* Wait for SIGSTOP sent by tgkill above. */
+	if (waitpid(chld, &status, 0) != chld || !WIFSTOPPED(status))
+		err(1, "waitpid");
+
+	printf("[RUN]\tSYSEMU\n");
+	if (ptrace(PTRACE_SYSEMU, chld, 0, 0) != 0)
+		err(1, "PTRACE_SYSEMU");
+	wait_trap(chld);
+
+	if (ptrace(PTRACE_GETREGS, chld, 0, &regs) != 0)
+		err(1, "PTRACE_GETREGS");
+
+	/*
+	 * Ptrace trapped prior to executing the syscall, thus r3 still has
+	 * the syscall number instead of the sys_gettid() result
+	 */
+	if (regs.user_syscall_nr != SYS_gettid ||
+	    regs.user_arg0 != 10 || regs.user_arg1 != 11 ||
+	    regs.user_arg2 != 12 || regs.user_arg3 != 13 ||
+	    regs.user_arg4 != 14 || regs.user_arg5 != 15) {
+		printf("[FAIL]\tInitial args are wrong (nr=%lu, args=%lu %lu %lu %lu %lu %lu)\n",
+			(unsigned long)regs.user_syscall_nr,
+			(unsigned long)regs.user_arg0,
+			(unsigned long)regs.user_arg1,
+			(unsigned long)regs.user_arg2,
+			(unsigned long)regs.user_arg3,
+			(unsigned long)regs.user_arg4,
+			(unsigned long)regs.user_arg5);
+		 nerrs++;
+	} else {
+		printf("[OK]\tInitial nr and args are correct\n"); }
+
+	printf("[RUN]\tRestart the syscall (ip = 0x%lx)\n",
+	       (unsigned long)regs.user_ip);
+
+	/*
+	 * Rewind to retry the same syscall again. This will basically test
+	 * the rewind process together with PTRACE_SETREGS and PTRACE_GETREGS.
+	 */
+	regs.user_ip -= 4;
+	if (ptrace(PTRACE_SETREGS, chld, 0, &regs) != 0)
+		err(1, "PTRACE_SETREGS");
+
+	if (ptrace(PTRACE_SYSEMU, chld, 0, 0) != 0)
+		err(1, "PTRACE_SYSEMU");
+	wait_trap(chld);
+
+	if (ptrace(PTRACE_GETREGS, chld, 0, &regs) != 0)
+		err(1, "PTRACE_GETREGS");
+
+	if (regs.user_syscall_nr != SYS_gettid ||
+	    regs.user_arg0 != 10 || regs.user_arg1 != 11 ||
+	    regs.user_arg2 != 12 || regs.user_arg3 != 13 ||
+	    regs.user_arg4 != 14 || regs.user_arg5 != 15) {
+		printf("[FAIL]\tRestart nr or args are wrong (nr=%lu, args=%lu %lu %lu %lu %lu %lu)\n",
+			(unsigned long)regs.user_syscall_nr,
+			(unsigned long)regs.user_arg0,
+			(unsigned long)regs.user_arg1,
+			(unsigned long)regs.user_arg2,
+			(unsigned long)regs.user_arg3,
+			(unsigned long)regs.user_arg4,
+			(unsigned long)regs.user_arg5);
+		nerrs++;
+	} else {
+		printf("[OK]\tRestarted nr and args are correct\n");
+	}
+
+	printf("[RUN]\tChange nr and args and restart the syscall (ip = 0x%lx)\n",
+	       (unsigned long)regs.user_ip);
+
+	/*
+	 * Inject a new syscall (getpid) in the same place the previous
+	 * syscall (gettid), rewind and re-execute.
+	 */
+	regs.user_syscall_nr = SYS_getpid;
+	regs.user_arg0 = 20;
+	regs.user_arg1 = 21;
+	regs.user_arg2 = 22;
+	regs.user_arg3 = 23;
+	regs.user_arg4 = 24;
+	regs.user_arg5 = 25;
+	regs.user_ip -= 4;
+
+	if (ptrace(PTRACE_SETREGS, chld, 0, &regs) != 0)
+		err(1, "PTRACE_SETREGS");
+
+	if (ptrace(PTRACE_SYSEMU, chld, 0, 0) != 0)
+		err(1, "PTRACE_SYSEMU");
+	wait_trap(chld);
+
+	if (ptrace(PTRACE_GETREGS, chld, 0, &regs) != 0)
+		err(1, "PTRACE_GETREGS");
+
+	/* Check that ptrace stopped at the new syscall that was
+	 * injected, and guarantee that it haven't executed, i.e, user_args
+	 * contain the arguments and not the syscall return value, for
+	 * instance.
+	 */
+	if (regs.user_syscall_nr != SYS_getpid
+		|| regs.user_arg0 != 20 || regs.user_arg1 != 21
+		|| regs.user_arg2 != 22 || regs.user_arg3 != 23
+		|| regs.user_arg4 != 24 || regs.user_arg5 != 25) {
+
+		printf("[FAIL]\tRestart nr or args are wrong (nr=%lu, args=%lu %lu %lu %lu %lu %lu)\n",
+			(unsigned long)regs.user_syscall_nr,
+			(unsigned long)regs.user_arg0,
+			(unsigned long)regs.user_arg1,
+			(unsigned long)regs.user_arg2,
+			(unsigned long)regs.user_arg3,
+			(unsigned long)regs.user_arg4,
+			(unsigned long)regs.user_arg5);
+		nerrs++;
+	} else {
+		printf("[OK]\tReplacement nr and args are correct\n");
+	}
+
+	if (ptrace(PTRACE_CONT, chld, 0, 0) != 0)
+		err(1, "PTRACE_CONT");
+
+	if (waitpid(chld, &status, 0) != chld)
+		err(1, "waitpid");
+
+	/* Guarantee that the process executed properly, returning 0 */
+	if (!WIFEXITED(status) || WEXITSTATUS(status) != 0) {
+		printf("[FAIL]\tChild failed\n");
+		nerrs++;
+	} else {
+		printf("[OK]\tChild exited cleanly\n");
+	}
+}
+
+int ptrace_syscall(void)
+{
+	test_ptrace_syscall_restart();
+
+	return nerrs;
+}
+
+int main(void)
+{
+	return test_harness(ptrace_syscall, "ptrace_syscall");
+}
-- 
2.19.0

Re: [1/3] powerpc: Redefine TIF_32BITS thread flag

[Michael Ellerman]

On Thu, 2018-09-20 at 16:45:05 UTC, Breno Leitao wrote:
> Moving TIF_32BIT to use bit 20 instead of 4 in the task flag field.
> 
> This change is making room for an upcoming new task macro
> (_TIF_SYSCALL_EMU) which is preferred to set a bit in the lower 16-bits
> part of the word.
> 
> This upcoming flag macro will take part in a composed macro
> (_TIF_SYSCALL_DOTRACE) which will contain other flags as well, and it is
> preferred that the whole _TIF_SYSCALL_DOTRACE macro only sets the lower 16
> bits of a word, so, it could be handled using immediate operations (as load
> immediate, add immediate, ...) where the immediate operand (SI) is limited
> to 16-bits.
> 
> Another possible solution would be using the LOAD_REG_IMMEDIATE() macro
> to load a full 64-bits word immediate, but it takes 5 operations instead of
> one.
> 
> Having TIF_32BITS being redefined to use an upper bit is not a problem
> since there is only one place in the assembly code where TIF_32BIT is being
> used, and it could be replaced with an operation with right shift (addis),
> since it is used alone, i.e. not being part of a composed macro, which has
> different bits set, and would require LOAD_REG_IMMEDIATE().
> 
> Tested on a 64 bits Big Endian machine running a 32 bits task.
> 
> Signed-off-by: Breno Leitao <leitao@debian.org>

Series applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/16d7c69c898531210d13dbd1eb2053

cheers