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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20170316160544.GC15810@htj.duckdns.org>
Date:   Thu, 16 Mar 2017 12:05:44 -0400
From:   Tejun Heo <tj@...nel.org>
To:     Oleg Nesterov <oleg@...hat.com>
Cc:     Linus Torvalds <torvalds@...ux-foundation.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Thomas Gleixner <tglx@...utronix.de>, Chris Mason <clm@...com>,
        linux-kernel@...r.kernel.org, kernel-team@...com,
        Li Zefan <lizefan@...wei.com>,
        Johannes Weiner <hannes@...xchg.org>, cgroups@...r.kernel.org
Subject: Re: [PATCH 2/2] kthread, cgroup: close race window where new
 kthreads can be migrated to non-root cgroups

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