[PATCH v5 0/4] arch/arm: support seccomp

[PATCH v5 0/4] arch/arm: support seccomp

[Kees Cook]

This adds support for seccomp BPF to ARM. When built with the seccomp
improvement patch waiting in linux-next ("seccomp: Make syscall skipping
and nr changes more consistent"), this passes the seccomp regression
test suite: https://github.com/redpig/seccomp

Thanks,

-Kees

---
v5:
 - clean up seccomp failure path, as requested by Will Deacon.
v4:
 - fixed syscall_get_arch, thanks to Will Deacon.
v3:
 - updates suggested by Russell King:
   - reduced scope of expansion
   - leveraged TIF_SYSCALL_WORK bit mask
   - fixed syscall==-1 short-circuit logic
v2:
 - expanded ptrace_syscall_trace() into both callers and do
   secure_computing() hookup there, as requested by Al Viro.

[PATCH 1/4] arch/arm: add syscall_get_arch

[Kees Cook]

From: Will Drewry <wad@chromium.org>

Provide an ARM implementation of syscall_get_arch. This is a pre-requisite
for CONFIG_HAVE_ARCH_SECCOMP_FILTER.

Signed-off-by: Will Drewry <wad@chromium.org>
Signed-off-by: Kees Cook <keescook@chromium.org>
---
 arch/arm/include/asm/syscall.h |    9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/arch/arm/include/asm/syscall.h b/arch/arm/include/asm/syscall.h
index 9fdded6..f1d96d4 100644
--- a/arch/arm/include/asm/syscall.h
+++ b/arch/arm/include/asm/syscall.h
@@ -7,6 +7,8 @@
 #ifndef _ASM_ARM_SYSCALL_H
 #define _ASM_ARM_SYSCALL_H
 
+#include <linux/audit.h> /* for AUDIT_ARCH_* */
+#include <linux/elf.h> /* for ELF_EM */
 #include <linux/err.h>
 #include <linux/sched.h>
 
@@ -95,4 +97,11 @@ static inline void syscall_set_arguments(struct task_struct *task,
 	memcpy(&regs->ARM_r0 + i, args, n * sizeof(args[0]));
 }
 
+static inline int syscall_get_arch(struct task_struct *task,
+				   struct pt_regs *regs)
+{
+	/* ARM tasks don't change audit architectures on the fly. */
+	return AUDIT_ARCH_ARM;
+}
+
 #endif /* _ASM_ARM_SYSCALL_H */
-- 
1.7.9.5

[PATCH 2/4] arch/arm: move secure_computing into trace

[Kees Cook]

There is very little difference in the TIF_SECCOMP and TIF_SYSCALL_WORK
path in entry-common.S, so merge TIF_SECCOMP into TIF_SYSCALL_WORK and
move seccomp into the syscall_trace_enter() handler.

Expanded some of the tracehook logic into the callers to make this code
more readable. Since tracehook needs to do register changing, this portion
is best left in its own function instead of copy/pasting into the callers.

Additionally, the return value for secure_computing() is now checked
and a -1 value will result in the system call being skipped.

Signed-off-by: Kees Cook <keescook@chromium.org>
---
 arch/arm/include/asm/thread_info.h |    7 ++++---
 arch/arm/kernel/entry-common.S     |   10 ----------
 arch/arm/kernel/ptrace.c           |   29 ++++++++++++++++++++---------
 3 files changed, 24 insertions(+), 22 deletions(-)

diff --git a/arch/arm/include/asm/thread_info.h b/arch/arm/include/asm/thread_info.h
index 8477b4c..cddda1f 100644
--- a/arch/arm/include/asm/thread_info.h
+++ b/arch/arm/include/asm/thread_info.h
@@ -151,10 +151,10 @@ extern int vfp_restore_user_hwstate(struct user_vfp __user *,
 #define TIF_SYSCALL_TRACE	8
 #define TIF_SYSCALL_AUDIT	9
 #define TIF_SYSCALL_TRACEPOINT	10
+#define TIF_SECCOMP		11	/* seccomp syscall filtering active */
 #define TIF_USING_IWMMXT	17
 #define TIF_MEMDIE		18	/* is terminating due to OOM killer */
 #define TIF_RESTORE_SIGMASK	20
-#define TIF_SECCOMP		21
 #define TIF_SWITCH_MM		22	/* deferred switch_mm */
 
 #define _TIF_SIGPENDING		(1 << TIF_SIGPENDING)
@@ -163,11 +163,12 @@ extern int vfp_restore_user_hwstate(struct user_vfp __user *,
 #define _TIF_SYSCALL_TRACE	(1 << TIF_SYSCALL_TRACE)
 #define _TIF_SYSCALL_AUDIT	(1 << TIF_SYSCALL_AUDIT)
 #define _TIF_SYSCALL_TRACEPOINT	(1 << TIF_SYSCALL_TRACEPOINT)
-#define _TIF_USING_IWMMXT	(1 << TIF_USING_IWMMXT)
 #define _TIF_SECCOMP		(1 << TIF_SECCOMP)
+#define _TIF_USING_IWMMXT	(1 << TIF_USING_IWMMXT)
 
 /* Checks for any syscall work in entry-common.S */
-#define _TIF_SYSCALL_WORK (_TIF_SYSCALL_TRACE | _TIF_SYSCALL_AUDIT | _TIF_SYSCALL_TRACEPOINT)
+#define _TIF_SYSCALL_WORK (_TIF_SYSCALL_TRACE | _TIF_SYSCALL_AUDIT | \
+			   _TIF_SYSCALL_TRACEPOINT | _TIF_SECCOMP)
 
 /*
  * Change these and you break ASM code in entry-common.S
diff --git a/arch/arm/kernel/entry-common.S b/arch/arm/kernel/entry-common.S
index 3471175..8355d4b 100644
--- a/arch/arm/kernel/entry-common.S
+++ b/arch/arm/kernel/entry-common.S
@@ -417,16 +417,6 @@ local_restart:
 	ldr	r10, [tsk, #TI_FLAGS]		@ check for syscall tracing
 	stmdb	sp!, {r4, r5}			@ push fifth and sixth args
 
-#ifdef CONFIG_SECCOMP
-	tst	r10, #_TIF_SECCOMP
-	beq	1f
-	mov	r0, scno
-	bl	__secure_computing	
-	add	r0, sp, #S_R0 + S_OFF		@ pointer to regs
-	ldmia	r0, {r0 - r3}			@ have to reload r0 - r3
-1:
-#endif
-
 	tst	r10, #_TIF_SYSCALL_WORK		@ are we tracing syscalls?
 	bne	__sys_trace
 
diff --git a/arch/arm/kernel/ptrace.c b/arch/arm/kernel/ptrace.c
index 739db3a..518536d 100644
--- a/arch/arm/kernel/ptrace.c
+++ b/arch/arm/kernel/ptrace.c
@@ -916,16 +916,11 @@ enum ptrace_syscall_dir {
 	PTRACE_SYSCALL_EXIT,
 };
 
-static int ptrace_syscall_trace(struct pt_regs *regs, int scno,
-				enum ptrace_syscall_dir dir)
+static int tracehook_report_syscall(struct pt_regs *regs,
+				    enum ptrace_syscall_dir dir)
 {
 	unsigned long ip;
 
-	current_thread_info()->syscall = scno;
-
-	if (!test_thread_flag(TIF_SYSCALL_TRACE))
-		return scno;
-
 	/*
 	 * IP is used to denote syscall entry/exit:
 	 * IP = 0 -> entry, =1 -> exit
@@ -944,19 +939,35 @@ static int ptrace_syscall_trace(struct pt_regs *regs, int scno,
 
 asmlinkage int syscall_trace_enter(struct pt_regs *regs, int scno)
 {
-	scno = ptrace_syscall_trace(regs, scno, PTRACE_SYSCALL_ENTER);
+	current_thread_info()->syscall = scno;
+
+	/* Do the secure computing check first; failures should be fast. */
+	if (secure_computing(scno) == -1)
+		return -1;
+
+	if (test_thread_flag(TIF_SYSCALL_TRACE))
+		scno = tracehook_report_syscall(regs, PTRACE_SYSCALL_ENTER);
+
 	if (test_thread_flag(TIF_SYSCALL_TRACEPOINT))
 		trace_sys_enter(regs, scno);
+
 	audit_syscall_entry(AUDIT_ARCH_ARM, scno, regs->ARM_r0, regs->ARM_r1,
 			    regs->ARM_r2, regs->ARM_r3);
+
 	return scno;
 }
 
 asmlinkage int syscall_trace_exit(struct pt_regs *regs, int scno)
 {
-	scno = ptrace_syscall_trace(regs, scno, PTRACE_SYSCALL_EXIT);
+	current_thread_info()->syscall = scno;
+
+	if (test_thread_flag(TIF_SYSCALL_TRACE))
+		scno = tracehook_report_syscall(regs, PTRACE_SYSCALL_EXIT);
+
 	if (test_thread_flag(TIF_SYSCALL_TRACEPOINT))
 		trace_sys_exit(regs, scno);
+
 	audit_syscall_exit(regs);
+
 	return scno;
 }
-- 
1.7.9.5

[PATCH 3/4] arch/arm: allow a scno of -1 to not cause a SIGILL

[Kees Cook]

On tracehook-friendly platforms, a system call number of -1 falls
through without running much code or taking much action.

ARM is different. This adds a short-circuit check in the trace path to
avoid any additional work, as suggested by Russell King, to make sure
that ARM behaves the same way as other platforms.

Signed-off-by: Kees Cook <keescook@chromium.org>
---
 arch/arm/kernel/entry-common.S |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/arch/arm/kernel/entry-common.S b/arch/arm/kernel/entry-common.S
index 8355d4b..0bef977 100644
--- a/arch/arm/kernel/entry-common.S
+++ b/arch/arm/kernel/entry-common.S
@@ -448,7 +448,10 @@ __sys_trace:
 	ldmccia	r1, {r0 - r6}			@ have to reload r0 - r6
 	stmccia	sp, {r4, r5}			@ and update the stack args
 	ldrcc	pc, [tbl, scno, lsl #2]		@ call sys_* routine
-	b	2b
+	cmp	scno, #-1			@ skip the syscall?
+	bne	2b
+	add	sp, sp, #S_OFF			@ restore stack
+	b	ret_slow_syscall
 
 __sys_trace_return:
 	str	r0, [sp, #S_R0 + S_OFF]!	@ save returned r0
-- 
1.7.9.5

[PATCH 4/4] arch/arm: select HAVE_ARCH_SECCOMP_FILTER

[Kees Cook]

From: Will Drewry <wad@chromium.org>

Reflect architectural support for seccomp filter.

Signed-off-by: Will Drewry <wad@chromium.org>
Signed-off-by: Kees Cook <keescook@chromium.org>
---
 arch/arm/Kconfig |    1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index ade7e92..0e8d490 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -21,6 +21,7 @@ config ARM
 	select HAVE_AOUT
 	select HAVE_ARCH_JUMP_LABEL if !XIP_KERNEL
 	select HAVE_ARCH_KGDB
+	select HAVE_ARCH_SECCOMP_FILTER
 	select HAVE_ARCH_TRACEHOOK
 	select HAVE_BPF_JIT
 	select HAVE_C_RECORDMCOUNT
-- 
1.7.9.5

[PATCH v5 0/4] arch/arm: support seccomp

[Andy Lutomirski]

On 11/10/2012 02:44 PM, Kees Cook wrote:
> This adds support for seccomp BPF to ARM. When built with the seccomp
> improvement patch waiting in linux-next ("seccomp: Make syscall skipping
> and nr changes more consistent"), this passes the seccomp regression
> test suite: https://github.com/redpig/seccomp

I don't know if this matters much on ARM, but should something like
https://git.kernel.org/?p=linux/kernel/git/luto/linux.git;a=commit;h=dee1ae5a05149f30baeccb4b8f68f22525ba9ca2
be added?

--Andy

> 
> Thanks,
> 
> -Kees
> 
> ---
> v5:
>  - clean up seccomp failure path, as requested by Will Deacon.
> v4:
>  - fixed syscall_get_arch, thanks to Will Deacon.
> v3:
>  - updates suggested by Russell King:
>    - reduced scope of expansion
>    - leveraged TIF_SYSCALL_WORK bit mask
>    - fixed syscall==-1 short-circuit logic
> v2:
>  - expanded ptrace_syscall_trace() into both callers and do
>    secure_computing() hookup there, as requested by Al Viro.
> 

[PATCH v5 0/4] arch/arm: support seccomp

[Kees Cook]

On Tue, Nov 13, 2012 at 11:23 AM, Andy Lutomirski <luto@amacapital.net> wrote:
> On 11/10/2012 02:44 PM, Kees Cook wrote:
>> This adds support for seccomp BPF to ARM. When built with the seccomp
>> improvement patch waiting in linux-next ("seccomp: Make syscall skipping
>> and nr changes more consistent"), this passes the seccomp regression
>> test suite: https://github.com/redpig/seccomp
>
> I don't know if this matters much on ARM, but should something like
> https://git.kernel.org/?p=linux/kernel/git/luto/linux.git;a=commit;h=dee1ae5a05149f30baeccb4b8f68f22525ba9ca2
> be added?

Not sure -- I'll leave that up to other folks.

-Kees

-- 
Kees Cook
Chrome OS Security

[PATCH v5 0/4] arch/arm: support seccomp

[Kees Cook]

Hi, any more thoughts on this series? I'd really like to get it into
-next. It's been running happily for a while now in the Chrome OS ARM
devices.

Thanks,

-Kees

On Sat, Nov 10, 2012 at 2:44 PM, Kees Cook <keescook@chromium.org> wrote:
> This adds support for seccomp BPF to ARM. When built with the seccomp
> improvement patch waiting in linux-next ("seccomp: Make syscall skipping
> and nr changes more consistent"), this passes the seccomp regression
> test suite: https://github.com/redpig/seccomp
>
> Thanks,
>
> -Kees
>
> ---
> v5:
>  - clean up seccomp failure path, as requested by Will Deacon.
> v4:
>  - fixed syscall_get_arch, thanks to Will Deacon.
> v3:
>  - updates suggested by Russell King:
>    - reduced scope of expansion
>    - leveraged TIF_SYSCALL_WORK bit mask
>    - fixed syscall==-1 short-circuit logic
> v2:
>  - expanded ptrace_syscall_trace() into both callers and do
>    secure_computing() hookup there, as requested by Al Viro.

-- 
Kees Cook
Chrome OS Security

[PATCH v5 0/4] arch/arm: support seccomp

[Will Deacon]

On Wed, Nov 14, 2012 at 07:07:13PM +0000, Kees Cook wrote:
> Hi, any more thoughts on this series? I'd really like to get it into
> -next. It's been running happily for a while now in the Chrome OS ARM
> devices.

I'm not familiar with seccomp in general, but the changes look ok now from
an ARM point-of-view:

  Reviewed-by: Will Deacon <will.deacon@arm.com>

These should probably go via Russell (who will put them into -next), so
please send them to the patch system.

Will

[PATCH v5 0/4] arch/arm: support seccomp

[Kees Cook]

On Thu, Nov 15, 2012 at 5:25 AM, Will Deacon <will.deacon@arm.com> wrote:
> On Wed, Nov 14, 2012 at 07:07:13PM +0000, Kees Cook wrote:
>> Hi, any more thoughts on this series? I'd really like to get it into
>> -next. It's been running happily for a while now in the Chrome OS ARM
>> devices.
>
> I'm not familiar with seccomp in general, but the changes look ok now from
> an ARM point-of-view:
>
>   Reviewed-by: Will Deacon <will.deacon@arm.com>
>
> These should probably go via Russell (who will put them into -next), so
> please send them to the patch system.

Thanks. Is there a sensible way to send an ordered patch series to the
patch system? The Help doc says:
   Please omit [PATCH] and other "useful" prefixes.
(and it wants the "KernelVersion: " in there too)

Is there some way to indicate 1/4, 2/4, etc?

-Kees

-- 
Kees Cook
Chrome OS Security

[PATCH v5 0/4] arch/arm: support seccomp

[Will Deacon]

On Thu, Nov 15, 2012 at 08:23:48PM +0000, Kees Cook wrote:
> On Thu, Nov 15, 2012 at 5:25 AM, Will Deacon <will.deacon@arm.com> wrote:
> > On Wed, Nov 14, 2012 at 07:07:13PM +0000, Kees Cook wrote:
> >> Hi, any more thoughts on this series? I'd really like to get it into
> >> -next. It's been running happily for a while now in the Chrome OS ARM
> >> devices.
> >
> > I'm not familiar with seccomp in general, but the changes look ok now from
> > an ARM point-of-view:
> >
> >   Reviewed-by: Will Deacon <will.deacon@arm.com>
> >
> > These should probably go via Russell (who will put them into -next), so
> > please send them to the patch system.
> 
> Thanks. Is there a sensible way to send an ordered patch series to the
> patch system? The Help doc says:
>    Please omit [PATCH] and other "useful" prefixes.
> (and it wants the "KernelVersion: " in there too)
> 
> Is there some way to indicate 1/4, 2/4, etc?

If you're using git send-email, you don't need to do anything other than
add a new tag below your S-o-B to indicate the kernel version, e.g:

  KernelVersion: 3.7-rc5

then send the patches (1 by 1 so they don't get re-ordered) to
patches at arm.linux.org.uk

You should receive a response indicating whether they were correctly
formatted or not.

Will

[PATCH v5 0/4] arch/arm: support seccomp

[Will Drewry]

On Thu, Nov 15, 2012 at 7:25 AM, Will Deacon <will.deacon@arm.com> wrote:
> On Wed, Nov 14, 2012 at 07:07:13PM +0000, Kees Cook wrote:
>> Hi, any more thoughts on this series? I'd really like to get it into
>> -next. It's been running happily for a while now in the Chrome OS ARM
>> devices.
>
> I'm not familiar with seccomp in general, but the changes look ok now from
> an ARM point-of-view:
>
>   Reviewed-by: Will Deacon <will.deacon@arm.com>
>
> These should probably go via Russell (who will put them into -next), so
> please send them to the patch system.
>

For the seccomp side,

Acked-by: Will Drewry <wad@chromium.org>

for the whole series.  Thanks!