[PATCH v8 9/9] seccomp: implement SECCOMP_FILTER_FLAG_TSYNC
From: oleg@redhat.com (Oleg Nesterov)
Date: 2014-06-25 14:23:42
Also in:
linux-api, linux-arch, linux-mips, lkml
On 06/24, Kees Cook wrote:
quoted hunk ↗ jump to hunk
+static void seccomp_sync_threads(void) +{ + struct task_struct *thread, *caller; + + BUG_ON(!spin_is_locked(¤t->sighand->siglock)); + + /* Synchronize all threads. */ + caller = current; + for_each_thread(caller, thread) { + /* Get a task reference for the new leaf node. */ + get_seccomp_filter(caller); + /* + * Drop the task reference to the shared ancestor since + * current's path will hold a reference. (This also + * allows a put before the assignment.) + */ + put_seccomp_filter(thread); + thread->seccomp.filter = caller->seccomp.filter; + /* Opt the other thread into seccomp if needed. + * As threads are considered to be trust-realm + * equivalent (see ptrace_may_access), it is safe to + * allow one thread to transition the other. + */ + if (thread->seccomp.mode == SECCOMP_MODE_DISABLED) { + /* + * Don't let an unprivileged task work around + * the no_new_privs restriction by creating + * a thread that sets it up, enters seccomp, + * then dies. + */ + if (task_no_new_privs(caller)) + task_set_no_new_privs(thread); + + seccomp_assign_mode(thread, SECCOMP_MODE_FILTER); + } + } +}
OK, personally I think this all make sense. I even think that perhaps SECCOMP_FILTER_FLAG_TSYNC should allow filter == NULL, a thread might want to "sync" without adding the new filter, but this is minor/offtopic. But. Doesn't this change add the new security hole? Obviously, we should not allow to install a filter and then (say) exec a suid binary, that is why we have no_new_privs/LSM_UNSAFE_NO_NEW_PRIVS. But what if "thread->seccomp.filter = caller->seccomp.filter" races with any user of task_no_new_privs() ? Say, suppose this thread has already passed check_unsafe_exec/etc and it is going to exec the suid binary? Oleg.