All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/4] arch/arm: support seccomp
@ 2012-11-01 19:46 Kees Cook
  2012-11-01 19:46 ` [PATCH 1/4] arch/arm: add syscall_get_arch Kees Cook
                   ` (3 more replies)
  0 siblings, 4 replies; 15+ messages in thread
From: Kees Cook @ 2012-11-01 19:46 UTC (permalink / raw)
  To: linux-kernel
  Cc: Russell King, Will Deacon, Will Drewry, Geremy Condra, Al Viro,
	Catalin Marinas

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

---
v2:
 - expanded ptrace_syscall_trace() into both callers and do
   secure_computing() hookup there, as requested by Al Viro.


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 1/4] arch/arm: add syscall_get_arch
  2012-11-01 19:46 [PATCH v2 0/4] arch/arm: support seccomp Kees Cook
@ 2012-11-01 19:46 ` Kees Cook
  2012-11-01 19:46 ` [PATCH 2/4] arch/arm: move secure_computing into trace Kees Cook
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 15+ messages in thread
From: Kees Cook @ 2012-11-01 19:46 UTC (permalink / raw)
  To: linux-kernel
  Cc: Russell King, Will Deacon, Will Drewry, Geremy Condra, Al Viro,
	Catalin Marinas, 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 |   13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/arch/arm/include/asm/syscall.h b/arch/arm/include/asm/syscall.h
index 9fdded6..803f433 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,15 @@ 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. */
+#ifdef __ARMEB__
+	return AUDIT_ARCH_ARMEB;
+#else
+	return AUDIT_ARCH_ARM;
+#endif
+}
+
 #endif /* _ASM_ARM_SYSCALL_H */
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 2/4] arch/arm: move secure_computing into trace
  2012-11-01 19:46 [PATCH v2 0/4] arch/arm: support seccomp Kees Cook
  2012-11-01 19:46 ` [PATCH 1/4] arch/arm: add syscall_get_arch Kees Cook
@ 2012-11-01 19:46 ` Kees Cook
  2012-11-01 20:33   ` Russell King - ARM Linux
  2012-11-01 19:46 ` [PATCH 3/4] arch/arm: allow a scno of -1 to not cause a SIGILL Kees Cook
  2012-11-01 19:46 ` [PATCH 4/4] arch/arm: select HAVE_ARCH_SECCOMP_FILTER Kees Cook
  3 siblings, 1 reply; 15+ messages in thread
From: Kees Cook @ 2012-11-01 19:46 UTC (permalink / raw)
  To: linux-kernel
  Cc: Russell King, Will Deacon, Will Drewry, Geremy Condra, Al Viro,
	Catalin Marinas, Kees Cook

There is very little difference in the TIF_SECCOMP and TIF_SYSCALL_TRACE
path in entry-common.S. In order to add support for
CONFIG_HAVE_ARCH_SECCOMP_FILTER without mangling the assembly too badly,
seccomp was moved into the syscall_trace_enter() handler.

Expanded ptrace_syscall_trace() into both callers to make code more
readable, as requested by Al Viro.

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/kernel/entry-common.S |    9 ++-------
 arch/arm/kernel/ptrace.c       |   39 +++++++++++++++++++++++++--------------
 2 files changed, 27 insertions(+), 21 deletions(-)

diff --git a/arch/arm/kernel/entry-common.S b/arch/arm/kernel/entry-common.S
index 3471175..c781012 100644
--- a/arch/arm/kernel/entry-common.S
+++ b/arch/arm/kernel/entry-common.S
@@ -418,13 +418,8 @@ local_restart:
 	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:
+	tst	r10, #_TIF_SECCOMP		@ is seccomp enabled?
+	bne	__sys_trace
 #endif
 
 	tst	r10, #_TIF_SYSCALL_WORK		@ are we tracing syscalls?
diff --git a/arch/arm/kernel/ptrace.c b/arch/arm/kernel/ptrace.c
index 739db3a..6b0e14b 100644
--- a/arch/arm/kernel/ptrace.c
+++ b/arch/arm/kernel/ptrace.c
@@ -916,13 +916,15 @@ enum ptrace_syscall_dir {
 	PTRACE_SYSCALL_EXIT,
 };
 
-static int ptrace_syscall_trace(struct pt_regs *regs, int scno,
-				enum ptrace_syscall_dir dir)
+asmlinkage int syscall_trace_enter(struct pt_regs *regs, int scno)
 {
 	unsigned long ip;
 
 	current_thread_info()->syscall = scno;
 
+	if (secure_computing(scno) == -1)
+		return -1;
+
 	if (!test_thread_flag(TIF_SYSCALL_TRACE))
 		return scno;
 
@@ -931,20 +933,13 @@ static int ptrace_syscall_trace(struct pt_regs *regs, int scno,
 	 * IP = 0 -> entry, =1 -> exit
 	 */
 	ip = regs->ARM_ip;
-	regs->ARM_ip = dir;
-
-	if (dir == PTRACE_SYSCALL_EXIT)
-		tracehook_report_syscall_exit(regs, 0);
-	else if (tracehook_report_syscall_entry(regs))
+	regs->ARM_ip = PTRACE_SYSCALL_ENTER;
+	if (tracehook_report_syscall_entry(regs))
 		current_thread_info()->syscall = -1;
-
 	regs->ARM_ip = ip;
-	return current_thread_info()->syscall;
-}
 
-asmlinkage int syscall_trace_enter(struct pt_regs *regs, int scno)
-{
-	scno = ptrace_syscall_trace(regs, scno, PTRACE_SYSCALL_ENTER);
+	scno = current_thread_info()->syscall;
+
 	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,
@@ -954,7 +949,23 @@ asmlinkage int syscall_trace_enter(struct pt_regs *regs, int scno)
 
 asmlinkage int syscall_trace_exit(struct pt_regs *regs, int scno)
 {
-	scno = ptrace_syscall_trace(regs, scno, PTRACE_SYSCALL_EXIT);
+	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
+	 */
+	ip = regs->ARM_ip;
+	regs->ARM_ip = PTRACE_SYSCALL_EXIT;
+	tracehook_report_syscall_exit(regs, 0);
+	regs->ARM_ip = ip;
+
+	scno = current_thread_info()->syscall;
 	if (test_thread_flag(TIF_SYSCALL_TRACEPOINT))
 		trace_sys_exit(regs, scno);
 	audit_syscall_exit(regs);
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 3/4] arch/arm: allow a scno of -1 to not cause a SIGILL
  2012-11-01 19:46 [PATCH v2 0/4] arch/arm: support seccomp Kees Cook
  2012-11-01 19:46 ` [PATCH 1/4] arch/arm: add syscall_get_arch Kees Cook
  2012-11-01 19:46 ` [PATCH 2/4] arch/arm: move secure_computing into trace Kees Cook
@ 2012-11-01 19:46 ` Kees Cook
  2012-11-01 20:25   ` Russell King - ARM Linux
  2012-11-01 19:46 ` [PATCH 4/4] arch/arm: select HAVE_ARCH_SECCOMP_FILTER Kees Cook
  3 siblings, 1 reply; 15+ messages in thread
