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@xxxxxxxxxxxx> 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 _______________________________________________ Containers mailing list Containers@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linuxfoundation.org/mailman/listinfo/containers
