On Thu, Oct 06, 2016 at 03:43:51PM -0700, Dmitry Torokhov wrote:
> 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@xxxxxxxxx> 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().
Umm, sorry, no, it actually always returns 0, regardless even of
capabilities. Permissions are indeed being relaxed ;)
--
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe cgroups" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
