| Message ID | 1391767892-5395-2-git-send-email-takahiro.akashi@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 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
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];
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