All of lore.kernel.org
 help / color / mirror / Atom feed
From: Oleg Nesterov <oleg-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
To: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: Linus Torvalds
	<torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>,
	Andrew Morton
	<akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>,
	Peter Zijlstra <peterz-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>,
	Thomas Gleixner <tglx-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>,
	Chris Mason <clm-b10kYP2dOMg@public.gmane.org>,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	kernel-team-b10kYP2dOMg@public.gmane.org,
	Li Zefan <lizefan-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>,
	Johannes Weiner <hannes-druUgvl0LCNAfugRpC6u6w@public.gmane.org>,
	cgroups-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Subject: Re: [PATCH 2/2] kthread, cgroup: close race window where new kthreads can be migrated to non-root cgroups
Date: Thu, 16 Mar 2017 17:17:57 +0100	[thread overview]
Message-ID: <20170316161756.GA27613@redhat.com> (raw)
In-Reply-To: <20170316160544.GC15810-piEFEHQLUPpN0TnZuCh8vA@public.gmane.org>

On 03/16, Tejun Heo wrote:
>
> 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.

This is clear,

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

This too.

But this also means that __cgroup_procs_write() will always fail if
this task is a kernel thread which was not created by kthread_create().

Currently you can use kernel_thread() (although you shouldn't) and it
can be migrated, this won't work after your patch.

Oleg.

WARNING: multiple messages have this Message-ID (diff)
From: Oleg Nesterov <oleg@redhat.com>
To: Tejun Heo <tj@kernel.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Thomas Gleixner <tglx@linutronix.de>, Chris Mason <clm@fb.com>,
	linux-kernel@vger.kernel.org, kernel-team@fb.com,
	Li Zefan <lizefan@huawei.com>,
	Johannes Weiner <hannes@cmpxchg.org>,
	cgroups@vger.kernel.org
Subject: Re: [PATCH 2/2] kthread, cgroup: close race window where new kthreads can be migrated to non-root cgroups
Date: Thu, 16 Mar 2017 17:17:57 +0100	[thread overview]
Message-ID: <20170316161756.GA27613@redhat.com> (raw)
In-Reply-To: <20170316160544.GC15810@htj.duckdns.org>

On 03/16, Tejun Heo wrote:
>
> 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.

This is clear,

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

This too.

But this also means that __cgroup_procs_write() will always fail if
this task is a kernel thread which was not created by kthread_create().

Currently you can use kernel_thread() (although you shouldn't) and it
can be migrated, this won't work after your patch.

Oleg.

  parent reply	other threads:[~2017-03-16 16:17 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-03-15 23:18 [PATCH 1/2] kthread: add barriers to set_kthread_struct() and to_kthread() Tejun Heo
2017-03-15 23:19 ` [PATCH 2/2] kthread, cgroup: close race window where new kthreads can be migrated to non-root cgroups Tejun Heo
2017-03-16 15:02   ` Oleg Nesterov
     [not found]     ` <20170316150233.GB24478-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2017-03-16 15:39       ` Oleg Nesterov
2017-03-16 15:39         ` Oleg Nesterov
     [not found]         ` <20170316153925.GA26391-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2017-03-16 16:07           ` Tejun Heo
2017-03-16 16:07             ` Tejun Heo
     [not found]             ` <20170316160734.GD15810-piEFEHQLUPpN0TnZuCh8vA@public.gmane.org>
2017-03-16 16:31               ` Oleg Nesterov
2017-03-16 16:31                 ` Oleg Nesterov
2017-03-16 17:41                 ` Tejun Heo
2017-03-16 16:05     ` Tejun Heo
     [not found]       ` <20170316160544.GC15810-piEFEHQLUPpN0TnZuCh8vA@public.gmane.org>
2017-03-16 16:17         ` Oleg Nesterov [this message]
2017-03-16 16:17           ` Oleg Nesterov
2017-03-16 17:03           ` Tejun Heo
     [not found]   ` <20170315231920.GB13656-piEFEHQLUPpN0TnZuCh8vA@public.gmane.org>
2017-03-16 20:54     ` [PATCH v2] cgroup, kthread: " Tejun Heo
2017-03-16 20:54       ` Tejun Heo
2017-03-17 13:50       ` Oleg Nesterov
2017-03-17 14:44         ` Tejun Heo
2017-03-16 14:54 ` [PATCH 1/2] kthread: add barriers to set_kthread_struct() and to_kthread() Oleg Nesterov
2017-03-16 15:33   ` Tejun Heo
2017-03-16 15:38     ` Tejun Heo
2017-03-16 15:46       ` Oleg Nesterov
2017-03-16 15:55       ` Peter Zijlstra
2017-03-16 16:09         ` Tejun Heo
2017-03-16 16:14           ` Peter Zijlstra

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170316161756.GA27613@redhat.com \
    --to=oleg-h+wxahxf7alqt0dzr+alfa@public.gmane.org \
    --cc=akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org \
    --cc=cgroups-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=clm-b10kYP2dOMg@public.gmane.org \
    --cc=hannes-druUgvl0LCNAfugRpC6u6w@public.gmane.org \
    --cc=kernel-team-b10kYP2dOMg@public.gmane.org \
    --cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=lizefan-hv44wF8Li93QT0dZR+AlfA@public.gmane.org \
    --cc=peterz-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org \
    --cc=tglx-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org \
    --cc=tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \
    --cc=torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.