[PATCH] s390/syscall: Set -ENOSYS return value before calling syscall_enter_from_user_mode_work()

[PATCH] s390/syscall: Set -ENOSYS return value before calling syscall_enter_from_user_mode_work()

[Heiko Carstens]

On s390 gpr2 contains both the system call return value and the system call
number. With [1] it is expected to set gpr2 to -ENOSYS before calling
syscall_enter_from_user_mode_work(), so that a skipped system call has a
defined default return value, and not the system call number like today.

To make that work and stay compatible with existing ptrace users, replace
PIF_SYSCALL_RET_SET with PIF_SYSCALL_ENTRY. PIF_SYSCALL_ENTRY is set before
syscall_enter_from_user_mode_work() is called and indicates that reads and
writes to gpr2 via PTRACE_PEEKUSR, PTRACE_POKEUSR, PTRACE_GETREGSET, and
PTRACE_SETREGSET are redirected to regs->int_code, which contains the
system call number. gpr2, which contains the system call return value, can
only be changed via syscall_set_return_value() (e.g. seccomp).

After syscall_enter_from_user_mode_work() returns, PIF_SYSCALL_ENTRY is
cleared and the code behaves as before.

PIF_SYSCALL_RET_SET was introduced with [2]. The flag was set by
syscall_set_return_value() and allowed seccomp to skip syscalls
and inject a return value despite the gpr2 aliasing.

Kernel seccomp and ptrace selftests, as well as strace selftests, show no
difference with and without this change.

[1] 05c033db7e9a ("entry, treewide: Make syscall_enter_from_user_mode[_work]() indicate syscall execution")
[2] 56e62a737028 ("s390: convert to generic entry")

Fixes: 05c033db7e9a ("entry, treewide: Make syscall_enter_from_user_mode[_work]() indicate syscall execution")
Cc: stable@vger.kernel.org
Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
---
 arch/s390/include/asm/ptrace.h  |  4 ++--
 arch/s390/include/asm/syscall.h |  1 -
 arch/s390/kernel/ptrace.c       | 28 +++++++++++++++++++++++-----
 arch/s390/kernel/syscall.c      | 19 +++++++++----------
 4 files changed, 34 insertions(+), 18 deletions(-)

diff --git a/arch/s390/include/asm/ptrace.h b/arch/s390/include/asm/ptrace.h
index 3e56ef8c268b..d8f3a2dbe689 100644
--- a/arch/s390/include/asm/ptrace.h
+++ b/arch/s390/include/asm/ptrace.h
@@ -15,14 +15,14 @@
 
 #define PIF_SYSCALL			0	/* inside a system call */
 #define PIF_PSW_ADDR_ADJUSTED		1	/* psw address has been adjusted */
-#define PIF_SYSCALL_RET_SET		2	/* return value was set via ptrace */
+#define PIF_SYSCALL_ENTRY		2	/* ptrace gpr2 access redirected to syscall nr */
 #define PIF_FTRACE_FULL_REGS		3	/* all register contents valid (ftrace) */
 #define PIF_WARN_TRAP_SKIPPED		4	/* __WARN_trap() was skipped */
 #define PIF_PGM_CHECK			5	/* inside a program check */
 
 #define _PIF_SYSCALL			BIT(PIF_SYSCALL)
 #define _PIF_ADDR_PSW_ADJUSTED		BIT(PIF_PSW_ADDR_ADJUSTED)
-#define _PIF_SYSCALL_RET_SET		BIT(PIF_SYSCALL_RET_SET)
+#define _PIF_SYSCALL_ENTRY		BIT(PIF_SYSCALL_ENTRY)
 #define _PIF_FTRACE_FULL_REGS		BIT(PIF_FTRACE_FULL_REGS)
 #define _PIF_WARN_TRAP_SKIPPED		BIT(PIF_WARN_TRAP_SKIPPED)
 #define _PIF_PGM_CHECK			BIT(PIF_PGM_CHECK)
diff --git a/arch/s390/include/asm/syscall.h b/arch/s390/include/asm/syscall.h
index 5f310caad1fc..47f66202977f 100644
--- a/arch/s390/include/asm/syscall.h
+++ b/arch/s390/include/asm/syscall.h
@@ -59,7 +59,6 @@ static inline void syscall_set_return_value(struct task_struct *task,
 					    struct pt_regs *regs,
 					    int error, long val)
 {
-	set_pt_regs_flag(regs, PIF_SYSCALL_RET_SET);
 	regs->gprs[2] = error ? error : val;
 }
 
