Hi Tejun,
On Mon, Apr 14, 2014 at 02:03:23PM -0400, Tejun Heo wrote:
> On Mon, Apr 14, 2014 at 10:47:54AM -0400, Aristeu Rozanski wrote:
> > may_access() is currently used to validate both new exceptions from children
> > groups and to check if an access is allowed. This not only makes it hard to
> > understand and maintain, but it's also incorrect.
> >
> > It currently allows one to:
> >
> > # mkdir new_group
> > # cd new_group
> > # echo $$ >tasks
> > # echo "c 1:3 w" >devices.deny
> > # echo >/dev/null
> > # echo $?
> > 0
>
> It'd be nice to explain how this is broken and why the code paths
> can't be shared.
ok
> > This patch implements the device file access check separately and fixes
> > the issue.
> >
> > This is broken since c39a2a3018f8065cb5ea38b0314c1bbedb2cfa0d
>
> Please use 12-digits-of-SHA1 ("Subject of the patch") format.
ok
> > After review, this should be considered for stable series.
> >
> > Signed-off-by: Aristeu Rozanski <aris@xxxxxxxxxx>
>
> Can you please cc stable on the next posting?
ok
> > - rc = may_access(dev_cgroup, &ex, dev_cgroup->behavior);
> > + behavior = dev_cgroup->behavior;
> > + list_for_each_entry_rcu(ex, &dev_cgroup->exceptions, list) {
> > + if (type == DEV_BLOCK && !(ex->type & DEV_BLOCK))
> > + continue;
> > + if (type == DEV_CHAR && !(ex->type & DEV_CHAR))
> > + continue;
> > + if (ex->major != ~0 && major != ex->major)
> > + continue;
> > + if (ex->minor != ~0 && minor != ex->minor)
> > + continue;
> > + if (access & (~ex->access))
> > + continue;
> > + match = true;
> > + break;
>
> Can't we at least factor out the above part and share it between the
> two functions?
ok
> Currently, there's a lot of duplication in rather
> delicate code and it's not clear where they differ and why.
Are you referring to the duplication in this patch or other areas too?
--
Aristeu
--
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
