Why does devices cgroup check for CAP_SYS_ADMIN explicitly?

[Tejun Heo]

Hey, Eric.

On Tue, Nov 06, 2012 at 07:34:07AM -0800, Eric W. Biederman wrote:
> Having thought about this a little more I can give a definitive answer.
> 
> Adding a process to the device control group is equivalent to calling
> mknod, as it allows that process to open device nodes, or equivalently
> not open device nodes.  Therefore a capable check is absolutely
> required.
> 
> Without a capability check it would be possible to remove access to
> /dev/console for a suid root application keeping it from reporting
> attempts to hack it for example.

You understand that the whole thing is gated by VFS permission check,
right?  I'm kinda lost what you're talking about.

> The generic cgroup check in attach_task_by_pid to see if you can move
> another process into a cgroup needs to be a capability call and not a
> test for uid == 0.
> 
> static int attach_task_by_pid(struct cgroup *cgrp, u64 pid, bool threadgroup)
> {
> 	if (pid) {
> 		tsk = find_task_by_vpid(pid);
> 
> 		/*
> 		 * even if we're attaching all tasks in the thread group, we
> 		 * only need to check permissions on one of them.
> 		 */
> 		tcred = __task_cred(tsk);
> 		if (!uid_eq(cred->euid, GLOBAL_ROOT_UID) &&
>                                         ^^^^^^^^^^^^^^^
> 		    !uid_eq(cred->euid, tcred->uid) &&
> 		    !uid_eq(cred->euid, tcred->suid)) {
> 			rcu_read_unlock();
> 			ret = -EACCES;
> 			goto out_unlock_cgroup;

This one isn't gated by VFS so we need to add CAP check to this
function.  No?

Thanks.

-- 
tejun