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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
