On Mon, 2017-02-27 at 12:48 -0800, Nick Kralevich wrote: > On Mon, Feb 27, 2017 at 11:53 AM, Stephen Smalley <s...@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.