IronPort-PHdr: =?us-ascii?q?9a23=3AYgM+bhSWroV4G2lpQGovpQRkztpsv+yvbD5Q0YIu?=
 =?us-ascii?q?jvd0So/mwa67ZROCt8tkgFKBZ4jH8fUM07OQ6PG9HzBeqs/Z6TgrS99lb1c9k8?=
 =?us-ascii?q?IYnggtUoauKHbQC7rUVRE8B9lIT1R//nu2YgB/Ecf6YEDO8DXptWZBUhrwOhBo?=
 =?us-ascii?q?KevrB4Xck9q41/yo+53Ufg5EmCexbal8IRiyrQjdrMYbjIptJqos1hfFv2ZDdv?=
 =?us-ascii?q?hLy29vOV+dhQv36N2q/J5k/SRQuvYh+NBFXK7nYak2TqFWASo/PWwt68LlqRfM?=
 =?us-ascii?q?TQ2U5nsBSWoWiQZHAxLE7B7hQJj8tDbxu/dn1ymbOc32Sq00WSin4qx2RhLklD?=
 =?us-ascii?q?sLOjgk+2zMlMd+kLxUrw6gpxxnwo7bfoeVNOZlfqjAed8WXHdNUtpNWyBEBI6z?=
 =?us-ascii?q?YZEPD+4cNuhGqYfzqUYFoR+nCQWyGO/jzzlFjWL006InyeQsCQLI0hEgEdwQvn?=
 =?us-ascii?q?rbrtv1NKAOXu6yw6bGwi7Ob+9V1Drn9ITFaAwtrPOKULltccTR004vFwbdg1uN?=
 =?us-ascii?q?tYzqISuV1uQTvGid8uFuSOevhHQjqwF1vDeuxtonh47Sho0I0VDJ7jl5wYYpKt?=
 =?us-ascii?q?24T053e9ikEIBKuC2AOIt2Rd0iTnhutS0nybMGoYa2cDUFxZko3RLSa+GLf5KW?=
 =?us-ascii?q?7h/sSuqdOyp0iXR4c7ylnRmy61KvyujkW8mx11ZFszRKn8HXtnAIyxzT8s+HSu?=
 =?us-ascii?q?Zh/ku52TaAyQTT6uZcLEAoj6XbMZ8hwqMrlpYJrUTCHjP5mEXxjKOMcEUr5vOo?=
 =?us-ascii?q?5Pj9brXjp5+cM5d4igD4MqswhsyyGfk0PwcBUmSB+emwyafv8VP2TblUlPE6j7?=
 =?us-ascii?q?HVsJXAKsQaoq65DRVV0oEm6xunFDepzc8YkGIbLFNFZB2Hj4/pN0vIIPDjF/iz?=
 =?us-ascii?q?mVuskDB1x/zeJL3uHo3NLmTfkLfmZbt96FBTyBA1zd9B45JYE60BL+zpVU/0r9?=
 =?us-ascii?q?HXFBk5PBGuw+bgCdVyy5kSVn6IAq+cKKnSq0OH5vozI+mQY48YoDL9K/kj5/7z?=
 =?us-ascii?q?gn41gFwdcrez3ZsRdn+4Gu9rI1uWYXXymNcNC2QKsRQkTOzsllKCVSRfZ3GoX6?=
 =?us-ascii?q?Iz/js7Ep6pDZ/fRoCxh7yMxD20HphLZmBcF1+DC2vneJ+fVvcWdi2dP89hnSYY?=
 =?us-ascii?q?VbS7V4Ah0hSuvhfgy7V7NurU5jEYtZX72dh3+eLTmx8y9SJvAsSS1GGNSG50nm?=
 =?us-ascii?q?cWSDMswK9/pkl9wE+Z0adkm/xYCcBT5/RRXwc4Mp7cz+p6B8rpWgLdY9eJTEqm?=
 =?us-ascii?q?Q9S9DDE1T9IxxcUBY1x6G9m4iRDDxSWqCacPl7OXHJw07r7c33/pKsZl0XnGya?=
 =?us-ascii?q?0hgkI+QsRVKG2mgrdz9w3UB47OiUWWibymergb3C7I7G2D13aBvFlEUA5sVqXI?=
 =?us-ascii?q?RXYfZk3Vrdni6UPCSLiuCbsjMgRf08KNNqxKatjxh1VcWPjjIMjeY362m2qoCh?=
 =?us-ascii?q?aI3K2DbIXxdmUexiXdD1ILkwAJ8XmaMgg+A3Tpn2WLIyZjGhrMQwu4/vNzp1u4?=
 =?us-ascii?q?VEg9z0eBaEg3hJSv/RtAvuCRU/Me2Po/vS4lrzhlVAKm08n+F8uLpw0ner5VJ9?=
 =?us-ascii?q?w6/gEUhiriqwVhM8n4fOhZjVkEflEy5hm22g=3D=3D?=
