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

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1488225201.19819.37.camel@tycho.nsa.gov>
Date:   Mon, 27 Feb 2017 14:53:21 -0500
From:   Stephen Smalley <sds@...ho.nsa.gov>
To:     Paul Moore <paul@...l-moore.com>,
        John Stultz <john.stultz@...aro.org>,
        Jeffrey Vander Stoep <jeffv@...gle.com>
Cc:     Antonio Murdaca <amurdaca@...hat.com>,
        lkml <linux-kernel@...r.kernel.org>,
        Android Kernel Team <kernel-team@...roid.com>,
        Nick Kralevich <nnk@...gle.com>
Subject: Re: [Regression?] 1ea0ce4069 ("selinux: allow changing labels for
 cgroupfs") stops Android from booting

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@...aro.or
> > 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).