Messages in this thread | | | Date | Thu, 6 Oct 2016 15:43:51 -0700 | From | Dmitry Torokhov <> | Subject | Re: [PATCH 2/2] cgroup: Add a allow_attach policy for Android |
| |
On Wed, Oct 05, 2016 at 12:18:17PM -0700, John Stultz wrote: > On Wed, Oct 5, 2016 at 12:10 PM, Dmitry Torokhov > <dmitry.torokhov@gmail.com> wrote: > > On Mon, Oct 03, 2016 at 09:41:30PM -0700, John Stultz wrote: > >> +#ifdef CONFIG_CGROUP_NICE_ATTACH > >> +int cgroup_nice_allow_attach(struct cgroup_taskset *tset) > >> +{ > >> + const struct cred *cred = current_cred(), *tcred; > >> + struct task_struct *task; > >> + struct cgroup_subsys_state *css; > >> + > >> + if (capable(CAP_SYS_NICE)) > >> + return 0; > >> + > >> + cgroup_taskset_for_each(task, css, tset) { > >> + tcred = __task_cred(task); > > > > __task_cred() requires RCU lock (courtesy Ricky Z). > > Again, hopefully this isn't an issue with the new approach, but for > the short term I'll see if we can get this fixed in the android tree. >
Actually, it should all be simply removed from there right away, as this ends up being basically noop (but with all the locking violations and races):
cgroup_taskset_for_each() needs tasks to be placed on cset->mg_tasks list, but nobody does this in the ->allow_access() code path, so this loops always executes exactly 0 times and the whole thing is exactly equivalent of doing
return capable(CAP_SYS_NICE) ? 0 : -EACESS;
which can be done right in cgroup_procs_write_permission().
Thanks.
-- Dmitry
| |