[RFC PATCH 1/3] powerpc/kernel: Get pt_regs from r9 before calling do_syscall_trace_enter()

[RFC PATCH 1/3] powerpc/kernel: Get pt_regs from r9 before calling do_syscall_trace_enter()

[Michael Ellerman]

To call do_syscall_trace_enter() we need pt_regs in r3, but we don't need
to recalculate it based on r1, it's already in r9.

Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 arch/powerpc/kernel/entry_64.S | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
index afbc20019c2e..b55c393310f3 100644
--- a/arch/powerpc/kernel/entry_64.S
+++ b/arch/powerpc/kernel/entry_64.S
@@ -236,7 +236,9 @@ syscall_error:
 /* Traced system call support */
 syscall_dotrace:
 	bl	save_nvgprs
-	addi	r3,r1,STACK_FRAME_OVERHEAD
+
+	/* Get pt_regs into r3 */
+	mr	r3, r9
 	bl	do_syscall_trace_enter
 	/*
 	 * Restore argument registers possibly just changed.
-- 
2.1.0

[RFC PATCH 2/3] powerpc/kernel: Prepare for seccomp-filter in the 64-bit syscall path

[Michael Ellerman]

In order to support seccomp-filter we need to be able to cope with
seccomp potentially setting a return value for the syscall.

Currently this doesn't work, because we assume any failure from
do_syscall_trace_enter() should result in ENOSYS being returned to
userspace.

The complication is that we don't know if seccomp has set a return
value, in fact the failure may not even be caused by seccomp it may have
been from ptrace. So in some cases we should return ENOSYS, and in
others we should return whatever's in regs, but we don't know which at
this level.

The trick is to pre-fill regs->result with -ENOSYS, so that we return
that unless seccomp overwrites it with something else.

Note that it's negative ENOSYS, so that we still go via the
syscall_error path on the way out and set CR0[SO].

On the other hand in syscall_set_return_value() we set the return value
as it should be presented to userspace. That is mainly for consistency
with syscall_get_error().

Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 arch/powerpc/include/asm/syscall.h | 13 +++++++++++++
 arch/powerpc/kernel/entry_64.S     | 37 +++++++++++++++++++++++++++++++------
 2 files changed, 44 insertions(+), 6 deletions(-)

diff --git a/arch/powerpc/include/asm/syscall.h b/arch/powerpc/include/asm/syscall.h
index ff21b7a2f0cc..3f61ef03a54a 100644
--- a/arch/powerpc/include/asm/syscall.h
+++ b/arch/powerpc/include/asm/syscall.h
@@ -50,6 +50,12 @@ static inline void syscall_set_return_value(struct task_struct *task,
 					    struct pt_regs *regs,
 					    int error, long val)
 {
+	/*
+	 * We are setting the return value _as presented to userspace_.
+	 * So we set CR0[SO] and also negate error, making it positive.
+	 * That means we will _not_ go through the syscall_error path on the
+	 * exit to userspace.
+	 */
 	if (error) {
 		regs->ccr |= 0x10000000L;
 		regs->gpr[3] = -error;
@@ -57,6 +63,13 @@ static inline void syscall_set_return_value(struct task_struct *task,
 		regs->ccr &= ~0x10000000L;
 		regs->gpr[3] = val;
 	}
+
+	/*
+	 * Set regs->result to match r3. This mirrors the way a regular syscall
+	 * exit works. It also makes the return value juggling in
+	 * syscall_dotrace work.
+	 */
+	regs->result = regs->gpr[3];
 }
 
 static inline void syscall_get_arguments(struct task_struct *task,
diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
index b55c393310f3..3c912d9047c4 100644
--- a/arch/powerpc/kernel/entry_64.S
+++ b/arch/powerpc/kernel/entry_64.S
@@ -143,8 +143,7 @@ END_FW_FTR_SECTION_IFSET(FW_FEATURE_SPLPAR)
 	CURRENT_THREAD_INFO(r11, r1)
 	ld	r10,TI_FLAGS(r11)
 	andi.	r11,r10,_TIF_SYSCALL_DOTRACE
-	bne	syscall_dotrace
-.Lsyscall_dotrace_cont:
+	bne	syscall_dotrace /* does not return */
 	cmpldi	0,r0,NR_syscalls
 	bge-	syscall_enosys
 
@@ -235,27 +234,53 @@ syscall_error:
 	
 /* Traced system call support */
 syscall_dotrace:
+	/* Save non-volatile GPRs so seccomp/ptrace etc. can see them */
 	bl	save_nvgprs
 
+	/*
+	 * Seccomp may set regs->result, but we don't know at this level, so
+	 * preload result with ENOSYS here. That way below in the path to
+	 * .Lsyscall_exit we can load regs->result and get either ENOSYS, or
+	 * the value set by seccomp.
+	 */
+	li	r3,-ENOSYS
+	std	r3,RESULT(r1)
+
 	/* Get pt_regs into r3 */
 	mr	r3, r9
 	bl	do_syscall_trace_enter
+
 	/*
-	 * Restore argument registers possibly just changed.
-	 * We use the return value of do_syscall_trace_enter
-	 * for the call number to look up in the table (r0).
+	 * We use the return value of do_syscall_trace_enter() as the syscall
+	 * number. If the syscall was rejected for any reason
+	 * do_syscall_trace_enter() returns -1 and the test below against
+	 * NR_syscalls will fail.
 	 */
 	mr	r0,r3
+
+	/* Restore argument registers just clobbered and/or possibly changed. */
 	ld	r3,GPR3(r1)
 	ld	r4,GPR4(r1)
 	ld	r5,GPR5(r1)
 	ld	r6,GPR6(r1)
 	ld	r7,GPR7(r1)
 	ld	r8,GPR8(r1)
+
+	/* Repopulate r9 and r10 for the system_call path */
 	addi	r9,r1,STACK_FRAME_OVERHEAD
 	CURRENT_THREAD_INFO(r10, r1)
 	ld	r10,TI_FLAGS(r10)
-	b	.Lsyscall_dotrace_cont
+
+	/* Check the syscall number is valid */
+	cmpldi	0,r0,NR_syscalls
+	blt+	system_call
+
+	/*
+	 * Syscall number is bad, get the result, either ENOSYS from above or
+	 * something set by seccomp.
+	 */
+	ld	r3,RESULT(r1)
+	b	.Lsyscall_exit
 
 syscall_enosys:
 	li	r3,-ENOSYS
-- 
2.1.0

[RFC PATCH 3/3] powerpc: Enable seccomp-filter for 64-bit

[Michael Ellerman]

We now have the right pieces in place to enable seccomp-filter on
64-bit. We select HAVE_ARCH_SECCOMP_FILTER to enable the code, and we
also need to switch to using secure_computing() rather than the strict
variant.

Enabling support on 32-bit should just be a matter of fixing the
assembler in entry_32.S in a similar fashion to the 64-bit code.

Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 arch/powerpc/Kconfig         | 1 +
 arch/powerpc/kernel/ptrace.c | 6 ++++++
 2 files changed, 7 insertions(+)

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 190cc48abc0c..a90bbd2c3f3b 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -104,6 +104,7 @@ config PPC
 	select HAVE_EFFICIENT_UNALIGNED_ACCESS if !CPU_LITTLE_ENDIAN
 	select HAVE_KPROBES
 	select HAVE_ARCH_KGDB
+	select HAVE_ARCH_SECCOMP_FILTER if PPC64
 	select HAVE_KRETPROBES
 	select HAVE_ARCH_TRACEHOOK
 	select HAVE_MEMBLOCK
diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c
index f21897b42057..473d555ce60c 100644
--- a/arch/powerpc/kernel/ptrace.c
+++ b/arch/powerpc/kernel/ptrace.c
@@ -1772,7 +1772,13 @@ long do_syscall_trace_enter(struct pt_regs *regs)
 
 	user_exit();
 
+#ifdef CONFIG_PPC64
+	/* Do the secure computing check first */
+	if (secure_computing() == -1)
+		return -1L;
+#else
 	secure_computing_strict(regs->gpr[0]);
+#endif
 
 	if (test_thread_flag(TIF_SYSCALL_TRACE) &&
 	    tracehook_report_syscall_entry(regs))
-- 
2.1.0

Re: [RFC PATCH 2/3] powerpc/kernel: Prepare for seccomp-filter in the 64-bit syscall path

[Michael Neuling]

On Fri, 2015-05-15 at 18:29 +1000, Michael Ellerman wrote:
> In order to support seccomp-filter we need to be able to cope with
> seccomp potentially setting a return value for the syscall.
>=20
> Currently this doesn't work, because we assume any failure from
> do_syscall_trace_enter() should result in ENOSYS being returned to
> userspace.
>=20
> The complication is that we don't know if seccomp has set a return
> value, in fact the failure may not even be caused by seccomp it may have
> been from ptrace. So in some cases we should return ENOSYS, and in
> others we should return whatever's in regs, but we don't know which at
> this level.
>=20
> The trick is to pre-fill regs->result with -ENOSYS, so that we return
> that unless seccomp overwrites it with something else.
>=20
> Note that it's negative ENOSYS, so that we still go via the
> syscall_error path on the way out and set CR0[SO].
>=20
> On the other hand in syscall_set_return_value() we set the return value
> as it should be presented to userspace. That is mainly for consistency
> with syscall_get_error().
>=20
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
> ---
>  arch/powerpc/include/asm/syscall.h | 13 +++++++++++++
>  arch/powerpc/kernel/entry_64.S     | 37 +++++++++++++++++++++++++++++++-=
-----
>  2 files changed, 44 insertions(+), 6 deletions(-)
>=20
> diff --git a/arch/powerpc/include/asm/syscall.h b/arch/powerpc/include/as=
m/syscall.h
> index ff21b7a2f0cc..3f61ef03a54a 100644
> --- a/arch/powerpc/include/asm/syscall.h
> +++ b/arch/powerpc/include/asm/syscall.h
> @@ -50,6 +50,12 @@ static inline void syscall_set_return_value(struct tas=
k_struct *task,
>  					    struct pt_regs *regs,
>  					    int error, long val)
>  {
> +	/*
> +	 * We are setting the return value _as presented to userspace_.
> +	 * So we set CR0[SO] and also negate error, making it positive.
> +	 * That means we will _not_ go through the syscall_error path on the
> +	 * exit to userspace.
> +	 */
>  	if (error) {
>  		regs->ccr |=3D 0x10000000L;
>  		regs->gpr[3] =3D -error;
> @@ -57,6 +63,13 @@ static inline void syscall_set_return_value(struct tas=
k_struct *task,
>  		regs->ccr &=3D ~0x10000000L;
>  		regs->gpr[3] =3D val;
>  	}
> +
> +	/*
> +	 * Set regs->result to match r3. This mirrors the way a regular syscall
> +	 * exit works. It also makes the return value juggling in
> +	 * syscall_dotrace work.
> +	 */
> +	regs->result =3D regs->gpr[3];
>  }
> =20
>  static inline void syscall_get_arguments(struct task_struct *task,
> diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_6=
4.S
> index b55c393310f3..3c912d9047c4 100644
> --- a/arch/powerpc/kernel/entry_64.S
> +++ b/arch/powerpc/kernel/entry_64.S
> @@ -143,8 +143,7 @@ END_FW_FTR_SECTION_IFSET(FW_FEATURE_SPLPAR)
>  	CURRENT_THREAD_INFO(r11, r1)
>  	ld	r10,TI_FLAGS(r11)
>  	andi.	r11,r10,_TIF_SYSCALL_DOTRACE
> -	bne	syscall_dotrace
> -.Lsyscall_dotrace_cont:
> +	bne	syscall_dotrace /* does not return */
>  	cmpldi	0,r0,NR_syscalls
>  	bge-	syscall_enosys
> =20
> @@ -235,27 +234,53 @@ syscall_error:
>  =09
>  /* Traced system call support */
>  syscall_dotrace:
> +	/* Save non-volatile GPRs so seccomp/ptrace etc. can see them */
>  	bl	save_nvgprs
> =20
> +	/*
> +	 * Seccomp may set regs->result, but we don't know at this level, so
> +	 * preload result with ENOSYS here. That way below in the path to
> +	 * .Lsyscall_exit we can load regs->result and get either ENOSYS, or
> +	 * the value set by seccomp.
> +	 */
> +	li	r3,-ENOSYS
> +	std	r3,RESULT(r1)
> +
>  	/* Get pt_regs into r3 */
>  	mr	r3, r9
>  	bl	do_syscall_trace_enter
> +
>  	/*
> -	 * Restore argument registers possibly just changed.
> -	 * We use the return value of do_syscall_trace_enter
> -	 * for the call number to look up in the table (r0).
> +	 * We use the return value of do_syscall_trace_enter() as the syscall
> +	 * number. If the syscall was rejected for any reason
> +	 * do_syscall_trace_enter() returns -1 and the test below against
> +	 * NR_syscalls will fail.
>  	 */
>  	mr	r0,r3
> +
> +	/* Restore argument registers just clobbered and/or possibly changed. *=
/
>  	ld	r3,GPR3(r1)
>  	ld	r4,GPR4(r1)
>  	ld	r5,GPR5(r1)
>  	ld	r6,GPR6(r1)
>  	ld	r7,GPR7(r1)
>  	ld	r8,GPR8(r1)
> +
> +	/* Repopulate r9 and r10 for the system_call path */
>  	addi	r9,r1,STACK_FRAME_OVERHEAD
>  	CURRENT_THREAD_INFO(r10, r1)
>  	ld	r10,TI_FLAGS(r10)
> -	b	.Lsyscall_dotrace_cont
> +
> +	/* Check the syscall number is valid */
> +	cmpldi	0,r0,NR_syscalls
> +	blt+	system_call
> +
> +	/*
> +	 * Syscall number is bad, get the result, either ENOSYS from above or
> +	 * something set by seccomp.
> +	 */
> +	ld	r3,RESULT(r1)
> +	b	.Lsyscall_exit
>=20

Minor nit... one thing that confused me is this last label below here
"syscall_enosys".  You are talking about enosys a bunch above but if you
perform the syscall, you don't actually use it as you exit via the
branch just above.

Should we rename it to syscall_enosys_early: or something else?

>  syscall_enosys:
>  	li	r3,-ENOSYS