Two-phase seccomp and an x86_64 fast path

On my VM, getpid takes about 70ns. Before this patch, adding a single-instruction always-accept seccomp filter added about 134ns of overhead to getpid. With this patch, the overhead is down to about 13ns.

I'm not really thrilled by this patch. It has two main issues:

1. Calling into code in kernel/seccomp.c from assembly feels ugly.

2. The x86 64-bit syscall entry now has four separate code paths:
fast, seccomp only, audit only, and slow. This kind of sucks. Would it be worth trying to rewrite the whole thing in C with a two-phase slow path approach like I'm using here for seccomp?

Signed-off-by Andy Lutomirski <[email protected]>

---
 arch/x86/kernel/entry_64.S | 45 +++++++++++++++++++++++++++++++++++++++++++++
 include/linux/seccomp.h    |  4 ++--
 2 files changed, 47 insertions(+), 2 deletions(-)

arch/x86/kernel/entry_64.S +7 -0

index f9e713a..feb32b2 100644
@@ -683,6 +683,45 @@ sysret_signal:
        FIXUP_TOP_OF_STACK %r11, -ARGOFFSET
        jmp int_check_syscall_exit_work
 
+#ifdef CONFIG_SECCOMP
+       /*
+        * Fast path for seccomp without any other slow path triggers.
+        */
+seccomp_fastpath:
+       /* Build seccomp_data */
+       pushq %r9                               /* args[5] */

arch/x86/kernel/entry_64.S +28 -0

+       addq $8*8, %rsp
+       cmpq $1, %rax
+       ja seccomp_invoke_phase2
+       LOAD_ARGS 0  /* restore clobbered regs */
+       jb system_call_fastpath
+       jmp ret_from_sys_call
+
+seccomp_invoke_phase2:
+       SAVE_REST
+       FIXUP_TOP_OF_STACK %rdi
+       movq %rax,%rdi
+       call seccomp_phase2
+
+       /* if seccomp says to skip, then set orig_ax to -1 and skip */
+       test %eax,%eax
+       jz 1f
+       movq $-1, ORIG_RAX(%rsp)
+1:
+       mov ORIG_RAX(%rsp), %rax                /* reload rax */
+       jmp system_call_post_trace              /* and maybe do the syscall */
+#endif
+
 #ifdef CONFIG_AUDITSYSCALL
        /*
         * Fast path for syscall audit without full syscall trace.

@@ -717,6 +756,10 @@ sysret_audit:
 
        /* Do syscall tracing */
 tracesys:
+#ifdef CONFIG_SECCOMP
+       testl $(_TIF_WORK_SYSCALL_ENTRY & ~_TIF_SECCOMP),TI_flags+THREAD_INFO(%rsp,RIP-ARGOFFSET)
+       jz seccomp_fastpath
+#endif
 #ifdef CONFIG_AUDITSYSCALL
        testl $(_TIF_WORK_SYSCALL_ENTRY & ~_TIF_SYSCALL_AUDIT),TI_flags+THREAD_INFO(%rsp,RIP-ARGOFFSET)
        jz auditsys

@@ -725,6 +768,8 @@ tracesys:
        FIXUP_TOP_OF_STACK %rdi
        movq %rsp,%rdi
        call syscall_trace_enter
+
+system_call_post_trace:
        /*
         * Reload arg registers from stack in case ptrace changed them.
         * We don't reload %rax because syscall_trace_enter() returned

include/linux/seccomp.h +2 -3

index 4fc7a84..d3d4c52 100644
@@ -37,8 +37,8 @@ static inline int secure_computing(void)
 #define SECCOMP_PHASE1_OK      0
 #define SECCOMP_PHASE1_SKIP    1
 
-extern u32 seccomp_phase1(struct seccomp_data *sd);
-int seccomp_phase2(u32 phase1_result);
+asmlinkage __visible extern u32 seccomp_phase1(struct seccomp_data *sd);
+asmlinkage __visible int seccomp_phase2(u32 phase1_result);
 #else
 extern void secure_computing_strict(int this_syscall);
 #endif
--

1.9.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Quoted 4 lines · Andy Lutomirski

On my VM, getpid takes about 70ns. Before this patch, adding a single-instruction always-accept seccomp filter added about 134ns of overhead to getpid. With this patch, the overhead is down to about 13ns.

interesting.
Is this the gain from patch 4 into patch 5 or from patch 0 to patch 5? 13ns is still with seccomp enabled, but without filters?

Quoted 37 lines

I'm not really thrilled by this patch. It has two main issues:

1. Calling into code in kernel/seccomp.c from assembly feels ugly.

2. The x86 64-bit syscall entry now has four separate code paths:
fast, seccomp only, audit only, and slow. This kind of sucks. Would it be worth trying to rewrite the whole thing in C with a two-phase slow path approach like I'm using here for seccomp?

Signed-off-by Andy Lutomirski <[email protected]>

---
 arch/x86/kernel/entry_64.S | 45 +++++++++++++++++++++++++++++++++++++++++++++
 include/linux/seccomp.h    |  4 ++--
 2 files changed, 47 insertions(+), 2 deletions(-)

arch/x86/kernel/entry_64.S +13 -0

index f9e713a..feb32b2 100644
@@ -683,6 +683,45 @@ sysret_signal:
        FIXUP_TOP_OF_STACK %r11, -ARGOFFSET
        jmp int_check_syscall_exit_work
 
+#ifdef CONFIG_SECCOMP
+       /*
+        * Fast path for seccomp without any other slow path triggers.
+        */
+seccomp_fastpath:
+       /* Build seccomp_data */
+       pushq %r9                               /* args[5] */
+       pushq %r8                               /* args[4] */
+       pushq %r10                              /* args[3] */
+       pushq %rdx                              /* args[2] */
+       pushq %rsi                              /* args[1] */
+       pushq %rdi                              /* args[0] */
+       pushq RIP-ARGOFFSET+6*8(%rsp)           /* rip */

