Messages in this thread | | | Date | Thu, 28 May 2020 22:36:03 -0700 | From | Kees Cook <> | Subject | Re: [PATCH v2 1/2] seccomp: notify user trap about unused filter |
| |
On Fri, May 29, 2020 at 01:32:03AM +0200, Jann Horn wrote: > On Fri, May 29, 2020 at 1:11 AM Kees Cook <keescook@chromium.org> wrote: > > So, is it safe to detach the filter in release_task()? Has dethreading > > happened yet? i.e. can we race TSYNC? -- is there a possible > > inc-from-zero? > > release_task -> __exit_signal -> __unhash_process -> > list_del_rcu(&p->thread_node) drops us from the thread list under > siglock, which is the same lock TSYNC uses.
Ah, there it is. I missed the __unhash_process() in __exit_signal, but once I saw the call to release_task(), I figured it was safe at that point. So this seems correct:
> > I *think* we can do it > > before the release_thread() call (instead of after cgroup_release()).
> One other interesting thing that can look at seccomp state is > task_seccomp() in procfs - that can still happen at this point. At the > moment, procfs only lets you see the numeric filter state, not the > actual filter contents, so that's not a problem; but if we ever add a > procfs interface for dumping seccomp filters (in addition to the > ptrace interface that already exists), that's something to keep in > mind.
Right -- but we can just reuse the get/put to pin the filter while dumping it from proc (there IS someone working on this feature...)
> > (Actually, all our refcount_inc()s should be > > refcount_inc_not_zero() just for robustness.) > > Eeeh... wouldn't that just make the code more complicated for no good reason?
Sorry, ignore that. I got myself briefly confused -- we're fine; refcount_inc() already does inc-from-zero checking.
-- Kees Cook
| |