lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Thu, 09 Mar 2017 12:57:14 -0500 From: Stephen Smalley <sds@...ho.nsa.gov> To: Greg KH <greg@...ah.com> Cc: Nick Kralevich <nnk@...gle.com>, Paul Moore <paul@...l-moore.com>, John Stultz <john.stultz@...aro.org>, Jeffrey Vander Stoep <jeffv@...gle.com>, Antonio Murdaca <amurdaca@...hat.com>, lkml <linux-kernel@...r.kernel.org>, Android Kernel Team <kernel-team@...roid.com> Subject: Re: [Regression?] 1ea0ce4069 ("selinux: allow changing labels for cgroupfs") stops Android from booting 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@...ho.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").
Powered by blists - more mailing lists