On Mon, Dec 9, 2019 at 8:57 AM Ondrej Mosnacek <omosnace@xxxxxxxxxx> 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...
>
> Fixes: b1d9e6b0646d ("LSM: Switch to lists of hooks")
> Signed-off-by: Ondrej Mosnacek <omosnace@xxxxxxxxxx>
> ---
> security/selinux/hooks.c | 97 +++++++++++++++++++++++++++-------------
> 1 file changed, 66 insertions(+), 31 deletions(-)
Please note that this patch applies on top of Linus' tree instead of
selinux/next, since there are some new hooks there that aren't in
next.
--
Ondrej Mosnacek <omosnace at redhat dot com>
Software Engineer, Security Technologies
Red Hat, Inc.
