On Tue, Sep 20, 2016 at 06:53:35PM +0200, Mickaël Salaün wrote: > > On 20/09/2016 02:30, Alexei Starovoitov wrote: > > On Tue, Sep 20, 2016 at 12:49:13AM +0200, Mickaël Salaün wrote: > >> 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 -- 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