Re: [PATCH 2/2] kthread, cgroup: close race window where new kthreads can be migrated to non-root cgroups

From: Tejun Heo
Date: Thu Mar 16 2017 - 12:07:00 EST


Hello,

On Thu, Mar 16, 2017 at 04:02:34PM +0100, Oleg Nesterov wrote:
> > +bool kthread_initialized(struct task_struct *k)
> > +{
> > + struct kthread *kthread = to_kthread(k);
> > +
> > + return kthread && test_bit(KTHREAD_INITIALIZED, &kthread->flags);
> > +}
>
> Not sure I understand...
>
> With this patch you can no longer migrate a kernel thread created by
> kernel_thread() ? Note that to_kthread() is NULL unless it was created
> by kthread_create().

Yeah, what it does is preventing migration of kthreads until the
kthread owner wakes it up for the first time. The problem is that
kthread_bind() seals up future cgroup migrations from userland but
doesn't move back the kthread to the root cgroup, so the userland has
a window where it can mangle with cgroup membership inbetween and
break things.

The NULL test is there because the test may be performed before the
kthread itself sets up its struct kthread.

An alternative approach could be making kthread_bind() migrate the
kthread back to root cgroup, which btw is why affinity is fine as the
function overwrites it after setting NO_SETAFFINITY; however, the
problem there is that userland can put the kthread into !root cgroup
and starve it before it reaches create->done.

Thanks.

--
tejun