linux-kernel - Re: [RFC PATCH v2 11/17] cgroup: Implement new thread mode semantics

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]

Date:   Mon, 22 May 2017 13:32:20 -0400
From:   Waiman Long <longman@...hat.com>
To:     Tejun Heo <tj@...nel.org>
Cc:     Li Zefan <lizefan@...wei.com>,
        Johannes Weiner <hannes@...xchg.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...hat.com>, cgroups@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-doc@...r.kernel.org,
        linux-mm@...ck.org, kernel-team@...com, pjt@...gle.com,
        luto@...capital.net, efault@....de
Subject: Re: [RFC PATCH v2 11/17] cgroup: Implement new thread mode semantics

On 05/22/2017 01:13 PM, Waiman Long wrote:
> On 05/19/2017 04:26 PM, Tejun Heo wrote:
>>> @@ -2982,22 +3010,48 @@ static int cgroup_enable_threaded(struct cgroup *cgrp)
>>>  	LIST_HEAD(csets);
>>>  	struct cgrp_cset_link *link;
>>>  	struct css_set *cset, *cset_next;
>>> +	struct cgroup *child;
>>>  	int ret;
>>> +	u16 ss_mask;
>>>  
>>>  	lockdep_assert_held(&cgroup_mutex);
>>>  
>>>  	/* noop if already threaded */
>>> -	if (cgrp->proc_cgrp)
>>> +	if (cgroup_is_threaded(cgrp))
>>>  		return 0;
>>>  
>>> -	/* allow only if there are neither children or enabled controllers */
>>> -	if (css_has_online_children(&cgrp->self) || cgrp->subtree_control)
>>> +	/*
>>> +	 * Allow only if it is not the root and there are:
>>> +	 * 1) no children,
>>> +	 * 2) no non-threaded controllers are enabled, and
>>> +	 * 3) no attached tasks.
>>> +	 *
>>> +	 * With no attached tasks, it is assumed that no css_sets will be
>>> +	 * linked to the current cgroup. This may not be true if some dead
>>> +	 * css_sets linger around due to task_struct leakage, for example.
>>> +	 */
>> It doesn't look like the code is actually making this (incorrect)
>> assumption.  I suppose the comment is from before
>> cgroup_is_populated() was added?
> Yes, it is a bug. I should have checked the tasks_count instead of using
> cgroup_is_populated. Thanks for catching that.

Sorry, I would like to take it back. I think cgroup_is_populated() will
be set if there is any task attached to the cgroup. So I think it is
doing the right thing with regard to (3).

Cheers,
Longman