On Wed, Jan 8, 2020 at 9:10 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.
>
> Fixes: b1d9e6b0646d ("LSM: Switch to lists of hooks")
> Signed-off-by: Ondrej Mosnacek <omosnace@xxxxxxxxxx>
> ---
>
> Changes in v2:
> - rebased on latest selinux/next
> - added comment above selinux_nf_ip_exit() call to ensure it remains
> ordered correctly
>
> security/selinux/hooks.c | 101 +++++++++++++++++++++++++++------------
> 1 file changed, 70 insertions(+), 31 deletions(-)
Thanks Ondrej, I've merged this into selinux/next and added the stable
kernel CC. Normally when we mark something for stable I send it up to
Linus during the -rcX development phase, but I think this case is
somewhat unique in that it isn't widespread (and there is no
indication it will become widespread) and it requires privilege to
trigger. Also, while not a major factor, we are at -rc5 which means
we are very near the end of the -rcX cycle and I'd rather not
accidentally break something else this late in an attempt to fix such
a limited problem.
Comments and objections are welcome ;)
--
paul moore
www.paul-moore.com
