Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934451AbcLMQjp (ORCPT ); Tue, 13 Dec 2016 11:39:45 -0500 Received: from nm29-vm4.bullet.mail.ne1.yahoo.com ([98.138.91.189]:34010 "EHLO nm29-vm4.bullet.mail.ne1.yahoo.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S934372AbcLMQja (ORCPT ); Tue, 13 Dec 2016 11:39:30 -0500 X-Yahoo-Newman-Id: 741910.82124.bm@smtp213.mail.ne1.yahoo.com X-Yahoo-Newman-Property: ymail-3 X-YMail-OSG: 7vQQ9ZQVM1lK2eKuQDGVVb6hI3BFmpNXD2YTtGlQpHX2oDI _NSQiNF1NQ0_3SXOBkXEGBbd50BO8y6TIP0vCD7Xgkfzif4LRZifQteWbuG8 ZNK8cb9AFIts3TEzKIv2SbR50WM4Qdq4vFRsh8Oiv7fL._pDT1nMe6Lz8Dt_ t2rbbocm.v.4k7lt4kCpmpkC4ejHaIFMRiXpxSe41Oc2ICWwSoiJEoqahKGa 7TS5Sc9xGu.gJmFshPhzCcRubIHvsyCkb3TpIuv7VHL3bHL7HtqZfkifWYc. O8W57nKACKHtjpCus1sRqbggSVVrExRoV9zmwqD_HptC4Ap.ex5nKDfMA5Mr xFgNOt8GrmdtOxjkBL3oVNLOSMY_W1eL5eNseXbqgG1yIW2thcurn6aSfYPi Dwe7I9xyFczvK0lBeSwnaAv9Dy0BSVesGBh.AXe.1UJCP0UVB2Z3A.0ld.rf 7dF6dNhqS4MBslMX_Xkie6PhIli8bwIbiWo0UIJHrDRvpzWKNBjWknbB3FDs jkciW1ztfcq23u6B0bv6x5XKuHRlV51oooriYIPKm1iMmtuFhpg-- X-Yahoo-SMTP: OIJXglSswBDfgLtXluJ6wiAYv6_cnw-- Subject: Re: [PATCH v5] cgroup: Add new capability to allow a process to migrate other tasks between cgroups To: mtk.manpages@gmail.com, John Stultz References: <1481593143-18756-1-git-send-email-john.stultz@linaro.org> Cc: lkml , Tejun Heo , Li Zefan , Jonathan Corbet , "open list:CONTROL GROUP (CGROUP)" , Android Kernel Team , Rom Lemarchand , Colin Cross , Dmitry Shmidt , Todd Kjos , Christian Poetzsch , Amit Pundir , Dmitry Torokhov , Kees Cook , "Serge E . Hallyn" , Andy Lutomirski , Linux API From: Casey Schaufler Message-ID: <221e80bd-3d99-6c35-dcd3-b2547f0abb11@schaufler-ca.com> Date: Tue, 13 Dec 2016 08:39:21 -0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.5.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5461 Lines: 133 On 12/13/2016 1:47 AM, Michael Kerrisk (man-pages) wrote: > Hi John, > > On 13 December 2016 at 02:39, John Stultz wrote: >> This patch adds CAP_GROUP_MIGRATE and logic to allows a process > s/CAP_GROUP_MIGRATE/CAP_CGROUP_MIGRATE/ > >> to migrate other tasks between cgroups. >> >> 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 >> for non-android environments. Efforts to re-use CAP_SYS_RESOURCE >> for this purpose (which Android has since adopted) was also >> stymied by concerns about risks from future cgroups that could be >> considered "dangerous" by how they might change system semantics. >> >> So to avoid overlapping usage, this patch adds a brand new >> process capability flag (CAP_CGROUP_MIGRATE), and uses it when >> checking if a task can migrate other tasks between cgroups. >> >> I've tested this with AOSP master (though its a bit hacked in as >> I still need to properly get the selinux bits aware of the new >> capability bit) with selinux set to permissive and it seems to be >> working well. >> >> Thoughts and feedback would be appreciated! > So, back to the discussion of silos. I understand the argument for > wanting a new silo. But, in that case can we at least try not to make > it a single-use silo? > > How about CAP_CGROUP_CONTROL or some such, with the idea that this > might be a capability that allows the holder to step outside usual > cgroup rules? At the moment, that capability would allow only one such > step, but maybe there would be others in the future. I agree, but want to put it more strongly. The granularity of capabilities can never be fine enough for some people, and this is an example of a case where you're going a bit too far. If the use case is Android as you say, you don't need this. As my friends on the far side of the aisle would say, "just write SELinux policy" to correctly control access as required. > > Cheers, > > Michael > > >> Cc: Tejun Heo >> Cc: Li Zefan >> Cc: Jonathan Corbet >> Cc: cgroups@vger.kernel.org >> Cc: Android Kernel Team >> Cc: Rom Lemarchand >> Cc: Colin Cross >> Cc: Dmitry Shmidt >> Cc: Todd Kjos >> Cc: Christian Poetzsch >> Cc: Amit Pundir >> Cc: Dmitry Torokhov >> Cc: Kees Cook >> Cc: Serge E. Hallyn >> Cc: Andy Lutomirski >> Cc: linux-api@vger.kernel.org >> Acked-by: Serge Hallyn >> Signed-off-by: John Stultz >> --- >> v2: Renamed to just CAP_CGROUP_MIGRATE as reccomended by Tejun >> v3: Switched to just using CAP_SYS_RESOURCE as suggested by Michael >> v4: Send out properly folded down version of the patch. :P >> v5: Switch back to CAP_CGROUP_MIGRATE due to concerns from Andy >> --- >> include/uapi/linux/capability.h | 5 ++++- >> kernel/cgroup.c | 3 ++- >> 2 files changed, 6 insertions(+), 2 deletions(-) >> >> diff --git a/include/uapi/linux/capability.h b/include/uapi/linux/capability.h >> index 49bc062..32d3829 100644 >> --- a/include/uapi/linux/capability.h >> +++ b/include/uapi/linux/capability.h >> @@ -349,8 +349,11 @@ struct vfs_cap_data { >> >> #define CAP_AUDIT_READ 37 >> >> +/* Allow migration of other tasks between cgroups */ >> >> -#define CAP_LAST_CAP CAP_AUDIT_READ >> +#define CAP_CGROUP_MIGRATE 38 >> + >> +#define CAP_LAST_CAP CAP_CGROUP_MIGRATE >> >> #define cap_valid(x) ((x) >= 0 && (x) <= CAP_LAST_CAP) >> >> diff --git a/kernel/cgroup.c b/kernel/cgroup.c >> index 2ee9ec3..784f115 100644 >> --- a/kernel/cgroup.c >> +++ b/kernel/cgroup.c >> @@ -2856,7 +2856,8 @@ 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)) >> + !uid_eq(cred->euid, tcred->suid) && >> + !ns_capable(tcred->user_ns, CAP_CGROUP_MIGRATE)) >> ret = -EACCES; >> >> if (!ret && cgroup_on_dfl(dst_cgrp)) { >> -- >> 2.7.4 >> >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-api" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html > >