Message-ID: <1489082234.10847.56.camel@tycho.nsa.gov>
Subject: Re: [Regression?] 1ea0ce4069 ("selinux: allow changing labels for
 cgroupfs") stops Android from booting
From: Stephen Smalley <sds@tycho.nsa.gov>
To: Greg KH <greg@kroah.com>
Cc: Nick Kralevich <nnk@google.com>, Paul Moore <paul@paul-moore.com>,
        John Stultz <john.stultz@linaro.org>,
        Jeffrey Vander Stoep <jeffv@google.com>,
        Antonio Murdaca <amurdaca@redhat.com>,
        lkml <linux-kernel@vger.kernel.org>,
        Android Kernel Team <kernel-team@android.com>
Date: Thu, 09 Mar 2017 12:57:14 -0500
In-Reply-To: <20170309172815.GA25234@kroah.com>
References: <CALAqxLVzkW-8cFHxhO=mTiDW=5EAr3Gyzf+zczEwtdP4kPxX5g@mail.gmail.com>
         <CAHC9VhTjUuA1pdOQ4aHhLUiPHuyGDbZ19r_QUR-MyyTEgkO_nQ@mail.gmail.com>
         <1488224547.19819.32.camel@tycho.nsa.gov>
         <1488225201.19819.37.camel@tycho.nsa.gov>
         <CAFJ0LnHTmvBCaXKA1-j2ywHdASeHQzYS5bu0CHAzjSnXXK55fQ@mail.gmail.com>
         <1488230608.19819.51.camel@tycho.nsa.gov>
         <20170309172815.GA25234@kroah.com>
Organization: National Security Agency
Content-Type: text/plain; charset="UTF-8"
Mime-Version: 1.0
Content-Transfer-Encoding: 8bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 3645
Lines: 102

On Thu, 2017-03-09 at 18:28 +0100, Greg KH wrote:
> On Mon, Feb 27, 2017 at 04:23:28PM -0500, Stephen Smalley wrote:
> > 
> > On Mon, 2017-02-27 at 12:48 -0800, Nick Kralevich wrote:
> > > 
> > > On Mon, Feb 27, 2017 at 11:53 AM, Stephen Smalley <sds@tycho.nsa.
> > > gov>
> > > wrote:
> > > > 
> > > > 
> > > > > 
> > > > > 
> > > > > I can reproduce it on angler (with a back-port of just that
> > > > > patch),
> > > > > although I am unclear on the cause.  The patch is only
> > > > > supposed
> > > > > to
> > > > > enable explicit setting of security labels by userspace on
> > > > > cgroup
> > > > > files, so it isn't supposed to cause any breakage under
> > > > > existing
> > > > > policy.  Prior to the patch, the kernel would always just
> > > > > return
> > > > > -1
> > > > > with errno EOPNOTSUPP upon attempts to set security labels on
> > > > > cgroup
> > > > > files; with the patch, the kernel may instead return -1 with
> > > > > errno
> > > > > EACCES if not allowed.  So I suppose if userspace was
> > > > > explicitly
> > > > > testing for EOPNOTSUPP and not failing hard in that case, it
> > > > > might
> > > > > cause breakage.  Not sure why existing userspace would be
> > > > > trying
> > > > > to
> > > > > relabel cgroup files, unless it is just a recursive
> > > > > restorecon
> > > > > that
> > > > > happens to traverse into a cgroup mount (and in that case,
> > > > > not
> > > > > sure
> > > > > why
> > > > > it would be fatal).  Other possible interaction would be use
> > > > > of
> > > > > setfscreatecon() prior to creating a file in cgroup.
> > > > 
> > > > Oh, I see - it is the latter.
> > > > 
> > > > For example, init.rc does mkdir /dev/cpuctl/bg_non_interactive,
> > > > which
> > > > internally looks up the context for that directory from
> > > > file_contexts
> > > > and does a setfscreatecon() followed by a mkdir().  Previously,
> > > > that
> > > > was ignored because cgroup did not support anything other than
> > > > the
> > > > policy-defined label.  But now it will try to use that label,
> > > > which
> > > > in
> > > > turn will trigger a denial in enforcing mode and the create
> > > > will
> > > > fail.
> > > > 
> > > > So this is an incompatible change and needs to be reverted.
> > > > We'll need to wrap it up with a policy capability or something
> > > > to
> > > > allow
> > > > it to be enabled only if the policy correctly supports
> > > > it.  Even
> > > > better, we should instead just allow the policy to specify
> > > > which
> > > > filesystems should support this behavior (already on the issues
> > > > list).
> > > > 
> > > 
> > > If Android is the only system affected by this bug, I would
> > > prefer to
> > > just fix Android to allow for this patch, rather than having
> > > additional kernel complexity.
> > 
> > Well, it does break userspace (even if it happens to only affect
> > Android, which isn't clear, e.g. possibly a distribution would
> > likewise
> > suffer breakage under a tighter policy), and we already have a
> > long-
> > standing open issue to replace the current set of whitelisted
> > filesystem types with something configuration-driven.  So I'm ok
> > with
> > reverting it and requiring it to be done in a more general way.
> >  The
> > latter is something we want regardless.
> > 
> 
> Please revert this, it's not ok to break working userspace
> code.  I've
> gotten a few off-line queries as to why this ended up being merged
> when
> it was known to break Android.

It should be fixed by commit 2651225b5ebcdde60f684c4db8ec7e9e3800a74f
("selinux: wrap cgroup seclabel support with its own policy
capability").