From: Kees Cook @ 2012-11-01 19:46 UTC (permalink / raw)
  To: linux-kernel
  Cc: Russell King, Will Deacon, Will Drewry, Geremy Condra, Al Viro,
	Catalin Marinas, Kees Cook

From: Will Drewry <wad@chromium.org>

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 lightweight check to arm_syscall()
to make sure that ARM behaves the same way.

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

diff --git a/arch/arm/kernel/traps.c b/arch/arm/kernel/traps.c
index b0179b8..f303ea6 100644
--- a/arch/arm/kernel/traps.c
+++ b/arch/arm/kernel/traps.c
@@ -540,6 +540,10 @@ asmlinkage int arm_syscall(int no, struct pt_regs *regs)
 	struct thread_info *thread = current_thread_info();
 	siginfo_t info;
 
+	/* Emulate/fallthrough. */
+	if (no == -1)
+		return regs->ARM_r0;
+
 	if ((no >> 16) != (__ARM_NR_BASE>> 16))
 		return bad_syscall(no, regs);
 
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 4/4] arch/arm: select HAVE_ARCH_SECCOMP_FILTER
  2012-11-01 19:46 [PATCH v2 0/4] arch/arm: support seccomp Kees Cook
                   ` (2 preceding siblings ...)
  2012-11-01 19:46 ` [PATCH 3/4] arch/arm: allow a scno of -1 to not cause a SIGILL Kees Cook
