Re: [PATCH 06/14] x86/ptrace: run seccomp after ptrace

[Andy Lutomirski]

On Thu, Jun 9, 2016 at 2:01 PM, Kees Cook <keescook@xxxxxxxxxxxx> wrote:
> 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@xxxxxxxxxx>
> Signed-off-by: Kees Cook <keescook@xxxxxxxxxxxx>
> Cc: x86@xxxxxxxxxx
> Cc: Andy Lutomirski <luto@xxxxxxxxxx>
> ---
>  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.

--Andy