Messages in this thread | | | Date | Tue, 14 Jan 2014 20:21:18 +0100 | From | Oleg Nesterov <> | Subject | Re: [PATCH 2/2] sys, seccomp: add PR_SECCOMP_EXT and SECCOMP_EXT_ACT_TSYNC |
| |
On 01/13, Will Drewry wrote: > > +static pid_t seccomp_sync_threads(void) > +{ > + struct task_struct *thread, *caller; > + pid_t failed = 0; > + thread = caller = current; > + > + read_lock(&tasklist_lock); > + if (thread_group_empty(caller)) > + goto done; > + while_each_thread(caller, thread) { > + task_lock(thread);
perhaps we take task_lock() to serialize with another caller of seccomp_sync_threads()...
If yes, then perhaps you can use ->siglock instead of tasklist_lock and do not use task_lock(). It would be even better to rely on rcu, but:
> + 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;
As I said, I do not understand this patch yet, but this looks suspicious.
Why we can't race with this thread doing clone(CLONE_THREAD) ? We do not the the new thread yet, but its ->seccomp can be already copied by copy_process(), no?
Oleg.
| |