Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1407745
| From | Jann Horn <jann@thejh.net> |
|---|---|
| Newsgroups | linux.kernel |
| Subject | Re: [PATCH] seccomp: plug syscall-dodging ptrace hole |
| Date | 2016-05-27 01:00 +0200 |
| Message-ID | <rDcpY-3Lo-15@gated-at.bofh.it> (permalink) |
| References | <rDaHw-2Xq-5@gated-at.bofh.it> |
| Organization | linux.* mail to news gateway |
[Multipart message — attachments visible in raw view] - view raw
On Thu, May 26, 2016 at 02:04:50PM -0700, Kees Cook wrote:
> One problem with seccomp was that ptrace could be used to change a
> syscall after seccomp filtering had completed. This was a well documented
> limitation, and it was recommended to block ptrace when defining a filter
> to avoid this problem. This can be quite a limitation for containers or
> other places where ptrace is desired even under seccomp filters.
>
> Since seccomp filtering has been split into pre-trace and trace phases
> (phase1 and phase2 respectively), it's possible to re-run phase1 seccomp
> after ptrace. This makes that change, and updates the test suite for
> both SECCOMP_RET_TRACE and PTRACE_SYSCALL manipulation.
Looks good to me. As far as I can tell, there are no codepaths that allow
manipulation of syscall arguments via ptrace register modification without
going through tracehook_report_syscall_entry() or seccomp_phase2(), and
the checks look good, too.
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
> include/linux/seccomp.h | 6 +
> include/linux/tracehook.h | 8 +-
> kernel/seccomp.c | 42 ++++++
> tools/testing/selftests/seccomp/seccomp_bpf.c | 176 ++++++++++++++++++++++++--
> 4 files changed, 220 insertions(+), 12 deletions(-)
>
> diff --git a/include/linux/seccomp.h b/include/linux/seccomp.h
> index 2296e6b2f690..e2b72394c200 100644
> --- a/include/linux/seccomp.h
> +++ b/include/linux/seccomp.h
> @@ -85,6 +85,7 @@ static inline int seccomp_mode(struct seccomp *s)
> #ifdef CONFIG_SECCOMP_FILTER
> extern void put_seccomp_filter(struct task_struct *tsk);
> extern void get_seccomp_filter(struct task_struct *tsk);
> +extern int seccomp_phase1_recheck(void);
> #else /* CONFIG_SECCOMP_FILTER */
> static inline void put_seccomp_filter(struct task_struct *tsk)
> {
> @@ -94,6 +95,11 @@ static inline void get_seccomp_filter(struct task_struct *tsk)
> {
> return;
> }
> +
> +static inline int seccomp_phase1_recheck(void)
> +{
> + return 0;
> +}
> #endif /* CONFIG_SECCOMP_FILTER */
>
> #if defined(CONFIG_SECCOMP_FILTER) && defined(CONFIG_CHECKPOINT_RESTORE)
> diff --git a/include/linux/tracehook.h b/include/linux/tracehook.h
> index 26c152122a42..69b584d88508 100644
> --- a/include/linux/tracehook.h
> +++ b/include/linux/tracehook.h
> @@ -48,6 +48,7 @@
>
> #include <linux/sched.h>
> #include <linux/ptrace.h>
> +#include <linux/seccomp.h>
> #include <linux/security.h>
> #include <linux/task_work.h>
> #include <linux/memcontrol.h>
> @@ -100,7 +101,12 @@ static inline int ptrace_report_syscall(struct pt_regs *regs)
> static inline __must_check int tracehook_report_syscall_entry(
> struct pt_regs *regs)
> {
> - return ptrace_report_syscall(regs);
> + int skip;
> +
> + skip = ptrace_report_syscall(regs);
> + if (skip)
> + return skip;
> + return seccomp_phase1_recheck();
> }
>
> /**
> diff --git a/kernel/seccomp.c b/kernel/seccomp.c
> index 7002796f14a4..6eaa3a1c5edb 100644
> --- a/kernel/seccomp.c
> +++ b/kernel/seccomp.c
> @@ -665,6 +665,46 @@ u32 seccomp_phase1(struct seccomp_data *sd)
> }
>
> /**
> + * seccomp_phase1_recheck() - recheck phase1 in the context of ptrace
> + *
> + * This re-runs phase 1 seccomp checks in the case where ptrace may have
> + * just changed things out from under us.
> + *
> + * Returns 0 if the syscall should be processed or -1 to skip the syscall.
> + */
> +int seccomp_phase1_recheck(void)
> +{
> + u32 action;
> +
> + /* If we're not under seccomp, continue normally. */
> + if (!test_thread_flag(TIF_SECCOMP))
> + return 0;
> +
> + /* Pass NULL struct seccomp_data to force reload after ptrace. */
> + action = seccomp_phase1(NULL);
> + switch (action) {
> + case SECCOMP_PHASE1_OK:
> + /* Passes seccomp, continue normally. */
> + break;
> + case SECCOMP_PHASE1_SKIP:
> + /* Skip the syscall. */
> + return -1;
> + default:
> + if ((action & SECCOMP_RET_ACTION) != SECCOMP_RET_TRACE) {
> + /* Impossible return value: kill the process. */
> + do_exit(SIGSYS);
> + }
> + /*
> + * We've hit a trace request, but ptrace already put us
> + * into this state, so just continue.
> + */
> + break;
> + }
> +
> + return 0;
> +}
> +
> +/**
> * seccomp_phase2() - finish slow path seccomp work for the current syscall
> * @phase1_result: The return value from seccomp_phase1()
> *
> @@ -701,6 +741,8 @@ int seccomp_phase2(u32 phase1_result)
> do_exit(SIGSYS);
> if (syscall_get_nr(current, regs) < 0)
> return -1; /* Explicit request to skip. */
> + if (seccomp_phase1_recheck() < 0)
> + return -1;
>
> return 0;
> }
[...]
Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread
[PATCH] seccomp: plug syscall-dodging ptrace hole Kees Cook <keescook@chromium.org> - 2016-05-26 23:10 +0200
Re: [PATCH] seccomp: plug syscall-dodging ptrace hole Jann Horn <jann@thejh.net> - 2016-05-27 01:00 +0200
Re: [PATCH] seccomp: plug syscall-dodging ptrace hole Andy Lutomirski <luto@amacapital.net> - 2016-05-27 04:20 +0200
Re: [PATCH] seccomp: plug syscall-dodging ptrace hole Kees Cook <keescook@chromium.org> - 2016-05-27 04:50 +0200
Re: [PATCH] seccomp: plug syscall-dodging ptrace hole Andy Lutomirski <luto@amacapital.net> - 2016-05-27 06:50 +0200
Re: [PATCH] seccomp: plug syscall-dodging ptrace hole Kees Cook <keescook@chromium.org> - 2016-05-27 20:50 +0200
Re: [PATCH] seccomp: plug syscall-dodging ptrace hole Andy Lutomirski <luto@amacapital.net> - 2016-05-27 22:00 +0200
Re: [PATCH] seccomp: plug syscall-dodging ptrace hole Andy Lutomirski <luto@amacapital.net> - 2016-05-27 22:20 +0200
Re: [PATCH] seccomp: plug syscall-dodging ptrace hole Kees Cook <keescook@chromium.org> - 2016-05-28 00:40 +0200
Re: [PATCH] seccomp: plug syscall-dodging ptrace hole Andy Lutomirski <luto@amacapital.net> - 2016-05-28 01:30 +0200
Re: [PATCH] seccomp: plug syscall-dodging ptrace hole Kees Cook <keescook@chromium.org> - 2016-05-28 04:40 +0200
csiph-web