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: <CAFJ0LnHTmvBCaXKA1-j2ywHdASeHQzYS5bu0CHAzjSnXXK55fQ@mail.gmail.com>
Date:   Mon, 27 Feb 2017 12:48:32 -0800
From:   Nick Kralevich <nnk@...gle.com>
To:     Stephen Smalley <sds@...ho.nsa.gov>
Cc:     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 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.

-- 
Nick Kralevich | Android Security | nnk@...gle.com | 650.214.4037