This patch enables secure computing (system call filtering) on arm64. System calls can be allowed or denied by loaded bpf-style rules. Architecture specific part is to run secure_computing() on syscall entry and check the result. See [1/2]
Prerequisites are: * "arm64: Add audit support" patch * "arm64: make a single hook to syscall_trace() for all syscall features" patch
This code is tested on ARMv8 fast model using libseccomp v2.1.1 with modifications for arm64 and verified by its "live" tests, 20, 21 and 24.
AKASHI Takahiro (2): arm64: Add seccomp support arm64: is_compat_task is defined both in asm/compat.h and linux/compat.h
arch/arm64/Kconfig | 17 +++++++++++++++++ arch/arm64/include/asm/compat.h | 2 ++ arch/arm64/include/asm/seccomp.h | 28 ++++++++++++++++++++++++++++ arch/arm64/include/asm/unistd.h | 3 +++ arch/arm64/kernel/entry.S | 4 ++++ arch/arm64/kernel/ptrace.c | 5 +++++ 6 files changed, 59 insertions(+) create mode 100644 arch/arm64/include/asm/seccomp.h
secure_computing() should always be called first in syscall_trace(), and 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.
This patch also defines specific system call numbers, __NR_seccomp_*, solely used by secure_computing() for seccomp mode 1 (only read, write, exit and sigreturn are allowd).
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org --- arch/arm64/Kconfig | 17 +++++++++++++++++ arch/arm64/include/asm/seccomp.h | 28 ++++++++++++++++++++++++++++ arch/arm64/include/asm/unistd.h | 3 +++ arch/arm64/kernel/entry.S | 4 ++++ arch/arm64/kernel/ptrace.c | 5 +++++ 5 files changed, 57 insertions(+) create mode 100644 arch/arm64/include/asm/seccomp.h
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig index a21455e..a0102f7 100644 --- a/arch/arm64/Kconfig +++ b/arch/arm64/Kconfig @@ -27,6 +27,7 @@ config ARM64 select HARDIRQS_SW_RESEND select HAVE_ARCH_AUDITSYSCALL select HAVE_ARCH_JUMP_LABEL + select HAVE_ARCH_SECCOMP_FILTER select HAVE_ARCH_TRACEHOOK select HAVE_DEBUG_BUGVERBOSE select HAVE_DEBUG_KMEMLEAK @@ -222,6 +223,22 @@ config HAVE_ARCH_TRANSPARENT_HUGEPAGE
source "mm/Kconfig"
+config SECCOMP + def_bool y + prompt "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. + + If unsure, say Y. Only embedded should say N here. + 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..3482155 --- /dev/null +++ b/arch/arm64/include/asm/seccomp.h @@ -0,0 +1,28 @@ +/* + * 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_sigreturn +#endif /* CONFIG_COMPAT */ + +#define __NR_seccomp_read __NR_read +#define __NR_seccomp_write __NR_write +#define __NR_seccomp_exit __NR_exit +#define __NR_seccomp_sigreturn __NR_rt_sigreturn + +#endif /* _ASM_SECCOMP_H */ diff --git a/arch/arm64/include/asm/unistd.h b/arch/arm64/include/asm/unistd.h index 4a09fdb..05f2db3 100644 --- a/arch/arm64/include/asm/unistd.h +++ b/arch/arm64/include/asm/unistd.h @@ -30,6 +30,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 96c2d03..55d4e6c 100644 --- a/arch/arm64/kernel/entry.S +++ b/arch/arm64/kernel/entry.S @@ -651,6 +651,10 @@ __sys_trace: mov x1, sp mov w0, #0 // trace entry bl syscall_trace +#ifdef CONFIG_SECCOMP + cmp w0, #0 // check seccomp result + b.lt ret_to_user // -1 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 8cdba09..3bfe398 100644 --- a/arch/arm64/kernel/ptrace.c +++ b/arch/arm64/kernel/ptrace.c @@ -26,6 +26,7 @@ #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> @@ -1064,6 +1065,10 @@ asmlinkage int syscall_trace(int dir, struct pt_regs *regs) { unsigned long saved_reg;
+ if (!dir && secure_computing((int)regs->syscallno)) + /* seccomp failures shouldn't expose any additional code. */ + return -1; + if (is_compat_task()) { /* AArch32 uses ip (r12) for scratch */ saved_reg = regs->regs[12];
On Friday 07 February 2014 19:11:31 AKASHI Takahiro wrote:
diff --git a/arch/arm64/include/asm/seccomp.h b/arch/arm64/include/asm/seccomp.h new file mode 100644 index 0000000..3482155 --- /dev/null +++ b/arch/arm64/include/asm/seccomp.h @@ -0,0 +1,28 @@ +/*
- 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_sigreturn +#endif /* CONFIG_COMPAT */
+#define __NR_seccomp_read __NR_read +#define __NR_seccomp_write __NR_write +#define __NR_seccomp_exit __NR_exit +#define __NR_seccomp_sigreturn __NR_rt_sigreturn
+#endif /* _ASM_SECCOMP_H */
This file looks extremely generic and can be shared by every architecture other than MIPS for all I can tell.
Please add it to include/asm-generic instead of arch/arm64, and add a line to arch/arm64/include/asm/Kbuild.
Arnd
On 7 February 2014 23:44, Arnd Bergmann arnd@arndb.de wrote:
On Friday 07 February 2014 19:11:31 AKASHI Takahiro wrote:
diff --git a/arch/arm64/include/asm/seccomp.h
b/arch/arm64/include/asm/seccomp.h
new file mode 100644 index 0000000..3482155 --- /dev/null +++ b/arch/arm64/include/asm/seccomp.h @@ -0,0 +1,28 @@ +/*
- 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_sigreturn +#endif /* CONFIG_COMPAT */
+#define __NR_seccomp_read __NR_read +#define __NR_seccomp_write __NR_write +#define __NR_seccomp_exit __NR_exit +#define __NR_seccomp_sigreturn __NR_rt_sigreturn
+#endif /* _ASM_SECCOMP_H */
This file looks extremely generic and can be shared by every architecture other than MIPS for all I can tell.
Please add it to include/asm-generic instead of arch/arm64, and add a line to arch/arm64/include/asm/Kbuild.
Thank you for your comment. I'm afraid that I can't do so because how compat syscall numbers are named varies from arch to arch. __NR_compat_read is used only on arm64, while __NR_ia32_read on x86. On other archs, __NR_read is common to 32-bit and 64-bit tasks.
-Takahiro AKASHI
Arnd
On Wednesday 12 February 2014, Takahiro Akashi wrote:
Thank you for your comment. I'm afraid that I can't do so because how compat syscall numbers are named varies from arch to arch. __NR_compat_read is used only on arm64, while __NR_ia32_read on x86. On other archs, __NR_read is common to 32-bit and 64-bit tasks.
It's fine, I'm the maintainer for asm-generic ;-)
All future architectures are required to do it the same way as arm64 and use the generic syscall ABI. It just means we won't be able to share this header with x86 or other architectures that use a nonstandard string. We should have it in the generic place even if arm64 is the only user for now.
Arnd
On Wednesday 12 February 2014, Arnd Bergmann wrote:
On Wednesday 12 February 2014, Takahiro Akashi wrote:
Thank you for your comment. I'm afraid that I can't do so because how compat syscall numbers are named varies from arch to arch. __NR_compat_read is used only on arm64, while __NR_ia32_read on x86. On other archs, __NR_read is common to 32-bit and 64-bit tasks.
It's fine, I'm the maintainer for asm-generic ;-)
All future architectures are required to do it the same way as arm64 and use the generic syscall ABI. It just means we won't be able to share this header with x86 or other architectures that use a nonstandard string. We should have it in the generic place even if arm64 is the only user for now.
Ok, I was wrong. The generic header file should only be used for architectures on which both ABIs use the generic syscall numbers, and arm64 obviously doesn't do that for arm32 compatibility.
The proper generic header file should contain
#if defined(CONFIG_COMPAT) && !defined(__NR_seccomp_read_32) #define __NR_seccomp_read_32 __NR_read #define __NR_seccomp_write_32 __NR_write #define __NR_seccomp_exit_32 __NR_exit #define __NR_seccomp_sigreturn_32 __NR_rt_sigreturn #endif /* CONFIG_COMPAT && ! already defined */
#define __NR_seccomp_read __NR_read #define __NR_seccomp_write __NR_write #define __NR_seccomp_exit __NR_exit #define __NR_seccomp_sigreturn __NR_rt_sigreturn
and then have arm64 override the first four by defining its own. I would still prefer having it done this way, but it actually doesn't gain us that much since this default wouldn't work on any 64-bit architecture currently implementing seccomp. I'll leave it up to you whether you want to do it or not.
Arnd
On Fri, Feb 07, 2014 at 10:11:31AM +0000, AKASHI Takahiro wrote:
--- a/arch/arm64/kernel/ptrace.c +++ b/arch/arm64/kernel/ptrace.c @@ -26,6 +26,7 @@ #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> @@ -1064,6 +1065,10 @@ asmlinkage int syscall_trace(int dir, struct pt_regs *regs) { unsigned long saved_reg;
- if (!dir && secure_computing((int)regs->syscallno))
/* seccomp failures shouldn't expose any additional code. */return -1;
That's only restricted to the arm64 code but could we use a more meaningful error number?
On 02/19/2014 12:38 AM, Catalin Marinas wrote:
On Fri, Feb 07, 2014 at 10:11:31AM +0000, AKASHI Takahiro wrote:
--- a/arch/arm64/kernel/ptrace.c +++ b/arch/arm64/kernel/ptrace.c @@ -26,6 +26,7 @@ #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> @@ -1064,6 +1065,10 @@ asmlinkage int syscall_trace(int dir, struct pt_regs *regs) { unsigned long saved_reg;
- if (!dir && secure_computing((int)regs->syscallno))
/* seccomp failures shouldn't expose any additional code. */return -1;That's only restricted to the arm64 code but could we use a more meaningful error number?
Other architectures, including arm, also return just -1 in syscall_trace_enter(), but of course, we can use another value, say, -EPERM or -ENOSYS?
-Takahiro AKASHI
On Wed, Feb 19, 2014 at 11:39:09AM +0000, AKASHI Takahiro wrote:
On 02/19/2014 12:38 AM, Catalin Marinas wrote:
On Fri, Feb 07, 2014 at 10:11:31AM +0000, AKASHI Takahiro wrote:
--- a/arch/arm64/kernel/ptrace.c +++ b/arch/arm64/kernel/ptrace.c @@ -26,6 +26,7 @@ #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> @@ -1064,6 +1065,10 @@ asmlinkage int syscall_trace(int dir, struct pt_regs *regs) { unsigned long saved_reg;
- if (!dir && secure_computing((int)regs->syscallno))
/* seccomp failures shouldn't expose any additional code. */return -1;That's only restricted to the arm64 code but could we use a more meaningful error number?
Other architectures, including arm, also return just -1 in syscall_trace_enter(), but of course, we can use another value, say, -EPERM or -ENOSYS?
Actually we have another case of setting regs->syscallno = ~0UL in the same function, so we could do the same (also in line with entry.S).
On 02/20/2014 01:41 AM, Catalin Marinas wrote:
On Wed, Feb 19, 2014 at 11:39:09AM +0000, AKASHI Takahiro wrote:
On 02/19/2014 12:38 AM, Catalin Marinas wrote:
On Fri, Feb 07, 2014 at 10:11:31AM +0000, AKASHI Takahiro wrote:
--- a/arch/arm64/kernel/ptrace.c +++ b/arch/arm64/kernel/ptrace.c @@ -26,6 +26,7 @@ #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> @@ -1064,6 +1065,10 @@ asmlinkage int syscall_trace(int dir, struct pt_regs *regs) { unsigned long saved_reg;
- if (!dir && secure_computing((int)regs->syscallno))
/* seccomp failures shouldn't expose any additional code. */return -1;That's only restricted to the arm64 code but could we use a more meaningful error number?
Other architectures, including arm, also return just -1 in syscall_trace_enter(), but of course, we can use another value, say, -EPERM or -ENOSYS?
Actually we have another case of setting regs->syscallno = ~0UL in the same function, so we could do the same (also in line with entry.S).
I believe that I got you now, but we need to distinguish failure case of seccomp and the existing (~0UL) case. In former case, depending on a bpf rule loaded into the kernel, errno may be assigned to any arbitrary number (not necessarily ENOSYS). So I will use another value for this specific seccomp case.
Thanks, -Takahiro AKASHI
kernel/seccomp.c includes linux/compat.h and, indicrectly, asm/compat.h via asm/syscall.h. Due to the duplicated definition of is_compat_task, compiling this file will fail in the case of !CONFIG_COMPAT. This patch makes the definition in asm/compat.h valid only if necessary.
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org --- arch/arm64/include/asm/compat.h | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/arch/arm64/include/asm/compat.h b/arch/arm64/include/asm/compat.h index fda2704..72f3b18 100644 --- a/arch/arm64/include/asm/compat.h +++ b/arch/arm64/include/asm/compat.h @@ -305,10 +305,12 @@ static inline int is_compat_thread(struct thread_info *thread)
#else /* !CONFIG_COMPAT */
+#ifndef is_compat_task /* it's there in linux/compat.h */ static inline int is_compat_task(void) { return 0; } +#endif
static inline int is_compat_thread(struct thread_info *thread) {
On Fri, Feb 07, 2014 at 10:11:32AM +0000, AKASHI Takahiro wrote:
kernel/seccomp.c includes linux/compat.h and, indicrectly, asm/compat.h via asm/syscall.h. Due to the duplicated definition of is_compat_task, compiling this file will fail in the case of !CONFIG_COMPAT. This patch makes the definition in asm/compat.h valid only if necessary.
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org
arch/arm64/include/asm/compat.h | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/arch/arm64/include/asm/compat.h b/arch/arm64/include/asm/compat.h index fda2704..72f3b18 100644 --- a/arch/arm64/include/asm/compat.h +++ b/arch/arm64/include/asm/compat.h @@ -305,10 +305,12 @@ static inline int is_compat_thread(struct thread_info *thread) #else /* !CONFIG_COMPAT */ +#ifndef is_compat_task /* it's there in linux/compat.h */ static inline int is_compat_task(void) { return 0; } +#endif
This is horrible! Can we instead include linux/compat.h where we need this macro and then remove this definition?
Will
On 02/18/2014 04:32 AM, Will Deacon wrote:
On Fri, Feb 07, 2014 at 10:11:32AM +0000, AKASHI Takahiro wrote:
kernel/seccomp.c includes linux/compat.h and, indicrectly, asm/compat.h via asm/syscall.h. Due to the duplicated definition of is_compat_task, compiling this file will fail in the case of !CONFIG_COMPAT. This patch makes the definition in asm/compat.h valid only if necessary.
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org
arch/arm64/include/asm/compat.h | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/arch/arm64/include/asm/compat.h b/arch/arm64/include/asm/compat.h index fda2704..72f3b18 100644 --- a/arch/arm64/include/asm/compat.h +++ b/arch/arm64/include/asm/compat.h @@ -305,10 +305,12 @@ static inline int is_compat_thread(struct thread_info *thread)
#else /* !CONFIG_COMPAT */
+#ifndef is_compat_task /* it's there in linux/compat.h */ static inline int is_compat_task(void) { return 0; } +#endif
This is horrible! Can we instead include linux/compat.h where we need this macro and then remove this definition?
It's fine with me, but asm/compat.h is also included in hw_breakpoint.c, process.c, ptrace.c and signal.c.
After replacing asm/compat.h to linux/compat.h in these files, I could successfully built the kernel, but I can't guarantee that the kernel works without any problem :-)
Anyway I will go forward with this change.
-Takahiro AKASHI
Will
This patch enables secure computing (system call filtering) on arm64. System calls can be allowed or denied by loaded bpf-style rules. Architecture specific part is to run secure_computing() on syscall entry and check the result. See [2/3]
Prerequisites are: * "arm64: Add audit support" patch * "arm64: make a single hook to syscall_trace() for all syscall features" patch
This code is tested on ARMv8 fast model using libseccomp v2.1.1 with modifications for arm64 and verified by its "live" tests, 20, 21 and 24.
Changes v1 -> v2: * added generic seccomp.h for arm64 to utilize it [1,2/3] * changed syscall_trace() to return more meaningful value (-EPERM) on seccomp failure case [2/3] * aligned with the change in "arm64: make a single hook to syscall_trace() for all syscall features" v2 [2/3] * removed is_compat_task() definition from compat.h [3/3]
AKASHI Takahiro (3): asm-generic: Add generic seccomp.h for secure computing mode 1 arm64: Add seccomp support arm64: is_compat_task is defined both in asm/compat.h and linux/compat.h
arch/arm64/Kconfig | 17 +++++++++++++++++ arch/arm64/include/asm/compat.h | 5 ----- arch/arm64/include/asm/seccomp.h | 25 +++++++++++++++++++++++++ arch/arm64/include/asm/unistd.h | 3 +++ arch/arm64/kernel/entry.S | 4 ++++ arch/arm64/kernel/hw_breakpoint.c | 2 +- arch/arm64/kernel/process.c | 2 +- arch/arm64/kernel/ptrace.c | 8 +++++++- arch/arm64/kernel/signal.c | 2 +- include/asm-generic/seccomp.h | 28 ++++++++++++++++++++++++++++ 10 files changed, 87 insertions(+), 9 deletions(-) create mode 100644 arch/arm64/include/asm/seccomp.h create mode 100644 include/asm-generic/seccomp.h
Those values (__NR_seccomp_*) are used solely in secure_computing() to identify mode 1 system calls. If compat system calls have different syscall numbers, asm/seccomp.h may override them.
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org --- include/asm-generic/seccomp.h | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 include/asm-generic/seccomp.h
diff --git a/include/asm-generic/seccomp.h b/include/asm-generic/seccomp.h new file mode 100644 index 0000000..5e97022 --- /dev/null +++ b/include/asm-generic/seccomp.h @@ -0,0 +1,28 @@ +/* + * include/asm-generic/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_GENERIC_SECCOMP_H +#define _ASM_GENERIC_SECCOMP_H + +#include <asm-generic/unistd.h> + +#if defined(CONFIG_COMPAT) && !defined(__NR_seccomp_read_32) +#define __NR_seccomp_read_32 __NR_read +#define __NR_seccomp_write_32 __NR_write +#define __NR_seccomp_exit_32 __NR_exit +#define __NR_seccomp_sigreturn_32 __NR_rt_sigreturn +#endif /* CONFIG_COMPAT && ! already defined */ + +#define __NR_seccomp_read __NR_read +#define __NR_seccomp_write __NR_write +#define __NR_seccomp_exit __NR_exit +#define __NR_seccomp_sigreturn __NR_rt_sigreturn + +#endif /* _ASM_GENERIC_SECCOMP_H */
secure_computing() should always be called first in syscall_trace(), and 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.
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org --- arch/arm64/Kconfig | 17 +++++++++++++++++ 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, 55 insertions(+) create mode 100644 arch/arm64/include/asm/seccomp.h
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig index a21455e..a0102f7 100644 --- a/arch/arm64/Kconfig +++ b/arch/arm64/Kconfig @@ -27,6 +27,7 @@ config ARM64 select HARDIRQS_SW_RESEND select HAVE_ARCH_AUDITSYSCALL select HAVE_ARCH_JUMP_LABEL + select HAVE_ARCH_SECCOMP_FILTER select HAVE_ARCH_TRACEHOOK select HAVE_DEBUG_BUGVERBOSE select HAVE_DEBUG_KMEMLEAK @@ -222,6 +223,22 @@ config HAVE_ARCH_TRANSPARENT_HUGEPAGE
source "mm/Kconfig"
+config SECCOMP + def_bool y + prompt "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. + + If unsure, say Y. Only embedded should say N here. + 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 4a09fdb..05f2db3 100644 --- a/arch/arm64/include/asm/unistd.h +++ b/arch/arm64/include/asm/unistd.h @@ -30,6 +30,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 96c2d03..7f3cbaf 100644 --- a/arch/arm64/kernel/entry.S +++ b/arch/arm64/kernel/entry.S @@ -651,6 +651,10 @@ __sys_trace: mov x1, sp mov w0, #0 // trace entry bl syscall_trace +#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 d4ce70e..f2a74bc 100644 --- a/arch/arm64/kernel/ptrace.c +++ b/arch/arm64/kernel/ptrace.c @@ -20,12 +20,14 @@ */
#include <linux/audit.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> @@ -1064,6 +1066,10 @@ asmlinkage int syscall_trace(int dir, struct pt_regs *regs) { unsigned long saved_reg;
+ if (!dir && secure_computing((int)regs->syscallno)) + /* seccomp failures shouldn't expose any additional code. */ + return -EPERM; + if (dir && test_thread_flag(TIF_SYSCALL_AUDIT)) audit_syscall_exit(regs);
On Tue, Feb 25, 2014 at 09:20:24AM +0000, AKASHI Takahiro wrote:
secure_computing() should always be called first in syscall_trace(), and 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.
[...]
diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c index d4ce70e..f2a74bc 100644 --- a/arch/arm64/kernel/ptrace.c +++ b/arch/arm64/kernel/ptrace.c @@ -20,12 +20,14 @@ */ #include <linux/audit.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> @@ -1064,6 +1066,10 @@ asmlinkage int syscall_trace(int dir, struct pt_regs *regs) { unsigned long saved_reg;
- if (!dir && secure_computing((int)regs->syscallno))
Why do you need this cast to (int)? Also, it's probably better to check for -1 explicitly here.
I'm slightly surprised that we do the secure computing check first. Doesn't this allow a debugger to change the syscall to something else after we've decided that it's ok?
Will
On 03/01/2014 02:20 AM, Will Deacon wrote:
On Tue, Feb 25, 2014 at 09:20:24AM +0000, AKASHI Takahiro wrote:
secure_computing() should always be called first in syscall_trace(), and 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.
[...]
diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c index d4ce70e..f2a74bc 100644 --- a/arch/arm64/kernel/ptrace.c +++ b/arch/arm64/kernel/ptrace.c @@ -20,12 +20,14 @@ */
#include <linux/audit.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> @@ -1064,6 +1066,10 @@ asmlinkage int syscall_trace(int dir, struct pt_regs *regs) { unsigned long saved_reg;
- if (!dir && secure_computing((int)regs->syscallno))
Why do you need this cast to (int)?
OK. I will remove it because gcc doesn't complain about it anyway.
Also, it's probably better to check for -1 explicitly here.
I wil fix it.
I'm slightly surprised that we do the secure computing check first. Doesn't this allow a debugger to change the syscall to something else after we've decided that it's ok?
To be honest, I just followed other architectures' implementation. Can you elaborate any use case that you have in your mind?
-Takahiro AKASHI
Will
On Thu, Mar 06, 2014 at 02:34:46AM +0000, AKASHI Takahiro wrote:
On 03/01/2014 02:20 AM, Will Deacon wrote:
On Tue, Feb 25, 2014 at 09:20:24AM +0000, AKASHI Takahiro wrote: I'm slightly surprised that we do the secure computing check first. Doesn't this allow a debugger to change the syscall to something else after we've decided that it's ok?
To be honest, I just followed other architectures' implementation. Can you elaborate any use case that you have in your mind?
My initial thought was that we should do the secure_computing check *after* the debugger has finished messing around with the registers. However, I suppose you'd have had to enable ptrace in your seccompd filter for that scenario to occur, so there's probably not an issue here after all.
Will
kernel/seccomp.c includes linux/compat.h and, indicrectly, asm/compat.h via asm/syscall.h. Due to the duplicated definitions of is_compat_task, compiling this file will fail in the case of !CONFIG_COMPAT. So this patch 1) removes is_compat_task() definition from asm/compat.h 2) replaces asm/compat.h to linux/compat.h in kernel/*.c
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org --- arch/arm64/include/asm/compat.h | 5 ----- arch/arm64/kernel/hw_breakpoint.c | 2 +- arch/arm64/kernel/process.c | 2 +- arch/arm64/kernel/ptrace.c | 2 +- arch/arm64/kernel/signal.c | 2 +- 5 files changed, 4 insertions(+), 9 deletions(-)
diff --git a/arch/arm64/include/asm/compat.h b/arch/arm64/include/asm/compat.h index fda2704..3b334f9 100644 --- a/arch/arm64/include/asm/compat.h +++ b/arch/arm64/include/asm/compat.h @@ -305,11 +305,6 @@ static inline int is_compat_thread(struct thread_info *thread)
#else /* !CONFIG_COMPAT */
-static inline int is_compat_task(void) -{ - return 0; -} - static inline int is_compat_thread(struct thread_info *thread) { return 0; diff --git a/arch/arm64/kernel/hw_breakpoint.c b/arch/arm64/kernel/hw_breakpoint.c index f17f581..a45e2db 100644 --- a/arch/arm64/kernel/hw_breakpoint.c +++ b/arch/arm64/kernel/hw_breakpoint.c @@ -20,6 +20,7 @@
#define pr_fmt(fmt) "hw-breakpoint: " fmt
+#include <linux/compat.h> #include <linux/cpu_pm.h> #include <linux/errno.h> #include <linux/hw_breakpoint.h> @@ -27,7 +28,6 @@ #include <linux/ptrace.h> #include <linux/smp.h>
-#include <asm/compat.h> #include <asm/current.h> #include <asm/debug-monitors.h> #include <asm/hw_breakpoint.h> diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c index 248a15d..c3805b6 100644 --- a/arch/arm64/kernel/process.c +++ b/arch/arm64/kernel/process.c @@ -20,6 +20,7 @@
#include <stdarg.h>
+#include <linux/compat.h> #include <linux/export.h> #include <linux/sched.h> #include <linux/kernel.h> @@ -44,7 +45,6 @@ #include <linux/personality.h> #include <linux/notifier.h>
-#include <asm/compat.h> #include <asm/cacheflush.h> #include <asm/fpsimd.h> #include <asm/mmu_context.h> diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c index f2a74bc..340d51f 100644 --- a/arch/arm64/kernel/ptrace.c +++ b/arch/arm64/kernel/ptrace.c @@ -20,6 +20,7 @@ */
#include <linux/audit.h> +#include <linux/compat.h> #include <linux/errno.h> #include <linux/kernel.h> #include <linux/sched.h> @@ -38,7 +39,6 @@ #include <linux/tracehook.h> #include <linux/elf.h>
-#include <asm/compat.h> #include <asm/debug-monitors.h> #include <asm/pgtable.h> #include <asm/syscall.h> diff --git a/arch/arm64/kernel/signal.c b/arch/arm64/kernel/signal.c index 890a591..4a09989 100644 --- a/arch/arm64/kernel/signal.c +++ b/arch/arm64/kernel/signal.c @@ -17,6 +17,7 @@ * along with this program. If not, see http://www.gnu.org/licenses/. */
+#include <linux/compat.h> #include <linux/errno.h> #include <linux/signal.h> #include <linux/personality.h> @@ -25,7 +26,6 @@ #include <linux/tracehook.h> #include <linux/ratelimit.h>
-#include <asm/compat.h> #include <asm/debug-monitors.h> #include <asm/elf.h> #include <asm/cacheflush.h>
On Tue, Feb 25, 2014 at 09:20:25AM +0000, AKASHI Takahiro wrote:
kernel/seccomp.c includes linux/compat.h and, indicrectly, asm/compat.h via asm/syscall.h. Due to the duplicated definitions of is_compat_task, compiling this file will fail in the case of !CONFIG_COMPAT. So this patch
- removes is_compat_task() definition from asm/compat.h
- replaces asm/compat.h to linux/compat.h in kernel/*.c
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org
You built this, right? If so,
Acked-by: Will Deacon will.deacon@arm.com
Will
(Please apply this patch after my ftrace patch and audit patch in order to avoid some conflict on arm64/Kconfig.)
This patch enables secure computing (system call filtering) on arm64. System calls can be allowed or denied by loaded bpf-style rules. Architecture specific part is to run secure_computing() on syscall entry and check the result. See [2/3]
Prerequisites are: * "arm64: make a single hook to syscall_trace() for all syscall features" patch * "arm64: split syscall_trace() into separate functions for enter/exit" patch * "arm64: Add audit support" patch
This code is tested on ARMv8 fast model using libseccomp v2.1.1 with modifications for arm64 and verified by its "live" tests, 20, 21 and 24.
Changes v2 -> v3: * removed unnecessary 'type cast' operations [2/3] * check for a return value (-1) of secure_computing() explicitly [2/3] * aligned with the patch, "arm64: split syscall_trace() into separate functions for enter/exit" [2/3] * changed default of CONFIG_SECCOMP to n [2/3]
Changes v1 -> v2: * added generic seccomp.h for arm64 to utilize it [1,2/3] * changed syscall_trace() to return more meaningful value (-EPERM) on seccomp failure case [2/3] * aligned with the change in "arm64: make a single hook to syscall_trace() for all syscall features" v2 [2/3] * removed is_compat_task() definition from compat.h [3/3]
AKASHI Takahiro (3): asm-generic: Add generic seccomp.h for secure computing mode 1 arm64: Add seccomp support arm64: is_compat_task is defined both in asm/compat.h and linux/compat.h
arch/arm64/Kconfig | 14 ++++++++++++++ arch/arm64/include/asm/compat.h | 5 ----- arch/arm64/include/asm/seccomp.h | 25 +++++++++++++++++++++++++ arch/arm64/include/asm/unistd.h | 3 +++ arch/arm64/kernel/entry.S | 4 ++++ arch/arm64/kernel/hw_breakpoint.c | 2 +- arch/arm64/kernel/process.c | 2 +- arch/arm64/kernel/ptrace.c | 8 +++++++- arch/arm64/kernel/signal.c | 2 +- include/asm-generic/seccomp.h | 28 ++++++++++++++++++++++++++++ 10 files changed, 84 insertions(+), 9 deletions(-) create mode 100644 arch/arm64/include/asm/seccomp.h create mode 100644 include/asm-generic/seccomp.h
Those values (__NR_seccomp_*) are used solely in secure_computing() to identify mode 1 system calls. If compat system calls have different syscall numbers, asm/seccomp.h may override them.
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org --- include/asm-generic/seccomp.h | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 include/asm-generic/seccomp.h
diff --git a/include/asm-generic/seccomp.h b/include/asm-generic/seccomp.h new file mode 100644 index 0000000..5e97022 --- /dev/null +++ b/include/asm-generic/seccomp.h @@ -0,0 +1,28 @@ +/* + * include/asm-generic/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_GENERIC_SECCOMP_H +#define _ASM_GENERIC_SECCOMP_H + +#include <asm-generic/unistd.h> + +#if defined(CONFIG_COMPAT) && !defined(__NR_seccomp_read_32) +#define __NR_seccomp_read_32 __NR_read +#define __NR_seccomp_write_32 __NR_write +#define __NR_seccomp_exit_32 __NR_exit +#define __NR_seccomp_sigreturn_32 __NR_rt_sigreturn +#endif /* CONFIG_COMPAT && ! already defined */ + +#define __NR_seccomp_read __NR_read +#define __NR_seccomp_write __NR_write +#define __NR_seccomp_exit __NR_exit +#define __NR_seccomp_sigreturn __NR_rt_sigreturn + +#endif /* _ASM_GENERIC_SECCOMP_H */
On Thu, Mar 13, 2014 at 10:17:01AM +0000, AKASHI Takahiro wrote:
Those values (__NR_seccomp_*) are used solely in secure_computing() to identify mode 1 system calls. If compat system calls have different syscall numbers, asm/seccomp.h may override them.
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org
include/asm-generic/seccomp.h | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 include/asm-generic/seccomp.h
I think you need an Ack from Arnd on this patch. The other patches in this series look ok but they depend on the ftrace patches, so we'll have to sort those out first.
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
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig index 7ca6799..21b33f4 100644 --- a/arch/arm64/Kconfig +++ b/arch/arm64/Kconfig @@ -27,6 +27,7 @@ config ARM64 select HARDIRQS_SW_RESEND select HAVE_ARCH_AUDITSYSCALL select HAVE_ARCH_JUMP_LABEL + select HAVE_ARCH_SECCOMP_FILTER select HAVE_ARCH_TRACEHOOK select HAVE_C_RECORDMCOUNT select HAVE_DEBUG_BUGVERBOSE @@ -229,6 +230,19 @@ config HAVE_ARCH_TRANSPARENT_HUGEPAGE
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 04f5d22..28bf882 100644 --- a/arch/arm64/include/asm/unistd.h +++ b/arch/arm64/include/asm/unistd.h @@ -30,6 +30,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 f06ee35..6ef266a 100644 --- a/arch/arm64/kernel/entry.S +++ b/arch/arm64/kernel/entry.S @@ -650,6 +650,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 d10c637..be2bb4c 100644 --- a/arch/arm64/kernel/ptrace.c +++ b/arch/arm64/kernel/ptrace.c @@ -20,12 +20,14 @@ */
#include <linux/audit.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> @@ -1067,6 +1069,10 @@ asmlinkage int syscall_trace_enter(struct pt_regs *regs) { unsigned long saved_reg;
+ if (secure_computing(regs->syscallno) == -1) + /* seccomp failures shouldn't expose any additional code. */ + return -EPERM; + if (test_thread_flag(TIF_SYSCALL_TRACE)) { /* * A scrach register (ip(r12) on AArch32, x7 on AArch64) is
kernel/seccomp.c includes linux/compat.h and, indicrectly, asm/compat.h via asm/syscall.h. Due to the duplicated definitions of is_compat_task, compiling this file will fail in the case of !CONFIG_COMPAT. So this patch 1) removes is_compat_task() definition from asm/compat.h 2) replaces asm/compat.h to linux/compat.h in kernel/*.c
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org --- arch/arm64/include/asm/compat.h | 5 ----- arch/arm64/kernel/hw_breakpoint.c | 2 +- arch/arm64/kernel/process.c | 2 +- arch/arm64/kernel/ptrace.c | 2 +- arch/arm64/kernel/signal.c | 2 +- 5 files changed, 4 insertions(+), 9 deletions(-)
diff --git a/arch/arm64/include/asm/compat.h b/arch/arm64/include/asm/compat.h index fda2704..3b334f9 100644 --- a/arch/arm64/include/asm/compat.h +++ b/arch/arm64/include/asm/compat.h @@ -305,11 +305,6 @@ static inline int is_compat_thread(struct thread_info *thread)
#else /* !CONFIG_COMPAT */
-static inline int is_compat_task(void) -{ - return 0; -} - static inline int is_compat_thread(struct thread_info *thread) { return 0; diff --git a/arch/arm64/kernel/hw_breakpoint.c b/arch/arm64/kernel/hw_breakpoint.c index f17f581..a45e2db 100644 --- a/arch/arm64/kernel/hw_breakpoint.c +++ b/arch/arm64/kernel/hw_breakpoint.c @@ -20,6 +20,7 @@
#define pr_fmt(fmt) "hw-breakpoint: " fmt
+#include <linux/compat.h> #include <linux/cpu_pm.h> #include <linux/errno.h> #include <linux/hw_breakpoint.h> @@ -27,7 +28,6 @@ #include <linux/ptrace.h> #include <linux/smp.h>
-#include <asm/compat.h> #include <asm/current.h> #include <asm/debug-monitors.h> #include <asm/hw_breakpoint.h> diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c index 1c0a9be..02eefc6 100644 --- a/arch/arm64/kernel/process.c +++ b/arch/arm64/kernel/process.c @@ -20,6 +20,7 @@
#include <stdarg.h>
+#include <linux/compat.h> #include <linux/export.h> #include <linux/sched.h> #include <linux/kernel.h> @@ -44,7 +45,6 @@ #include <linux/personality.h> #include <linux/notifier.h>
-#include <asm/compat.h> #include <asm/cacheflush.h> #include <asm/fpsimd.h> #include <asm/mmu_context.h> diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c index be2bb4c..674bae2 100644 --- a/arch/arm64/kernel/ptrace.c +++ b/arch/arm64/kernel/ptrace.c @@ -20,6 +20,7 @@ */
#include <linux/audit.h> +#include <linux/compat.h> #include <linux/errno.h> #include <linux/kernel.h> #include <linux/sched.h> @@ -38,7 +39,6 @@ #include <linux/tracehook.h> #include <linux/elf.h>
-#include <asm/compat.h> #include <asm/debug-monitors.h> #include <asm/pgtable.h> #include <asm/syscall.h> diff --git a/arch/arm64/kernel/signal.c b/arch/arm64/kernel/signal.c index 890a591..4a09989 100644 --- a/arch/arm64/kernel/signal.c +++ b/arch/arm64/kernel/signal.c @@ -17,6 +17,7 @@ * along with this program. If not, see http://www.gnu.org/licenses/. */
+#include <linux/compat.h> #include <linux/errno.h> #include <linux/signal.h> #include <linux/personality.h> @@ -25,7 +26,6 @@ #include <linux/tracehook.h> #include <linux/ratelimit.h>
-#include <asm/compat.h> #include <asm/debug-monitors.h> #include <asm/elf.h> #include <asm/cacheflush.h>
(Please apply this patch after my ftrace patch and audit patch in order to avoid some conflict on arm64/Kconfig.)
This patch enables secure computing (system call filtering) on arm64. System calls can be allowed or denied by loaded bpf-style rules. Architecture specific part is to run secure_computing() on syscall entry and check the result. See [2/2]
Prerequisites are: * "arm64: make a single hook to syscall_trace() for all syscall features" patch * "arm64: split syscall_trace() into separate functions for enter/exit" patch * "arm64: Add audit support" patch * "arm64: is_compat_task is defined both in asm/compat.h and linux/compat.h" patch
This code is tested on ARMv8 fast model using libseccomp v2.1.1 with modifications for arm64 and verified by its "live" tests, 20, 21 and 24.
Changes v3 -> v4: * removed the following patch and moved it to "arm64: prerequisites for audit and ftrace" patchset since it is required for audit and ftrace in case of !COMPAT, too. "arm64: is_compat_task is defined both in asm/compat.h and linux/compat.h"
Changes v2 -> v3: * removed unnecessary 'type cast' operations [2/3] * check for a return value (-1) of secure_computing() explicitly [2/3] * aligned with the patch, "arm64: split syscall_trace() into separate functions for enter/exit" [2/3] * changed default of CONFIG_SECCOMP to n [2/3]
Changes v1 -> v2: * added generic seccomp.h for arm64 to utilize it [1,2/3] * changed syscall_trace() to return more meaningful value (-EPERM) on seccomp failure case [2/3] * aligned with the change in "arm64: make a single hook to syscall_trace() for all syscall features" v2 [2/3] * removed is_compat_task() definition from compat.h [3/3]
AKASHI Takahiro (2): asm-generic: Add generic seccomp.h for secure computing mode 1 arm64: Add seccomp support
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 ++++++ include/asm-generic/seccomp.h | 28 ++++++++++++++++++++++++++++ 6 files changed, 80 insertions(+) create mode 100644 arch/arm64/include/asm/seccomp.h create mode 100644 include/asm-generic/seccomp.h
Those values (__NR_seccomp_*) are used solely in secure_computing() to identify mode 1 system calls. If compat system calls have different syscall numbers, asm/seccomp.h may override them.
Acked-by: Arnd Bergmann arnd@arndb.de Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org --- include/asm-generic/seccomp.h | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 include/asm-generic/seccomp.h
diff --git a/include/asm-generic/seccomp.h b/include/asm-generic/seccomp.h new file mode 100644 index 0000000..5e97022 --- /dev/null +++ b/include/asm-generic/seccomp.h @@ -0,0 +1,28 @@ +/* + * include/asm-generic/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_GENERIC_SECCOMP_H +#define _ASM_GENERIC_SECCOMP_H + +#include <asm-generic/unistd.h> + +#if defined(CONFIG_COMPAT) && !defined(__NR_seccomp_read_32) +#define __NR_seccomp_read_32 __NR_read +#define __NR_seccomp_write_32 __NR_write +#define __NR_seccomp_exit_32 __NR_exit +#define __NR_seccomp_sigreturn_32 __NR_rt_sigreturn +#endif /* CONFIG_COMPAT && ! already defined */ + +#define __NR_seccomp_read __NR_read +#define __NR_seccomp_write __NR_write +#define __NR_seccomp_exit __NR_exit +#define __NR_seccomp_sigreturn __NR_rt_sigreturn + +#endif /* _ASM_GENERIC_SECCOMP_H */
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
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig index 7c1f8c7..d5167d6 100644 --- a/arch/arm64/Kconfig +++ b/arch/arm64/Kconfig @@ -28,6 +28,7 @@ config ARM64 select HARDIRQS_SW_RESEND select HAVE_ARCH_AUDITSYSCALL select HAVE_ARCH_JUMP_LABEL + select HAVE_ARCH_SECCOMP_FILTER select HAVE_ARCH_TRACEHOOK select HAVE_C_RECORDMCOUNT select HAVE_DEBUG_BUGVERBOSE @@ -230,6 +231,19 @@ config HAVE_ARCH_TRANSPARENT_HUGEPAGE
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 04f5d22..28bf882 100644 --- a/arch/arm64/include/asm/unistd.h +++ b/arch/arm64/include/asm/unistd.h @@ -30,6 +30,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 f06ee35..6ef266a 100644 --- a/arch/arm64/kernel/entry.S +++ b/arch/arm64/kernel/entry.S @@ -650,6 +650,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 f9e1339..bb89fa3 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> @@ -1093,6 +1095,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);
What is the current status of this patch series? Is it on track for 3.17?
On Sat, 2014-03-15 at 14:50 +0900, AKASHI Takahiro wrote:
(Please apply this patch after my ftrace patch and audit patch in order to avoid some conflict on arm64/Kconfig.)
This patch enables secure computing (system call filtering) on arm64. System calls can be allowed or denied by loaded bpf-style rules. Architecture specific part is to run secure_computing() on syscall entry and check the result. See [2/2]
Prerequisites are:
- "arm64: make a single hook to syscall_trace() for all syscall features" patch
- "arm64: split syscall_trace() into separate functions for enter/exit" patch
- "arm64: Add audit support" patch
- "arm64: is_compat_task is defined both in asm/compat.h and linux/compat.h" patch
This code is tested on ARMv8 fast model using libseccomp v2.1.1 with modifications for arm64 and verified by its "live" tests, 20, 21 and 24.
Changes v3 -> v4:
- removed the following patch and moved it to "arm64: prerequisites for audit and ftrace" patchset since it is required for audit and ftrace in case of !COMPAT, too. "arm64: is_compat_task is defined both in asm/compat.h and linux/compat.h"
Changes v2 -> v3:
- removed unnecessary 'type cast' operations [2/3]
- check for a return value (-1) of secure_computing() explicitly [2/3]
- aligned with the patch, "arm64: split syscall_trace() into separate functions for enter/exit" [2/3]
- changed default of CONFIG_SECCOMP to n [2/3]
Changes v1 -> v2:
- added generic seccomp.h for arm64 to utilize it [1,2/3]
- changed syscall_trace() to return more meaningful value (-EPERM) on seccomp failure case [2/3]
- aligned with the change in "arm64: make a single hook to syscall_trace() for all syscall features" v2 [2/3]
- removed is_compat_task() definition from compat.h [3/3]
AKASHI Takahiro (2): asm-generic: Add generic seccomp.h for secure computing mode 1 arm64: Add seccomp support
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 ++++++ include/asm-generic/seccomp.h | 28 ++++++++++++++++++++++++++++ 6 files changed, 80 insertions(+) create mode 100644 arch/arm64/include/asm/seccomp.h create mode 100644 include/asm-generic/seccomp.h
On 06/25/2014 11:53 PM, Mark Salter wrote:
What is the current status of this patch series? Is it on track for 3.17?
I assume not as I saw no comments on this so far. But I will re-post a new version soon or later due to recent changes on seccomp.
-Takahiro AKASHI
On Sat, 2014-03-15 at 14:50 +0900, AKASHI Takahiro wrote:
(Please apply this patch after my ftrace patch and audit patch in order to avoid some conflict on arm64/Kconfig.)
This patch enables secure computing (system call filtering) on arm64. System calls can be allowed or denied by loaded bpf-style rules. Architecture specific part is to run secure_computing() on syscall entry and check the result. See [2/2]
Prerequisites are:
- "arm64: make a single hook to syscall_trace() for all syscall features" patch
- "arm64: split syscall_trace() into separate functions for enter/exit" patch
- "arm64: Add audit support" patch
- "arm64: is_compat_task is defined both in asm/compat.h and linux/compat.h" patch
This code is tested on ARMv8 fast model using libseccomp v2.1.1 with modifications for arm64 and verified by its "live" tests, 20, 21 and 24.
Changes v3 -> v4:
- removed the following patch and moved it to "arm64: prerequisites for audit and ftrace" patchset since it is required for audit and ftrace in case of !COMPAT, too. "arm64: is_compat_task is defined both in asm/compat.h and linux/compat.h"
Changes v2 -> v3:
- removed unnecessary 'type cast' operations [2/3]
- check for a return value (-1) of secure_computing() explicitly [2/3]
- aligned with the patch, "arm64: split syscall_trace() into separate functions for enter/exit" [2/3]
- changed default of CONFIG_SECCOMP to n [2/3]
Changes v1 -> v2:
- added generic seccomp.h for arm64 to utilize it [1,2/3]
- changed syscall_trace() to return more meaningful value (-EPERM) on seccomp failure case [2/3]
- aligned with the change in "arm64: make a single hook to syscall_trace() for all syscall features" v2 [2/3]
- removed is_compat_task() definition from compat.h [3/3]
AKASHI Takahiro (2): asm-generic: Add generic seccomp.h for secure computing mode 1 arm64: Add seccomp support
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 ++++++ include/asm-generic/seccomp.h | 28 ++++++++++++++++++++++++++++ 6 files changed, 80 insertions(+) create mode 100644 arch/arm64/include/asm/seccomp.h create mode 100644 include/asm-generic/seccomp.h
linaro-kernel@lists.linaro.org
-
AKASHI Takahiro -
Arnd Bergmann -
Catalin Marinas -
Mark Salter -
Takahiro Akashi
-
Will Deacon