From: Tejun Heo <tj@kernel.org> To: lizefan@huawei.com, hannes@cmpxchg.org Cc: cgroups@vger.kernel.org, linux-kernel@vger.kernel.org, kernel-team@fb.com, Tejun Heo <tj@kernel.org> Subject: [PATCH 2/4] cgroup: separate out cgroup_procs_write_permission() from __cgroup_procs_write() Date: Tue, 16 Jun 2015 15:10:15 -0400 [thread overview] Message-ID: <1434481817-32001-3-git-send-email-tj@kernel.org> (raw) In-Reply-To: <1434481817-32001-1-git-send-email-tj@kernel.org> Separate out task / process migration permission check from __cgroup_procs_write() into cgroup_procs_write_permission(). * Permission check is moved right above the actual migration and no longer performed while holding rcu_read_lock(). cgroup_procs_write_permission() uses get_task_cred() / put_cred() instead of __task_cred(). Also, !root trying to migrate kthreadd or PF_NO_SETAFFINITY tasks will now fail with -EINVAL rather than -EACCES which should be fine. * The same permission check is now performed even when moving self by specifying 0 as pid. This always succeeds so there's no functional difference. We'll add more permission checks later and the benefits of keeping both cases consistent outweigh the minute overhead of doing perm checks on pid 0 case. Signed-off-by: Tejun Heo <tj@kernel.org> --- kernel/cgroup.c | 38 ++++++++++++++++++++++++-------------- 1 file changed, 24 insertions(+), 14 deletions(-) diff --git a/kernel/cgroup.c b/kernel/cgroup.c index 12b580f..4504d64 100644 --- a/kernel/cgroup.c +++ b/kernel/cgroup.c @@ -2392,6 +2392,25 @@ static int cgroup_attach_task(struct cgroup *dst_cgrp, return ret; } +static int cgroup_procs_write_permission(struct task_struct *task) +{ + const struct cred *cred = current_cred(); + const struct cred *tcred = get_task_cred(task); + int ret = 0; + + /* + * even if we're attaching all tasks in the thread group, we only + * need to check permissions on one of them. + */ + if (!uid_eq(cred->euid, GLOBAL_ROOT_UID) && + !uid_eq(cred->euid, tcred->uid) && + !uid_eq(cred->euid, tcred->suid)) + ret = -EACCES; + + put_cred(tcred); + return ret; +} + /* * Find the task_struct of the task to attach by vpid and pass it along to the * function to attach either it or all tasks in its threadgroup. Will lock @@ -2401,7 +2420,6 @@ static ssize_t __cgroup_procs_write(struct kernfs_open_file *of, char *buf, size_t nbytes, loff_t off, bool threadgroup) { struct task_struct *tsk; - const struct cred *cred = current_cred(), *tcred; struct cgroup *cgrp; pid_t pid; int ret; @@ -2421,19 +2439,9 @@ static ssize_t __cgroup_procs_write(struct kernfs_open_file *of, char *buf, ret = -ESRCH; goto out_unlock_rcu; } - /* - * 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)) { - ret = -EACCES; - goto out_unlock_rcu; - } - } else + } else { tsk = current; + } if (threadgroup) tsk = tsk->group_leader; @@ -2451,7 +2459,9 @@ static ssize_t __cgroup_procs_write(struct kernfs_open_file *of, char *buf, get_task_struct(tsk); rcu_read_unlock(); - ret = cgroup_attach_task(cgrp, tsk, threadgroup); + ret = cgroup_procs_write_permission(tsk); + if (!ret) + ret = cgroup_attach_task(cgrp, tsk, threadgroup); put_task_struct(tsk); goto out_unlock_threadgroup; -- 2.4.3
WARNING: multiple messages have this Message-ID (diff)
From: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> To: lizefan-hv44wF8Li93QT0dZR+AlfA@public.gmane.org, hannes-druUgvl0LCNAfugRpC6u6w@public.gmane.org Cc: cgroups-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, kernel-team-b10kYP2dOMg@public.gmane.org, Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> Subject: [PATCH 2/4] cgroup: separate out cgroup_procs_write_permission() from __cgroup_procs_write() Date: Tue, 16 Jun 2015 15:10:15 -0400 [thread overview] Message-ID: <1434481817-32001-3-git-send-email-tj@kernel.org> (raw) In-Reply-To: <1434481817-32001-1-git-send-email-tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> Separate out task / process migration permission check from __cgroup_procs_write() into cgroup_procs_write_permission(). * Permission check is moved right above the actual migration and no longer performed while holding rcu_read_lock(). cgroup_procs_write_permission() uses get_task_cred() / put_cred() instead of __task_cred(). Also, !root trying to migrate kthreadd or PF_NO_SETAFFINITY tasks will now fail with -EINVAL rather than -EACCES which should be fine. * The same permission check is now performed even when moving self by specifying 0 as pid. This always succeeds so there's no functional difference. We'll add more permission checks later and the benefits of keeping both cases consistent outweigh the minute overhead of doing perm checks on pid 0 case. Signed-off-by: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> --- kernel/cgroup.c | 38 ++++++++++++++++++++++++-------------- 1 file changed, 24 insertions(+), 14 deletions(-) diff --git a/kernel/cgroup.c b/kernel/cgroup.c index 12b580f..4504d64 100644 --- a/kernel/cgroup.c +++ b/kernel/cgroup.c @@ -2392,6 +2392,25 @@ static int cgroup_attach_task(struct cgroup *dst_cgrp, return ret; } +static int cgroup_procs_write_permission(struct task_struct *task) +{ + const struct cred *cred = current_cred(); + const struct cred *tcred = get_task_cred(task); + int ret = 0; + + /* + * even if we're attaching all tasks in the thread group, we only + * need to check permissions on one of them. + */ + if (!uid_eq(cred->euid, GLOBAL_ROOT_UID) && + !uid_eq(cred->euid, tcred->uid) && + !uid_eq(cred->euid, tcred->suid)) + ret = -EACCES; + + put_cred(tcred); + return ret; +} + /* * Find the task_struct of the task to attach by vpid and pass it along to the * function to attach either it or all tasks in its threadgroup. Will lock @@ -2401,7 +2420,6 @@ static ssize_t __cgroup_procs_write(struct kernfs_open_file *of, char *buf, size_t nbytes, loff_t off, bool threadgroup) { struct task_struct *tsk; - const struct cred *cred = current_cred(), *tcred; struct cgroup *cgrp; pid_t pid; int ret; @@ -2421,19 +2439,9 @@ static ssize_t __cgroup_procs_write(struct kernfs_open_file *of, char *buf, ret = -ESRCH; goto out_unlock_rcu; } - /* - * 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)) { - ret = -EACCES; - goto out_unlock_rcu; - } - } else + } else { tsk = current; + } if (threadgroup) tsk = tsk->group_leader; @@ -2451,7 +2459,9 @@ static ssize_t __cgroup_procs_write(struct kernfs_open_file *of, char *buf, get_task_struct(tsk); rcu_read_unlock(); - ret = cgroup_attach_task(cgrp, tsk, threadgroup); + ret = cgroup_procs_write_permission(tsk); + if (!ret) + ret = cgroup_attach_task(cgrp, tsk, threadgroup); put_task_struct(tsk); goto out_unlock_threadgroup; -- 2.4.3
next prev parent reply other threads:[~2015-06-16 19:18 UTC|newest] Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top 2015-06-16 19:10 [PATCHSET cgroup/for-4.2] cgroup: require write perm on common ancestor for migration Tejun Heo 2015-06-16 19:10 ` Tejun Heo 2015-06-16 19:10 ` [PATCH 1/4] kernfs: make kernfs_get_inode() public Tejun Heo 2015-06-16 19:10 ` Tejun Heo 2015-06-16 20:58 ` Greg Kroah-Hartman 2015-06-16 20:58 ` Greg Kroah-Hartman 2015-06-16 19:10 ` Tejun Heo [this message] 2015-06-16 19:10 ` [PATCH 2/4] cgroup: separate out cgroup_procs_write_permission() from __cgroup_procs_write() Tejun Heo 2015-06-16 19:10 ` [PATCH 3/4] cgroup: require write perm on common ancestor when moving processes on the default hierarchy Tejun Heo 2015-06-16 19:10 ` Tejun Heo 2015-06-18 3:14 ` Zefan Li 2015-06-18 3:14 ` Zefan Li 2015-06-18 17:59 ` Tejun Heo 2015-06-18 17:59 ` [PATCH v2 " Tejun Heo 2015-06-18 19:04 ` Johannes Weiner 2015-06-18 19:04 ` Johannes Weiner 2015-06-16 19:10 ` [PATCH 4/4] cgroup: add delegation section to unified hierarchy documentation Tejun Heo 2015-06-16 19:10 ` Tejun Heo 2015-06-18 19:01 ` Johannes Weiner 2015-06-18 19:01 ` Johannes Weiner 2015-06-18 20:23 ` [PATCH v2 " Tejun Heo 2015-06-18 20:23 ` Tejun Heo 2015-06-18 20:46 ` Johannes Weiner 2015-06-18 20:46 ` Johannes Weiner 2015-06-18 20:55 ` [PATCHSET cgroup/for-4.2] cgroup: require write perm on common ancestor for migration Tejun Heo 2015-06-18 20:55 ` Tejun Heo
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=1434481817-32001-3-git-send-email-tj@kernel.org \ --to=tj@kernel.org \ --cc=cgroups@vger.kernel.org \ --cc=hannes@cmpxchg.org \ --cc=kernel-team@fb.com \ --cc=linux-kernel@vger.kernel.org \ --cc=lizefan@huawei.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.