Re: [Regression?] 1ea0ce4069 ("selinux: allow changing labels for cgroupfs") stops Android from booting

[Stephen Smalley]

On Mon, 2017-02-27 at 14:42 -0500, Stephen Smalley wrote:
> On Thu, 2017-02-23 at 19:01 -0500, Paul Moore wrote:
> > 
> > On Thu, Feb 23, 2017 at 1:43 PM, John Stultz <john.stultz@xxxxxxxxx
> > g>
> > wrote:
> > > 
> > > 
> > > Hey folks,
> > > ÂÂÂI've not been able to figure out why yet, but I wanted to
> > > raise
> > > the
> > > issue that last night I found I couldn't boot Android on my Hikey
> > > board with Linus' HEAD kernel. It seems to cause logd to crash
> > > repeatedly so I'm not able to get debug info from logcat.
> > > 
> > > I do see the following over and over on the console:
> > > 
> > > [ÂÂÂ12.505838] init: computing context for service 'logd'
> > > [ÂÂÂ12.506355] init: starting service 'logd'...
> > > [ÂÂÂ12.507683] init: property_set("ro.boottime.logd",
> > > "12500792498")
> > > failed: property already set
> > > [ÂÂÂ12.508701] init: Created socket '/dev/socket/logd', mode 666,
> > > user
> > > 1036, group 1036
> > > [ÂÂÂ12.509294] init: Created socket '/dev/socket/logdr', mode
> > > 666,
> > > user 1036, group 1036
> > > [ÂÂÂ12.509891] init: Created socket '/dev/socket/logdw', mode
> > > 222,
> > > user 1036, group 1036
> > > [ÂÂÂ12.510132] init: Opened file '/proc/kmsg', flags 0
> > > [ÂÂÂ12.510187] init: Opened file '/dev/kmsg', flags 1
> > > [ÂÂÂ12.510353] init: couldn't write 1941 to
> > > /dev/cpuset/system-background/tasks: No such file or directory
> > > [ÂÂÂ12.533046] init: Service 'logd' (pid 1941) exited with status
> > > 255
> > > 
> > > 
> > > I did some bisection and narrowed it down to 1ea0ce4069
> > > ("selinux:
> > > allow changing labels for cgroupfs"), which was merged in
> > > yesterday.
> > > I've not yet been able to figure out the root cause, but
> > > reverting
> > > that patch makes things work again.
> > > 
> > > So I wanted to raise the issue here so folks were aware.
> > > 
> > > If there is anything folks want me to test or try, please let me
> > > know.
> > 
> > Unfortunately I don't have an Android test system to play with,
> > have
> > any of the SEAndroid folks on the To/CC line seen a similar
> > problem?
> 
> 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).