[PATCH 2/2] arm64: Abstract syscallno manipulation
From: Will Deacon <hidden>
Date: 2017-08-01 11:49:11
On Tue, Jul 18, 2017 at 01:41:44PM +0100, Dave Martin wrote:
quoted hunk ↗ jump to hunk
The -1 "no syscall" value is written in various ways, shared with the user ABI in some places, and generally obscure. This patch attempts to make things a little more consistent and readable by replacing all these uses with a single #define. A couple of symbolic helpers are provided to clarify the intent further. Because the in-syscall check in do_signal() is changed from >= 0 to != NO_SYSCALL by this patch, different behaviour may be observable if syscallno is set to values less than -1 by a tracer. However, this is not different from the behaviour that is already observable if a tracer sets syscallno to a value >= __NR_(compat_)syscalls. It appears that this can cause spurious syscall restarting, but that is not a new behaviour either, and does not appear harmful. Signed-off-by: Dave Martin <Dave.Martin@arm.com> --- arch/arm64/include/asm/processor.h | 2 +- arch/arm64/include/asm/ptrace.h | 21 +++++++++++++++++++++ arch/arm64/kernel/entry.S | 10 ++++------ arch/arm64/kernel/ptrace.c | 2 +- arch/arm64/kernel/signal.c | 8 ++++---- arch/arm64/kernel/signal32.c | 2 +- 6 files changed, 32 insertions(+), 13 deletions(-)diff --git a/arch/arm64/include/asm/processor.h b/arch/arm64/include/asm/processor.h index 379def1..b7334f1 100644 --- a/arch/arm64/include/asm/processor.h +++ b/arch/arm64/include/asm/processor.h@@ -112,7 +112,7 @@ void tls_preserve_current_state(void); static inline void start_thread_common(struct pt_regs *regs, unsigned long pc) { memset(regs, 0, sizeof(*regs)); - regs->syscallno = ~0; + forget_syscall(regs); regs->pc = pc; }diff --git a/arch/arm64/include/asm/ptrace.h b/arch/arm64/include/asm/ptrace.h index 21c87dc..3a2d6cc 100644 --- a/arch/arm64/include/asm/ptrace.h +++ b/arch/arm64/include/asm/ptrace.h@@ -72,8 +72,19 @@ #define COMPAT_PT_TEXT_ADDR 0x10000 #define COMPAT_PT_DATA_ADDR 0x10004 #define COMPAT_PT_TEXT_END_ADDR 0x10008 + +/* + * If pt_regs.syscallno == NO_SYSCALL, then the thread is not executing + * a syscall -- i.e., its most recent entry into the kernel from + * userspace was not via SVC, or otherwise a tracer cancelled the syscall. + * + * This must have the value -1, for ABI compatibility with ptrace etc. + */ +#define NO_SYSCALL (-1) + #ifndef __ASSEMBLY__ #include <linux/bug.h> +#include <linux/types.h> /* sizeof(struct user) for AArch32 */ #define COMPAT_USER_SZ 296@@ -128,6 +139,16 @@ struct pt_regs { u64 unused; // maintain 16 byte alignment }; +static inline bool in_syscall(int syscallno) +{ + return syscallno != NO_SYSCALL; +}
I think it would be cleaner for this to take the pt_regs as an argument...
quoted hunk ↗ jump to hunk
diff --git a/arch/arm64/kernel/signal.c b/arch/arm64/kernel/signal.c index 4d04b89..3a59dae 100644 --- a/arch/arm64/kernel/signal.c +++ b/arch/arm64/kernel/signal.c@@ -387,7 +387,7 @@ static int restore_sigframe(struct pt_regs *regs, /* * Avoid sys_rt_sigreturn() restarting. */ - regs->syscallno = ~0; + forget_syscall(regs); err |= !valid_user_regs(®s->user_regs, current); if (err == 0)@@ -679,7 +679,7 @@ static void do_signal(struct pt_regs *regs) /* * If we were from a system call, check for system call restarting... */ - if (syscall >= 0) { + if (in_syscall(syscall)) {
.. then update the two callsites in here to pass the regs, and remove the syscall local variable. With that: Acked-by: Will Deacon <redacted> Will