diff --git a/arch/s390/kernel/ptrace.c b/arch/s390/kernel/ptrace.c
index d8d5e843859c..a3ae881bbff6 100644
--- a/arch/s390/kernel/ptrace.c
+++ b/arch/s390/kernel/ptrace.c
@@ -204,15 +204,24 @@ static unsigned long __peek_user(struct task_struct *child, addr_t addr)
 	addr_t offset, tmp;
 
 	if (addr < offsetof(struct user, regs.acrs)) {
-		/*
-		 * psw and gprs are stored on the stack
-		 */
-		tmp = *(addr_t *)((addr_t) &task_pt_regs(child)->psw + addr);
+		struct pt_regs *regs = task_pt_regs(child);
+
+		/* psw and gprs are on the stack. */
+		tmp = *(addr_t *)((addr_t)&regs->psw + addr);
 		if (addr == offsetof(struct user, regs.psw.mask)) {
 			/* Return a clean psw mask. */
 			tmp &= PSW_MASK_USER | PSW_MASK_RI;
 			tmp |= PSW_USER_BITS;
 		}
+		/*
+		 * During system call entry gpr2 contains the return
+		 * value. Redirect reads of gpr2 to int_code so that
+		 * readers see the system call number.
+		 */
+		if (addr == offsetof(struct user, regs.gprs[2])) {
+			if (test_pt_regs_flag(regs, PIF_SYSCALL_ENTRY))
+				tmp = syscall_get_nr(child, regs);
+		}
 
 	} else if (addr < offsetof(struct user, regs.orig_gpr2)) {
 		/*
@@ -347,8 +356,17 @@ static int __poke_user(struct task_struct *child, addr_t addr, addr_t data)
 		}
 
 		if (addr == offsetof(struct user, regs.gprs[2])) {
-			if (test_pt_regs_flag(regs, PIF_SYSCALL))
+			if (test_pt_regs_flag(regs, PIF_SYSCALL_ENTRY)) {
+				/*
+				 * System call entry path: only change system
+				 * call number in int_code, but leave gpr2
+				 * alone, which contains the return value.
+				 */
 				syscall_set_nr(child, regs, data);
+				return 0;
+			} else if (test_pt_regs_flag(regs, PIF_SYSCALL)) {
+				syscall_set_nr(child, regs, data);
+			}
 		}
 		*(addr_t *)((addr_t) &regs->psw + addr) = data;
 	} else if (addr < offsetof(struct user, regs.orig_gpr2)) {
diff --git a/arch/s390/kernel/syscall.c b/arch/s390/kernel/syscall.c
index bcc0b76bd868..923df3a6dc0d 100644
--- a/arch/s390/kernel/syscall.c
+++ b/arch/s390/kernel/syscall.c
@@ -117,24 +117,23 @@ void noinstr __do_syscall(struct pt_regs *regs, unsigned long flags)
 		regs->int_code &= ~0xffffUL;
 		regs->int_code |= nr;
 	}
-	regs->gprs[2] = nr;
 	if (nr == __NR_restart_syscall && !(current->restart_block.arch_data & 1)) {
 		regs->psw.addr = current->restart_block.arch_data;
 		current->restart_block.arch_data = 1;
 	}
 
-	permit = syscall_enter_from_user_mode_work(regs, &nr);
-
 	/*
-	 * In the s390 ptrace ABI, both the syscall number and the return value
-	 * use gpr2. However, userspace puts the syscall number either in the
-	 * svc instruction itself, or uses gpr1. To make at least skipping syscalls
-	 * work, the ptrace code sets PIF_SYSCALL_RET_SET, which is checked here
-	 * and if set, the syscall will be skipped.
+	 * gpr2 is the return value register. Set it to -ENOSYS so that a
+	 * skipped system call gets a defined default return value.
+	 * Set PIF_SYSCALL_ENTRY so that ptrace reads and writes to gpr2
+	 * are redirected to the system call number in int_code.
 	 */
-	if (unlikely(test_and_clear_pt_regs_flag(regs, PIF_SYSCALL_RET_SET) || !permit))
+	syscall_set_return_value(current, regs, -ENOSYS, 0);
+	set_pt_regs_flag(regs, PIF_SYSCALL_ENTRY);
+	permit = syscall_enter_from_user_mode_work(regs, &nr);
+	clear_pt_regs_flag(regs, PIF_SYSCALL_ENTRY);
+	if (unlikely(!permit))
 		goto out;
-	regs->gprs[2] = -ENOSYS;
 	if (likely(nr < NR_syscalls)) {
 		nr = array_index_nospec(nr, NR_syscalls);
 		regs->gprs[2] = sys_call_table[nr](regs);
-- 
2.53.0