diff mbox

[RESEND,v4,2/2] arm64: Add seccomp support

Message ID 1404459115-8292-3-git-send-email-takahiro.akashi@linaro.org (mailing list archive)
State New, archived
Headers show

Commit Message

AKASHI Takahiro July 4, 2014, 7:31 a.m. UTC 
secure_computing() should always be called first in syscall_trace_enter().
If it returns non-zero, we should stop further handling. Then that system
call may eventually fail, be trapped or the process itself be killed
depending on loaded rules.
In this case, syscall_trace_enter() returns a dedicated value in order to
skip a normal syscall table lookup because a seccomp rule may have already
overridden errno.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 arch/arm64/Kconfig               |   14 ++++++++++++++
 arch/arm64/include/asm/seccomp.h |   25 +++++++++++++++++++++++++
 arch/arm64/include/asm/unistd.h  |    3 +++
 arch/arm64/kernel/entry.S        |    4 ++++
 arch/arm64/kernel/ptrace.c       |    6 ++++++
 5 files changed, 52 insertions(+)
 create mode 100644 arch/arm64/include/asm/seccomp.h

Comments

Will Deacon July 9, 2014, 11:12 a.m. UTC | #1 
Hi Akashi,

On Fri, Jul 04, 2014 at 08:31:55AM +0100, AKASHI Takahiro wrote:
> secure_computing() should always be called first in syscall_trace_enter().
> If it returns non-zero, we should stop further handling. Then that system
> call may eventually fail, be trapped or the process itself be killed
> depending on loaded rules.
> In this case, syscall_trace_enter() returns a dedicated value in order to
> skip a normal syscall table lookup because a seccomp rule may have already
> overridden errno.

[...]

> diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c
> index 70526cf..baab5fc 100644
> --- a/arch/arm64/kernel/ptrace.c
> +++ b/arch/arm64/kernel/ptrace.c
> @@ -21,12 +21,14 @@
>  
>  #include <linux/audit.h>
>  #include <linux/compat.h>
> +#include <linux/errno.h>
>  #include <linux/kernel.h>
>  #include <linux/sched.h>
>  #include <linux/mm.h>
>  #include <linux/smp.h>
>  #include <linux/ptrace.h>
>  #include <linux/user.h>
> +#include <linux/seccomp.h>
>  #include <linux/security.h>
>  #include <linux/init.h>
>  #include <linux/signal.h>
> @@ -1109,6 +1111,10 @@ static void tracehook_report_syscall(struct pt_regs *regs,
>  
>  asmlinkage int syscall_trace_enter(struct pt_regs *regs)
>  {
> +	if (secure_computing(regs->syscallno) == -1)
> +		/* seccomp failures shouldn't expose any additional code. */
> +		return -EPERM;
> +
>  	if (test_thread_flag(TIF_SYSCALL_TRACE))
>  		tracehook_report_syscall(regs, PTRACE_SYSCALL_ENTER);

We return regs->syscallno immediately after this, so we have the same issue
that Kees identified for arch/arm/. Did you follow the discussion I had with
Andy?

Will
AKASHI Takahiro July 10, 2014, 4:33 a.m. UTC | #2 
Will,

 >  (1) Updating syscallno based on w8, but this ties us to the current ABI
 >      and could get messy if this register changes in the future.

So, is this the conclusion that I should follow?

-Takahiro AKASHI


On 07/09/2014 01:12 PM, Will Deacon wrote:
> Hi Akashi,
>
> On Fri, Jul 04, 2014 at 08:31:55AM +0100, AKASHI Takahiro wrote:
>> secure_computing() should always be called first in syscall_trace_enter().
>> If it returns non-zero, we should stop further handling. Then that system
>> call may eventually fail, be trapped or the process itself be killed
>> depending on loaded rules.
>> In this case, syscall_trace_enter() returns a dedicated value in order to
>> skip a normal syscall table lookup because a seccomp rule may have already
>> overridden errno.
>
> [...]
>
>> diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c
>> index 70526cf..baab5fc 100644
>> --- a/arch/arm64/kernel/ptrace.c
>> +++ b/arch/arm64/kernel/ptrace.c
>> @@ -21,12 +21,14 @@
>>
>>   #include <linux/audit.h>
>>   #include <linux/compat.h>
>> +#include <linux/errno.h>
>>   #include <linux/kernel.h>
>>   #include <linux/sched.h>
>>   #include <linux/mm.h>
>>   #include <linux/smp.h>
>>   #include <linux/ptrace.h>
>>   #include <linux/user.h>
>> +#include <linux/seccomp.h>
>>   #include <linux/security.h>
>>   #include <linux/init.h>
>>   #include <linux/signal.h>
>> @@ -1109,6 +1111,10 @@ static void tracehook_report_syscall(struct pt_regs *regs,
>>
>>   asmlinkage int syscall_trace_enter(struct pt_regs *regs)
>>   {
>> +	if (secure_computing(regs->syscallno) == -1)
>> +		/* seccomp failures shouldn't expose any additional code. */
>> +		return -EPERM;
>> +
>>   	if (test_thread_flag(TIF_SYSCALL_TRACE))
>>   		tracehook_report_syscall(regs, PTRACE_SYSCALL_ENTER);
>
> We return regs->syscallno immediately after this, so we have the same issue
> that Kees identified for arch/arm/. Did you follow the discussion I had with
> Andy?
>
> Will
>
Will Deacon July 10, 2014, 8:48 a.m. UTC | #3 
On Thu, Jul 10, 2014 at 05:33:50AM +0100, AKASHI Takahiro wrote:
> Will,
> 
>  >  (1) Updating syscallno based on w8, but this ties us to the current ABI
>  >      and could get messy if this register changes in the future.
> 
> So, is this the conclusion that I should follow?

I think so, with the exception that if the tracer/debugger sets it to -1 to
abort the syscall, then we should restore the original value.

Will
diff mbox

Patch

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 3a18571..eeac003 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -32,6 +32,7 @@  config ARM64
 	select HAVE_ARCH_AUDITSYSCALL
 	select HAVE_ARCH_JUMP_LABEL
 	select HAVE_ARCH_KGDB
+	select HAVE_ARCH_SECCOMP_FILTER
 	select HAVE_ARCH_TRACEHOOK
 	select HAVE_C_RECORDMCOUNT
 	select HAVE_DEBUG_BUGVERBOSE
@@ -259,6 +260,19 @@  config ARCH_HAS_CACHE_LINE_SIZE
 
 source "mm/Kconfig"
 
+config SECCOMP
+	bool "Enable seccomp to safely compute untrusted bytecode"
+	---help---
+	  This kernel feature is useful for number crunching applications
+	  that may need to compute untrusted bytecode during their
+	  execution. By using pipes or other transports made available to
+	  the process as file descriptors supporting the read/write
+	  syscalls, it's possible to isolate those applications in
+	  their own address space using seccomp. Once seccomp is
+	  enabled via prctl(PR_SET_SECCOMP), it cannot be disabled
+	  and the task is only allowed to execute a few safe syscalls
+	  defined by each seccomp mode.
+
 config XEN_DOM0
 	def_bool y
 	depends on XEN
diff --git a/arch/arm64/include/asm/seccomp.h b/arch/arm64/include/asm/seccomp.h
new file mode 100644
index 0000000..c76fac9
--- /dev/null
+++ b/arch/arm64/include/asm/seccomp.h
@@ -0,0 +1,25 @@ 
+/*
+ * arch/arm64/include/asm/seccomp.h
+ *
+ * Copyright (C) 2014 Linaro Limited
+ * Author: AKASHI Takahiro <takahiro.akashi@linaro.org>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+#ifndef _ASM_SECCOMP_H
+#define _ASM_SECCOMP_H
+
+#include <asm/unistd.h>
+
+#ifdef CONFIG_COMPAT
+#define __NR_seccomp_read_32		__NR_compat_read
+#define __NR_seccomp_write_32		__NR_compat_write
+#define __NR_seccomp_exit_32		__NR_compat_exit
+#define __NR_seccomp_sigreturn_32	__NR_compat_rt_sigreturn
+#endif /* CONFIG_COMPAT */
+
+#include <asm-generic/seccomp.h>
+
+#endif /* _ASM_SECCOMP_H */
diff --git a/arch/arm64/include/asm/unistd.h b/arch/arm64/include/asm/unistd.h
index c980ab7..729c155 100644
--- a/arch/arm64/include/asm/unistd.h
+++ b/arch/arm64/include/asm/unistd.h
@@ -31,6 +31,9 @@ 
  * Compat syscall numbers used by the AArch64 kernel.
  */
 #define __NR_compat_restart_syscall	0
+#define __NR_compat_exit		1
+#define __NR_compat_read		3
+#define __NR_compat_write		4
 #define __NR_compat_sigreturn		119
 #define __NR_compat_rt_sigreturn	173
 
diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
index 5141e79..fe55b4c 100644
--- a/arch/arm64/kernel/entry.S
+++ b/arch/arm64/kernel/entry.S
@@ -628,6 +628,10 @@  ENDPROC(el0_svc)
 __sys_trace:
 	mov	x0, sp
 	bl	syscall_trace_enter
+#ifdef CONFIG_SECCOMP
+	cmp	w0, #-EPERM			// check seccomp result
+	b.eq	ret_to_user			// -EPERM means 'rejected'
+#endif
 	adr	lr, __sys_trace_return		// return address
 	uxtw	scno, w0			// syscall number (possibly new)
 	mov	x1, sp				// pointer to regs
diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c
index 70526cf..baab5fc 100644
--- a/arch/arm64/kernel/ptrace.c
+++ b/arch/arm64/kernel/ptrace.c
@@ -21,12 +21,14 @@ 
 
 #include <linux/audit.h>
 #include <linux/compat.h>
+#include <linux/errno.h>
 #include <linux/kernel.h>
 #include <linux/sched.h>
 #include <linux/mm.h>
 #include <linux/smp.h>
 #include <linux/ptrace.h>
 #include <linux/user.h>
+#include <linux/seccomp.h>
 #include <linux/security.h>
 #include <linux/init.h>
 #include <linux/signal.h>
@@ -1109,6 +1111,10 @@  static void tracehook_report_syscall(struct pt_regs *regs,
 
 asmlinkage int syscall_trace_enter(struct pt_regs *regs)
 {
+	if (secure_computing(regs->syscallno) == -1)
+		/* seccomp failures shouldn't expose any additional code. */
+		return -EPERM;
+
 	if (test_thread_flag(TIF_SYSCALL_TRACE))
 		tracehook_report_syscall(regs, PTRACE_SYSCALL_ENTER);