On Tue, Sep 20, 2016 at 06:53:35PM +0200, Mickaël Salaün wrote:

On 20/09/2016 02:30, Alexei Starovoitov wrote:

quoted

On Tue, Sep 20, 2016 at 12:49:13AM +0200, Mickaël Salaün wrote:

quoted

Add security access check for cgroup backed FD. The "cgroup.procs" file
of the corresponding cgroup should be readable to identify the cgroup,
and writable to prove that the current process can manage this cgroup
(e.g. through delegation). This is similar to the check done by
cgroup_procs_write_permission().

Fixes: 4ed8ec521ed5 ("cgroup: bpf: Add BPF_MAP_TYPE_CGROUP_ARRAY")

I don't understand what 'fixes' is about.
Looks like new feature or tightening?
Since cgroup was opened by the process and it got an fd,
it had an access, so extra check here looks unnecessary.

It may not be a "fix", but this patch tighten the access control. The
current cgroup_get_from_fd() only rely on the access check done on the
passed FD. However, this FD come from a cgroup directory, not a
"cgroup.procs" (in this directory). The "cgroup.procs" is used for
cgroup delegation by cgroup_procs_write_permission(). Checking
"cgroup.procs" is then more consistent with access checks done by other
part of the cgroup code. Being able to open a cgroup directory only
means that the current process is able to list the cgroup hierarchy, not
necessarily to list the tasks in this cgroups.

Currently, bpf's access control and cgroup's are completely separate
and intentionally so.  I don't see why this matters given the current
model.

Thanks.

-- 
tejun