On Thu, Jun 9, 2016 at 3:52 PM, Andy Lutomirski [off-list ref] wrote:

On Thu, Jun 9, 2016 at 2:01 PM, Kees Cook [off-list ref] wrote:

quoted

This moves seccomp after ptrace on x86 to that seccomp can catch changes
made by ptrace. Emulation should skip the rest of processing too.

We can get rid of test_thread_flag because there's no longer any
opportunity for seccomp to mess with ptrace state before invoking
ptrace.

Suggested-by: Andy Lutomirski <luto@kernel.org>
Signed-off-by: Kees Cook <redacted>
Cc: x86@kernel.org
Cc: Andy Lutomirski <luto@kernel.org>
---
 arch/x86/entry/common.c | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/arch/x86/entry/common.c b/arch/x86/entry/common.c
index df56ca394877..81c0e12d831c 100644
--- a/arch/x86/entry/common.c
+++ b/arch/x86/entry/common.c

@@ -73,6 +73,7 @@ static long syscall_trace_enter(struct pt_regs *regs)

        struct thread_info *ti = pt_regs_to_thread_info(regs);
        unsigned long ret = 0;
+       bool emulated = false;
        u32 work;

        if (IS_ENABLED(CONFIG_DEBUG_ENTRY))

@@ -80,11 +81,19 @@ static long syscall_trace_enter(struct pt_regs *regs)

        work = ACCESS_ONCE(ti->flags) & _TIF_WORK_SYSCALL_ENTRY;

+       if (unlikely(work & _TIF_SYSCALL_EMU))
+               emulated = true;
+
+       if ((emulated || (work & _TIF_SYSCALL_TRACE)) &&
+           tracehook_report_syscall_entry(regs))
+               return -1L;
+
+       if (emulated)
+               return -1L;
+

I think that this code will result in ptrace-induced skips calling the
audit exit hook but not the audit entry hook.  I don't know whether
this is a problem.  It's also worth making sure that ptracing a
seccomp-skipped syscall calls the exit hook with the right regs.

I suspect it's fine, but I want to think about it a little bit more.

I poked at it a bit and this seems to work correctly.
selftests/x86/ptrace_syscall.c exercises PTRACE_SYSCALL_EMU pretty
well, and it still passes.

--Andy