From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tejun Heo Subject: Re: Why does devices cgroup check for CAP_SYS_ADMIN explicitly? Date: Tue, 6 Nov 2012 07:43:20 -0800 Message-ID: <20121106154320.GE30069@mtj.dyndns.org> References: <20121106023845.GI19354@mtj.dyndns.org> <877gpzrlir.fsf@xmission.com> <20121106150131.GA14640@sergelap> <20121106150639.GB30069@mtj.dyndns.org> <871ug6rbio.fsf@xmission.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=sender:date:from:to:cc:subject:message-id:references:mime-version :content-type:content-disposition:in-reply-to:user-agent; bh=/Uujj6cwiKPWhRHDHUn4fwrzMkCBfxs5sqf7ITIxCKw=; b=WO5LvmcvEs+wGuMoPVWg7qw3x+RZe4Mj3G4o7hTBoJSl6ETUM2jlPuRc+NsVReEZL5 cZNhwLP043u7oGSVdoyk9Q9LX/fRBDvwdshCSCMhcsMCJ8LRjDF7+ZCU8UoNWtAgxf70 zJ5IS5jXoIWlY9xpRn1iSrUz6SfSQLqOcE2L57C56fAkE8XCO+juFDS8uqL/w/EiTjqg DqGrElHZJDfEq67Qr19bhQu5W+pZhQzGhj9mTA2SLdyKyM2aBZn/hAXyPJ2/NAr8fyf3 cHfQQQguixJQO0lpuYeb45tj/gX9nf1RK0dMrLIWEKQwae3gJ9f7ICd94+nN7zFPpRag RiKA== Content-Disposition: inline In-Reply-To: <871ug6rbio.fsf-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org> List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: containers-bounces-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org Errors-To: containers-bounces-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org To: "Eric W. Biederman" Cc: containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org, Aristeu Rozanski , cgroups-u79uwXL29TY76Z2rM5mHXA@public.gmane.org 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