@ 2012-11-01 19:46 ` Kees Cook
  3 siblings, 0 replies; 15+ messages in thread
From: Kees Cook @ 2012-11-01 19:46 UTC (permalink / raw)
  To: linux-kernel
  Cc: Russell King, Will Deacon, Will Drewry, Geremy Condra, Al Viro,
	Catalin Marinas, 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


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH 3/4] arch/arm: allow a scno of -1 to not cause a SIGILL
  2012-11-01 19:46 ` [PATCH 3/4] arch/arm: allow a scno of -1 to not cause a SIGILL Kees Cook
@ 2012-11-01 20:25   ` Russell King - ARM Linux
  2012-11-01 21:51     ` Kees Cook
  0 siblings, 1 reply; 15+ messages in thread
From: Russell King - ARM Linux @ 2012-11-01 20:25 UTC (permalink / raw)
  To: Kees Cook
  Cc: linux-kernel, Will Deacon, Will Drewry, Geremy Condra, Al Viro,
	Catalin Marinas

On Thu, Nov 01, 2012 at 12:46:38PM -0700, Kees Cook wrote:
> From: Will Drewry <wad@chromium.org>
> 
> 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 lightweight check to arm_syscall()
> to make sure that ARM behaves the same way.
> 
> Signed-off-by: Will Drewry <wad@chromium.org>
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
>  arch/arm/kernel/traps.c |    4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/arch/arm/kernel/traps.c b/arch/arm/kernel/traps.c
> index b0179b8..f303ea6 100644
> --- a/arch/arm/kernel/traps.c
> +++ b/arch/arm/kernel/traps.c
> @@ -540,6 +540,10 @@ asmlinkage int arm_syscall(int no, struct pt_regs *regs)
>  	struct thread_info *thread = current_thread_info();
>  	siginfo_t info;
>  
> +	/* Emulate/fallthrough. */
> +	if (no == -1)
> +		return regs->ARM_r0;
> +

This won't work properly with OABI.  The problem is that OABI has an
offset on its syscall numbers which is removed/added at appropriate
times, and this is one of the places where it's put back.  So you end
up with -1 XOR 0x900000 here, not -1.

It'd probably be better to do this check in the asm code here, which
prevents that yuckyness from affecting this.

__sys_trace:
        mov     r1, scno
        add     r0, sp, #S_OFF
        bl      syscall_trace_enter

        adr     lr, BSYM(__sys_trace_return)    @ return address
        mov     scno, r0                        @ syscall number (possibly new)
        add     r1, sp, #S_R0 + S_OFF           @ pointer to regs
        cmp     scno, #NR_syscalls              @ check upper syscall limit
        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
+	cmp	scno, #-1
        bne     2b
