From: John Stultz <john.stultz@linaro.org> To: lkml <linux-kernel@vger.kernel.org> Cc: John Stultz <john.stultz@linaro.org>, Tejun Heo <tj@kernel.org>, Li Zefan <lizefan@huawei.com>, Jonathan Corbet <corbet@lwn.net>, cgroups@vger.kernel.org, Android Kernel Team <kernel-team@android.com>, Rom Lemarchand <romlem@android.com>, Colin Cross <ccross@android.com>, Dmitry Shmidt <dimitrysh@google.com>, Todd Kjos <tkjos@google.com>, Christian Poetzsch <christian.potzsch@imgtec.com>, Amit Pundir <amit.pundir@linaro.org>, Dmitry Torokhov <dmitry.torokhov@gmail.com>, Kees Cook <keescook@chromium.org>, "Serge E . Hallyn" <serge@hallyn.com>, linux-api@vger.kernel.org Subject: [RFC][PATCH v3] cgroup: Use CAP_SYS_RESOURCE to allow a process to migrate other tasks between cgroups Date: Thu, 20 Oct 2016 18:24:39 -0700 [thread overview] Message-ID: <1477013079-23692-1-git-send-email-john.stultz@linaro.org> (raw) This patch adds logic to allows a process to migrate other tasks between cgroups if they have CAP_SYS_RESOURCE. In Android (where this feature originated), the ActivityManager tracks various application states (TOP_APP, FOREGROUND, BACKGROUND, SYSTEM, etc), and then as applications change states, the SchedPolicy logic will migrate the application tasks between different cgroups used to control the different application states (for example, there is a background cpuset cgroup which can limit background tasks to stay on one low-power cpu, and the bg_non_interactive cpuctrl cgroup can then further limit those background tasks to a small percentage of that one cpu's cpu time). However, for security reasons, Android doesn't want to make the system_server (the process that runs the ActivityManager and SchedPolicy logic), run as root. So in the Android common.git kernel, they have some logic to allow cgroups to loosen their permissions so CAP_SYS_NICE tasks can migrate other tasks between cgroups. I feel the approach taken there overloads CAP_SYS_NICE a bit much, and is maybe more complicated then needed. So this patch, as suggested by Michael Kerrisk, simply adds a check for CAP_SYS_RESOURCE. I've tested this with AOSP master, and this seems to work well as Zygote and system_server already use CAP_SYS_RESOURCE. I've also submitted patches against the android-4.4 kernel to change it to use CAP_SYS_RESOURCE, and the Android developers have seemed ok with this change. Thouhts and feedback would be appreciated! Cc: Tejun Heo <tj@kernel.org> Cc: Li Zefan <lizefan@huawei.com> Cc: Jonathan Corbet <corbet@lwn.net> Cc: cgroups@vger.kernel.org Cc: Android Kernel Team <kernel-team@android.com> Cc: Rom Lemarchand <romlem@android.com> Cc: Colin Cross <ccross@android.com> Cc: Dmitry Shmidt <dimitrysh@google.com> Cc: Todd Kjos <tkjos@google.com> Cc: Christian Poetzsch <christian.potzsch@imgtec.com> Cc: Amit Pundir <amit.pundir@linaro.org> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com> Cc: Kees Cook <keescook@chromium.org> Cc: Serge E. Hallyn <serge@hallyn.com> Cc: linux-api@vger.kernel.org Signed-off-by: John Stultz <john.stultz@linaro.org> --- v2: Renamed to just CAP_CGROUP_MIGRATE as reccomended by Tejun v3: Switched to just using CAP_SYS_RESOURCE as suggested by Michael --- kernel/cgroup.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kernel/cgroup.c b/kernel/cgroup.c index 09f84d2..866059a 100644 --- a/kernel/cgroup.c +++ b/kernel/cgroup.c @@ -2857,7 +2857,7 @@ static int cgroup_procs_write_permission(struct task_struct *task, if (!uid_eq(cred->euid, GLOBAL_ROOT_UID) && !uid_eq(cred->euid, tcred->uid) && !uid_eq(cred->euid, tcred->suid) && - !ns_capable(tcred->user_ns, CAP_CGROUP_MIGRATE)) + !ns_capable(tcred->user_ns, CAP_SYS_RESOURCE)) ret = -EACCES; if (!ret && cgroup_on_dfl(dst_cgrp)) { -- 2.7.4
WARNING: multiple messages have this Message-ID (diff)
From: John Stultz <john.stultz-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> To: lkml <linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org> Cc: John Stultz <john.stultz-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>, Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>, Li Zefan <lizefan-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>, Jonathan Corbet <corbet-T1hC0tSOHrs@public.gmane.org>, cgroups-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Android Kernel Team <kernel-team-z5hGa2qSFaRBDgjK7y7TUQ@public.gmane.org>, Rom Lemarchand <romlem-z5hGa2qSFaRBDgjK7y7TUQ@public.gmane.org>, Colin Cross <ccross-z5hGa2qSFaRBDgjK7y7TUQ@public.gmane.org>, Dmitry Shmidt <dimitrysh-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>, Todd Kjos <tkjos-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>, Christian Poetzsch <christian.potzsch-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>, Amit Pundir <amit.pundir-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>, Dmitry Torokhov <dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>, Kees Cook <keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>, "Serge E . Hallyn" <serge-A9i7LUbDfNHQT0dZR+AlfA@public.gmane.org>, linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org Subject: [RFC][PATCH v3] cgroup: Use CAP_SYS_RESOURCE to allow a process to migrate other tasks between cgroups Date: Thu, 20 Oct 2016 18:24:39 -0700 [thread overview] Message-ID: <1477013079-23692-1-git-send-email-john.stultz@linaro.org> (raw) This patch adds logic to allows a process to migrate other tasks between cgroups if they have CAP_SYS_RESOURCE. In Android (where this feature originated), the ActivityManager tracks various application states (TOP_APP, FOREGROUND, BACKGROUND, SYSTEM, etc), and then as applications change states, the SchedPolicy logic will migrate the application tasks between different cgroups used to control the different application states (for example, there is a background cpuset cgroup which can limit background tasks to stay on one low-power cpu, and the bg_non_interactive cpuctrl cgroup can then further limit those background tasks to a small percentage of that one cpu's cpu time). However, for security reasons, Android doesn't want to make the system_server (the process that runs the ActivityManager and SchedPolicy logic), run as root. So in the Android common.git kernel, they have some logic to allow cgroups to loosen their permissions so CAP_SYS_NICE tasks can migrate other tasks between cgroups. I feel the approach taken there overloads CAP_SYS_NICE a bit much, and is maybe more complicated then needed. So this patch, as suggested by Michael Kerrisk, simply adds a check for CAP_SYS_RESOURCE. I've tested this with AOSP master, and this seems to work well as Zygote and system_server already use CAP_SYS_RESOURCE. I've also submitted patches against the android-4.4 kernel to change it to use CAP_SYS_RESOURCE, and the Android developers have seemed ok with this change. Thouhts and feedback would be appreciated! Cc: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> Cc: Li Zefan <lizefan-hv44wF8Li93QT0dZR+AlfA@public.gmane.org> Cc: Jonathan Corbet <corbet-T1hC0tSOHrs@public.gmane.org> Cc: cgroups-u79uwXL29TY76Z2rM5mHXA@public.gmane.org Cc: Android Kernel Team <kernel-team-z5hGa2qSFaRBDgjK7y7TUQ@public.gmane.org> Cc: Rom Lemarchand <romlem-z5hGa2qSFaRBDgjK7y7TUQ@public.gmane.org> Cc: Colin Cross <ccross-z5hGa2qSFaRBDgjK7y7TUQ@public.gmane.org> Cc: Dmitry Shmidt <dimitrysh-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> Cc: Todd Kjos <tkjos-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> Cc: Christian Poetzsch <christian.potzsch-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org> Cc: Amit Pundir <amit.pundir-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> Cc: Dmitry Torokhov <dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> Cc: Kees Cook <keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> Cc: Serge E. Hallyn <serge-A9i7LUbDfNHQT0dZR+AlfA@public.gmane.org> Cc: linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org Signed-off-by: John Stultz <john.stultz-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> --- v2: Renamed to just CAP_CGROUP_MIGRATE as reccomended by Tejun v3: Switched to just using CAP_SYS_RESOURCE as suggested by Michael --- kernel/cgroup.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kernel/cgroup.c b/kernel/cgroup.c index 09f84d2..866059a 100644 --- a/kernel/cgroup.c +++ b/kernel/cgroup.c @@ -2857,7 +2857,7 @@ static int cgroup_procs_write_permission(struct task_struct *task, if (!uid_eq(cred->euid, GLOBAL_ROOT_UID) && !uid_eq(cred->euid, tcred->uid) && !uid_eq(cred->euid, tcred->suid) && - !ns_capable(tcred->user_ns, CAP_CGROUP_MIGRATE)) + !ns_capable(tcred->user_ns, CAP_SYS_RESOURCE)) ret = -EACCES; if (!ret && cgroup_on_dfl(dst_cgrp)) { -- 2.7.4
next reply other threads:[~2016-10-21 1:24 UTC|newest] Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top 2016-10-21 1:24 John Stultz [this message] 2016-10-21 1:24 ` [RFC][PATCH v3] cgroup: Use CAP_SYS_RESOURCE to allow a process to migrate other tasks between cgroups John Stultz 2016-10-21 6:42 ` Michael Kerrisk (man-pages) 2016-10-21 6:42 ` Michael Kerrisk (man-pages) 2016-10-21 18:06 ` John Stultz 2016-10-21 18:06 ` John Stultz 2016-10-21 13:41 ` Serge E. Hallyn 2016-10-21 13:41 ` Serge E. Hallyn
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=1477013079-23692-1-git-send-email-john.stultz@linaro.org \ --to=john.stultz@linaro.org \ --cc=amit.pundir@linaro.org \ --cc=ccross@android.com \ --cc=cgroups@vger.kernel.org \ --cc=christian.potzsch@imgtec.com \ --cc=corbet@lwn.net \ --cc=dimitrysh@google.com \ --cc=dmitry.torokhov@gmail.com \ --cc=keescook@chromium.org \ --cc=kernel-team@android.com \ --cc=linux-api@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=lizefan@huawei.com \ --cc=romlem@android.com \ --cc=serge@hallyn.com \ --cc=tj@kernel.org \ --cc=tkjos@google.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.