On 12/10/19 2:43 PM, Casey Schaufler wrote:
On 12/10/2019 11:29 AM, Paul Moore wrote:
On Tue, Dec 10, 2019 at 6:19 AM Ondrej Mosnacek <omosnace@xxxxxxxxxx> wrote:
On Mon, Dec 9, 2019 at 2:21 PM Stephen Smalley <sds@xxxxxxxxxxxxx> wrote:
On 12/9/19 2:57 AM, Ondrej Mosnacek wrote:
Commit b1d9e6b0646d ("LSM: Switch to lists of hooks") switched the LSM
infrastructure to use per-hook lists, which meant that removing the
hooks for a given module was no longer atomic. Even though the commit
clearly documents that modules implementing runtime revmoval of hooks
(only SELinux attempts this madness) need to take special precautions to
avoid race conditions, SELinux has never addressed this.
By inserting an artificial delay between the loop iterations of
security_delete_hooks() (I used 100 ms), booting to a state where
SELinux is enabled, but policy is not yet loaded, and running these
commands:
while true; do ping -c 1 <some IP>; done &
echo -n 1 >/sys/fs/selinux/disable
kill %1
wait
...I was able to trigger NULL pointer dereferences in various places. I
also have a report of someone getting panics on a stock RHEL-8 kernel
after setting SELINUX=disabled in /etc/selinux/config and rebooting
(without adding "selinux=0" to kernel command-line).
Reordering the SELinux hooks such that those that allocate structures
are removed last seems to prevent these panics. It is very much possible
that this doesn't make the runtime disable completely race-free, but at
least it makes the operation much less fragile.
An alternative (and safer) solution would be to add NULL checks to each
hook, but doing this just to support the runtime disable hack doesn't
seem to be worth the effort...
Personally, I would prefer to just get rid of runtime disable
altogether; it also precludes making the hooks read-only after
initialization. IMHO, selinux=0 is the proper way to disable SELinux if
necessary. I believe there is an open bugzilla on Fedora related to
this issue, since runtime disable was originally introduced for Fedora.
I, too, would like to see it gone, but removing it immediately would
likely cause issues for existing users (see [1]) ...
[1] https://bugzilla.redhat.com/show_bug.cgi?id=1430944#c2
For the record, and for those who didn't click on the RHBZ link above,
I'm a big fan of getting rid of SELinux's runtime disable but concede
that it must be done in such a way to as not horribly break userspace.
Is there some reason that changing the "disable SELinux" option
has to remove the hooks? Why can't it set selinux_enabled to 0
and be done with it?
selinux_enabled is only used during initialization to deal with
selinux=0 across the different components of SELinux. It isn't checked
by the hooks themselves. And if we were to add a if (!selinux_enabled)
return 0 to the start of every hook, then that's just another easy
target for kernel exploits to leverage.