+	b	ret_slow_syscall


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2/4] arch/arm: move secure_computing into trace
  2012-11-01 19:46 ` [PATCH 2/4] arch/arm: move secure_computing into trace Kees Cook
@ 2012-11-01 20:33   ` Russell King - ARM Linux
  2012-11-01 20:50     ` Kees Cook
  0 siblings, 1 reply; 15+ messages in thread
From: Russell King - ARM Linux @ 2012-11-01 20:33 UTC (permalink / raw)
  To: Kees Cook
  Cc: linux-kernel, Will Deacon, Will Drewry, Geremy Condra, Al Viro,
	Catalin Marinas

On Thu, Nov 01, 2012 at 12:46:37PM -0700, Kees Cook wrote:
>  #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:
> +	tst	r10, #_TIF_SECCOMP		@ is seccomp enabled?
> +	bne	__sys_trace
>  #endif
>  
>  	tst	r10, #_TIF_SYSCALL_WORK		@ are we tracing syscalls?

It's pointless having:

	tst	r10, #_TIF_SECCOMP
	bne	__sys_trace
	tst	r10, #_TIF_SYSCALL_WORK
	bne	__sys_trace

Instead, make TIF_SECCOMP be bit 11, combine it into _TIF_SYSCALL_WORK, and
eliminate all of that CONFIG_SECCOMP block.

> diff --git a/arch/arm/kernel/ptrace.c b/arch/arm/kernel/ptrace.c
> index 739db3a..6b0e14b 100644
> --- a/arch/arm/kernel/ptrace.c
> +++ b/arch/arm/kernel/ptrace.c
> @@ -916,13 +916,15 @@ enum ptrace_syscall_dir {
>  	PTRACE_SYSCALL_EXIT,
>  };
>  
> -static int ptrace_syscall_trace(struct pt_regs *regs, int scno,
> -				enum ptrace_syscall_dir dir)
> +asmlinkage int syscall_trace_enter(struct pt_regs *regs, int scno)
>  {
>  	unsigned long ip;
>  
>  	current_thread_info()->syscall = scno;
>  
> +	if (secure_computing(scno) == -1)
> +		return -1;
> +
>  	if (!test_thread_flag(TIF_SYSCALL_TRACE))
>  		return scno;

I'm not sure this change is correct (combined with your hunk below).
What if we have auditing enabled but trace disabled?  How do we reach
audit_syscall_entry()?  Or the tracehook stuff?

This patch looks wrong in too many ways.

> @@ -931,20 +933,13 @@ static int ptrace_syscall_trace(struct pt_regs *regs, int scno,
>  	 * IP = 0 -> entry, =1 -> exit
>  	 */
>  	ip = regs->ARM_ip;
> -	regs->ARM_ip = dir;
> -
> -	if (dir == PTRACE_SYSCALL_EXIT)
> -		tracehook_report_syscall_exit(regs, 0);
> -	else if (tracehook_report_syscall_entry(regs))
> +	regs->ARM_ip = PTRACE_SYSCALL_ENTER;
> +	if (tracehook_report_syscall_entry(regs))
>  		current_thread_info()->syscall = -1;
> -
>  	regs->ARM_ip = ip;
> -	return current_thread_info()->syscall;
> -}
>  
> -asmlinkage int syscall_trace_enter(struct pt_regs *regs, int scno)
> -{
> -	scno = ptrace_syscall_trace(regs, scno, PTRACE_SYSCALL_ENTER);
> +	scno = current_thread_info()->syscall;
> +
>  	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,
> @@ -954,7 +949,23 @@ asmlinkage int syscall_trace_enter(struct pt_regs *regs, int scno)
>  
>  asmlinkage int syscall_trace_exit(struct pt_regs *regs, int scno)
>  {
> -	scno = ptrace_syscall_trace(regs, scno, PTRACE_SYSCALL_EXIT);
> +	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
> +	 */
> +	ip = regs->ARM_ip;
> +	regs->ARM_ip = PTRACE_SYSCALL_EXIT;
> +	tracehook_report_syscall_exit(regs, 0);
> +	regs->ARM_ip = ip;
> +
> +	scno = current_thread_info()->syscall;
>  	if (test_thread_flag(TIF_SYSCALL_TRACEPOINT))
>  		trace_sys_exit(regs, scno);
>  	audit_syscall_exit(regs);
> -- 
> 1.7.9.5
> 

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2/4] arch/arm: move secure_computing into trace
  2012-11-01 20:33   ` Russell King - ARM Linux
@ 2012-11-01 20:50     ` Kees Cook
  0 siblings, 0 replies; 15+ messages in thread
From: Kees Cook @ 2012-11-01 20:50 UTC (permalink / raw)
  To: Russell King - ARM Linux
  Cc: linux-kernel, Will Deacon, Will Drewry, Geremy Condra, Al Viro,
	Catalin Marinas

On Thu, Nov 1, 2012 at 1:33 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> It's pointless having:
>
>         tst     r10, #_TIF_SECCOMP
>         bne     __sys_trace
>         tst     r10, #_TIF_SYSCALL_WORK
>         bne     __sys_trace
>
> Instead, make TIF_SECCOMP be bit 11, combine it into _TIF_SYSCALL_WORK, and
> eliminate all of that CONFIG_SECCOMP block.

Ah! Good point; I'd missed that _WORK was a bit field. I'll make those changes.

>> diff --git a/arch/arm/kernel/ptrace.c b/arch/arm/kernel/ptrace.c
>> index 739db3a..6b0e14b 100644
>> --- a/arch/arm/kernel/ptrace.c
>> +++ b/arch/arm/kernel/ptrace.c
>> @@ -916,13 +916,15 @@ enum ptrace_syscall_dir {
>>       PTRACE_SYSCALL_EXIT,
>>  };
>>
>> -static int ptrace_syscall_trace(struct pt_regs *regs, int scno,
>> -                             enum ptrace_syscall_dir dir)
>> +asmlinkage int syscall_trace_enter(struct pt_regs *regs, int scno)
>
> I'm not sure this change is correct (combined with your hunk below).
> What if we have auditing enabled but trace disabled?  How do we reach
> audit_syscall_entry()?  Or the tracehook stuff?
>
> This patch looks wrong in too many ways.

Oh, yeah, you're totally right. I will fix that up. Thanks!

-Kees

-- 
Kees Cook
Chrome OS Security

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 3/4] arch/arm: allow a scno of -1 to not cause a SIGILL
  2012-11-01 20:25   ` Russell King - ARM Linux
