linux-kernel - Re: patch] cpusets, cgroups: disallow attaching kthreadd

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: <alpine.DEB.2.00.1110181336150.2639@chino.kir.corp.google.com>
Date:	Tue, 18 Oct 2011 13:38:43 -0700 (PDT)
From:	David Rientjes <rientjes@...gle.com>
To:	Mike Galbraith <efault@....de>
cc:	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	LKML <linux-kernel@...r.kernel.org>,
	Tejun Heo <htejun@...il.com>, Li Zefan <lizf@...fujitsu.com>,
	Paul Menage <paul@...lmenage.org>
Subject: Re: patch] cpusets, cgroups: disallow attaching kthreadd

On Tue, 18 Oct 2011, Mike Galbraith wrote:

> From: Peter Zijlstra <a.p.zijlstra@...llo.nl>
> 
> cgroups: disallow attaching kthreadd
> 
> Allowing kthreadd to be moved to a non-root group makes no sense, it being
> a global resource, and needlessly leads unsuspecting users toward trouble.
> 
> 1. An RT workqueue worker thread spawned in a task group with no rt_runtime
> allocated is not schedulable.  Simple user error, but harmful to the box.
> 
> 2. A worker thread which acquires PF_THREAD_BOUND can never leave a cpuset,
> rendering the cpuset immortal.
> 

Eek, this seems like complete overkill, there may be cgroups now or in the 
future that it would be perfectly acceptable to move kthreadd into without 
any negative effects.  There's no reason to restrict it this broadly, 
that's why I thought your change to cpusets was fine and acked it.  It's 
the perfect candidate for what the ->can_attach() pointer for a cgroup can 
identify is problematic for _that_ cgroup.

> Save the user some unexpected trouble, just say no.
> 
> Signed-off-by: Mike Galbraith <efault@....de>
> 

A patch from Peter that only has your sign-off?

> ---
>  kernel/cgroup.c |    9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> Index: linux-3.0-tip/kernel/cgroup.c
> ===================================================================
> --- linux-3.0-tip.orig/kernel/cgroup.c
> +++ linux-3.0-tip/kernel/cgroup.c
> @@ -60,6 +60,7 @@
>  #include <linux/eventfd.h>
>  #include <linux/poll.h>
>  #include <linux/flex_array.h> /* used in cgroup_attach_proc */
> +#include <linux/kthread.h>
>  
>  #include <linux/atomic.h>
>  
> @@ -1824,6 +1825,14 @@ int cgroup_attach_task(struct cgroup *cg
>  	struct cgroup *oldcgrp;
>  	struct cgroupfs_root *root = cgrp->root;
>  
> +	/*
> +	 * Workqueue threads may acquire PF_THREAD_BOUND and become
> +	 * trapped in a cpuset, or RT worker may be born in a cgroup
> +	 * with no rt_runtime allocated.  Just say no.
> +	 */
> +	if (tsk == kthreadd_task)
> +		return -EINVAL;
> +
>  	/* Nothing to do if the task is already in that cgroup */
>  	oldcgrp = task_cgroup_from_root(tsk, root);
>  	if (cgrp == oldcgrp)
> 
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/