[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:
+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.