@ 2012-11-01 21:51     ` Kees Cook
  0 siblings, 0 replies; 15+ messages in thread
From: Kees Cook @ 2012-11-01 21:51 UTC (permalink / raw)
  To: Russell King - ARM Linux
  Cc: linux-kernel, Will Deacon, Will Drewry, Geremy Condra, Al Viro,
	Catalin Marinas

On Thu, Nov 1, 2012 at 1:25 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Thu, Nov 01, 2012 at 12:46:38PM -0700, Kees Cook wrote:
>> From: Will Drewry <wad@chromium.org>
>>
>> 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 lightweight check to arm_syscall()
>> to make sure that ARM behaves the same way.
>>
>> Signed-off-by: Will Drewry <wad@chromium.org>
>> Signed-off-by: Kees Cook <keescook@chromium.org>
>> ---
>>  arch/arm/kernel/traps.c |    4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/arch/arm/kernel/traps.c b/arch/arm/kernel/traps.c
>> index b0179b8..f303ea6 100644
>> --- a/arch/arm/kernel/traps.c
>> +++ b/arch/arm/kernel/traps.c
>> @@ -540,6 +540,10 @@ asmlinkage int arm_syscall(int no, struct pt_regs *regs)
>>       struct thread_info *thread = current_thread_info();
>>       siginfo_t info;
>>
>> +     /* Emulate/fallthrough. */
>> +     if (no == -1)
>> +             return regs->ARM_r0;
>> +
>
> This won't work properly with OABI.  The problem is that OABI has an
> offset on its syscall numbers which is removed/added at appropriate
> times, and this is one of the places where it's put back.  So you end
> up with -1 XOR 0x900000 here, not -1.
>
> It'd probably be better to do this check in the asm code here, which
> prevents that yuckyness from affecting this.
>
> __sys_trace:
>         mov     r1, scno
>         add     r0, sp, #S_OFF
>         bl      syscall_trace_enter
>
>         adr     lr, BSYM(__sys_trace_return)    @ return address
>         mov     scno, r0                        @ syscall number (possibly new)
>         add     r1, sp, #S_R0 + S_OFF           @ pointer to regs
>         cmp     scno, #NR_syscalls              @ check upper syscall limit
>         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
> +       cmp     scno, #-1
>         bne     2b
> +       b       ret_slow_syscall
>

Ah! Good call, yes. I'll use this and include it in a v3 posting. Thanks!

-Kees

-- 
Kees Cook
Chrome OS Security

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 1/4] arch/arm: add syscall_get_arch
  2012-11-10 22:44 [PATCH v5 0/4] arch/arm: support seccomp Kees Cook
@ 2012-11-10 22:44   ` Kees Cook
  0 siblings, 0 replies; 15+ messages in thread
From: Kees Cook @ 2012-11-10 22:44 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: linux-kernel, Russell King, Will Deacon, Geremy Condra,
	Catalin Marinas, Al Viro, Kees Cook, Will Drewry

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


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 1/4] arch/arm: add syscall_get_arch
@ 2012-11-10 22:44   ` Kees Cook
  0 siblings, 0 replies; 15+ messages in thread
From: Kees Cook @ 2012-11-10 22:44 UTC (permalink / raw)
  To: linux-arm-kernel

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

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 1/4] arch/arm: add syscall_get_arch
  2012-11-08 20:59 [PATCH v4 0/4] arch/arm: support seccomp Kees Cook
@ 2012-11-08 20:59 ` Kees Cook
  0 siblings, 0 replies; 15+ messages in thread
From: Kees Cook @ 2012-11-08 20:59 UTC (permalink / raw)
  To: linux-kernel
  Cc: Russell King, Will Deacon, Geremy Condra, Catalin Marinas,
	Al Viro, Kees Cook, Will Drewry

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


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH 1/4] arch/arm: add syscall_get_arch
  2012-11-02  0:14 ` [PATCH 1/4] arch/arm: add syscall_get_arch Kees Cook
@ 2012-11-05 13:29   ` Will Deacon
  0 siblings, 0 replies; 15+ messages in thread
From: Will Deacon @ 2012-11-05 13:29 UTC (permalink / raw)
  To: Kees Cook
  Cc: linux-kernel, Russell King, Will Drewry, Geremy Condra, Al Viro,
	Catalin Marinas