arch/x86/kernel/entry_64.S sysret_signal: · line 700

Quoted 2 lines

arch/x86/kernel/entry_64.S +2 -0

+       pushq %rax                              /* nr and junk */
+       movl $AUDIT_ARCH_X86_64, 4(%rsp)        /* arch */

arch/x86/kernel/entry_64.S sysret_signal: · line 702

Quoted 2 lines

arch/x86/kernel/entry_64.S +2 -0

+       movq %rsp, %rdi
+       call seccomp_phase1

the assembler code is pretty much repeating what C does in populate_seccomp_data(). Assuming the whole gain came from patch 5 why asm version is so much faster than C?

it skips SAVE/RESTORE_REST... what else?
If the most of the gain is from all patches combined (mainly from 2 phase approach) then why bother with asm?

Somehow it feels that the gain is due to better branch prediction in asm version. If you change few 'unlikely' in C to 'likely', it may get to the same performance...

btw patches #1-3 look good to me. especially #1 is nice.

13ns is with the simplest nonempty filter. I hope that empty filters don't work.

Quoted 4 lines · Andy Lutomirski

13ns is with the simplest nonempty filter. I hope that empty filters don't work.

Why wouldn't they?

Quoted 6 lines · H. Peter Anvin

Quoted 4 lines · Andy Lutomirski

13ns is with the simplest nonempty filter. I hope that empty filters don't work.

Quoted 1 line

Why wouldn't they?

Is it permissible to fall off the end of a BPF program? I'm getting EINVAL trying to install an actual empty filter. The filter I tested with was:

Quoted 7 lines · Andy Lutomirski

Quoted 6 lines · H. Peter Anvin

Quoted 4 lines · Andy Lutomirski

13ns is with the simplest nonempty filter. I hope that empty filters don't work.

Quoted 6 lines

Quoted 1 line

Why wouldn't they?

Is it permissible to fall off the end of a BPF program? I'm getting EINVAL trying to install an actual empty filter. The filter I tested with was:

What I meant was that there has to be a well-defined behavior for the program falling off the end anyway, and that that should be preserved.

I guess it is possible to require that all code paths must provably reach a termination point.

Quoted 18 lines · H. Peter Anvin

Quoted 7 lines · Andy Lutomirski

Quoted 6 lines · H. Peter Anvin

Quoted 4 lines · Andy Lutomirski

13ns is with the simplest nonempty filter. I hope that empty filters don't work.

Quoted 6 lines

Quoted 1 line

Why wouldn't they?

Is it permissible to fall off the end of a BPF program? I'm getting EINVAL trying to install an actual empty filter. The filter I tested with was:

What I meant was that there has to be a well-defined behavior for the program falling off the end anyway, and that that should be preserved.

Quoted 3 lines

I guess it is possible to require that all code paths must provably reach a termination point.

Dunno. I haven't ever touched any of the actual BPF code. This whole patchset only changes the code that invokes the BPF evaluator.

Quoted 23 lines · Andy Lutomirski

Quoted 21 lines · H. Peter Anvin

Quoted 7 lines · Andy Lutomirski

Quoted 6 lines · H. Peter Anvin

Quoted 4 lines · Andy Lutomirski

13ns is with the simplest nonempty filter. I hope that empty filters don't work.

Quoted 6 lines

Quoted 1 line

Why wouldn't they?

Is it permissible to fall off the end of a BPF program? I'm getting EINVAL trying to install an actual empty filter. The filter I tested with was:

What I meant was that there has to be a well-defined behavior for the program falling off the end anyway, and that that should be preserved.

I guess it is possible to require that all code paths must provably reach a termination point.

Quoted 2 lines

Dunno. I haven't ever touched any of the actual BPF code. This whole patchset only changes the code that invokes the BPF evaluator.

Yes, this is how BPF works: runs to the end or exit early. With seccomp BPF specifically, the return value defaults to kill the process. If a filter was missing (NULL), or empty, or didn't explicitly return with a new value, the default (kill) should be taken.