Re: [PATCH v3 2/4] seccomp: release filter after task is fully dead

[Kees Cook <keescook@chromium.org>]

On Sun, May 31, 2020 at 01:50:29PM +0200, Christian Brauner wrote:
> The seccomp filter used to be released in free_task() which is called
> asynchronously via call_rcu() and assorted mechanisms. Since we need
> to inform tasks waiting on the seccomp notifier when a filter goes empty
> we will notify them as soon as a task has been marked fully dead in
> release_task(). To not split seccomp cleanup into two parts, move
> filter release out of free_task() and into release_task() after we've
> unhashed struct task from struct pid, exited signals, and unlinked it
> from the threadgroups' thread list. We'll put the empty filter
> notification infrastructure into it in a follow up patch.
> 
> This also renames put_seccomp_filter() to seccomp_filter_release() which
> is a more descriptive name of what we're doing here especially once
> we've added the empty filter notification mechanism in there.
> 
> We're also NULL-ing the task's filter tree entrypoint which seems
> cleaner than leaving a dangling pointer in there. Note that this shouldn't
> need any memory barriers since we're calling this when the task is in
> release_task() which means it's EXIT_DEAD. So it can't modify it's seccomp
> filters anymore. You can also see this from the point where we're calling
> seccomp_filter_release(). It's after __exit_signal() and at this point,
> tsk->sighand will already have been NULLed which is required for
> thread-sync and filter installation alike.
> 
> Cc: Tycho Andersen <tycho@tycho.ws>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Matt Denton <mpdenton@google.com>
> Cc: Sargun Dhillon <sargun@sargun.me>
> Cc: Jann Horn <jannh@google.com>
> Cc: Chris Palmer <palmer@google.com>
> Cc: Aleksa Sarai <cyphar@cyphar.com>
> Cc: Robert Sesek <rsesek@google.com>
> Cc: Jeffrey Vander Stoep <jeffv@google.com>
> Cc: Linux Containers <containers@lists.linux-foundation.org>
> Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>

Thanks! Applied with typo fixes to the commit log, a slightly expanded
comment on seccomp_filter_release() to just drive home the reason we
don't need barriers, and a variable renaming to avoid some needless
churn in the coming patches...

-- 
Kees Cook