On Fri, Nov 02, 2012 at 12:14:57AM +0000, Kees Cook wrote:
> 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 |   13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/arch/arm/include/asm/syscall.h b/arch/arm/include/asm/syscall.h
> index 9fdded6..803f433 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,15 @@ 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. */
> +#ifdef __ARMEB__
> +	return AUDIT_ARCH_ARMEB;
> +#else
> +	return AUDIT_ARCH_ARM;
> +#endif
> +}

I think you can just use AUDIT_ARCH_ARM unconditionally here as the syscall
ABI is not related to the endianness of the CPU. I believe AUDIT_ARCH_ARMEB
was added accidentally because somebody incorrectly thought the -EB suffix
signified EABI.

If it wasn't in a user-header I'd be all for deleting the old definition...

Will

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 1/4] arch/arm: add syscall_get_arch
  2012-11-02  0:14 [PATCH v3 0/4] arch/arm: support seccomp Kees Cook
@ 2012-11-02  0:14 ` Kees Cook
  2012-11-05 13:29   ` Will Deacon
  0 siblings, 1 reply; 15+ messages in thread
From: Kees Cook @ 2012-11-02  0:14 UTC (permalink / raw)
  To: linux-kernel
  Cc: Russell King, Will Deacon, Will Drewry, Geremy Condra, Al Viro,
	Catalin Marinas, 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 |   13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/arch/arm/include/asm/syscall.h b/arch/arm/include/asm/syscall.h
index 9fdded6..803f433 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,15 @@ 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. */
+#ifdef __ARMEB__
+	return AUDIT_ARCH_ARMEB;
+#else
+	return AUDIT_ARCH_ARM;
+#endif
+}
+
 #endif /* _ASM_ARM_SYSCALL_H */
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 1/4] arch/arm: add syscall_get_arch
  2012-10-30  0:41 [PATCH 0/4] arch/arm: support seccomp Kees Cook
@ 2012-10-30  0:41 ` Kees Cook
  0 siblings, 0 replies; 15+ messages in thread
From: Kees Cook @ 2012-10-30  0:41 UTC (permalink / raw)
  To: linux-kernel
  Cc: Russell King, Will Deacon, Will Drewry, Geremy Condra, 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 |   13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/arch/arm/include/asm/syscall.h b/arch/arm/include/asm/syscall.h
index 9fdded6..803f433 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,15 @@ 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. */
+#ifdef __ARMEB__
+	return AUDIT_ARCH_ARMEB;
+#else
+	return AUDIT_ARCH_ARM;
+#endif
+}
+
 #endif /* _ASM_ARM_SYSCALL_H */
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2012-11-10 22:46 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-11-01 19:46 [PATCH v2 0/4] arch/arm: support seccomp Kees Cook
2012-11-01 19:46 ` [PATCH 1/4] arch/arm: add syscall_get_arch Kees Cook
2012-11-01 19:46 ` [PATCH 2/4] arch/arm: move secure_computing into trace Kees Cook
2012-11-01 20:33   ` Russell King - ARM Linux
2012-11-01 20:50     ` Kees Cook
2012-11-01 19:46 ` [PATCH 3/4] arch/arm: allow a scno of -1 to not cause a SIGILL Kees Cook
2012-11-01 20:25   ` Russell King - ARM Linux
2012-11-01 21:51     ` Kees Cook
2012-11-01 19:46 ` [PATCH 4/4] arch/arm: select HAVE_ARCH_SECCOMP_FILTER Kees Cook
  -- strict thread matches above, loose matches on Subject: below --
2012-11-10 22:44 [PATCH v5 0/4] arch/arm: support seccomp Kees Cook
2012-11-10 22:44 ` [PATCH 1/4] arch/arm: add syscall_get_arch Kees Cook
2012-11-10 22:44   ` Kees Cook
2012-11-08 20:59 [PATCH v4 0/4] arch/arm: support seccomp Kees Cook
2012-11-08 20:59 ` [PATCH 1/4] arch/arm: add syscall_get_arch Kees Cook
2012-11-02  0:14 [PATCH v3 0/4] arch/arm: support seccomp Kees Cook
2012-11-02  0:14 ` [PATCH 1/4] arch/arm: add syscall_get_arch Kees Cook
2012-11-05 13:29   ` Will Deacon
2012-10-30  0:41 [PATCH 0/4] arch/arm: support seccomp Kees Cook
2012-10-30  0:41 ` [PATCH 1/4] arch/arm: add syscall_get_arch Kees Cook

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.