[PATCH 2/2] security,selinux: get rid of security_delete_hooks()

[Ondrej Mosnacek]

On Tue, Jan 7, 2020 at 9:00 PM James Morris <jmorris at namei.org> wrote:
> On Tue, 7 Jan 2020, Ondrej Mosnacek wrote:
>
> > The only user is SELinux, which is hereby converted to check the
> > disabled flag in each hook instead of removing the hooks from the list.
> >
> > The __lsm_ro_after_init macro is now removed and replaced with
> > __ro_after_init directly.
> >
> > This fixes a race condition in SELinux runtime disable, which was
> > introduced with the switch to hook lists in b1d9e6b0646d ("LSM: Switch
> > to lists of hooks").
> >
> > Suggested-by: Stephen Smalley <sds at tycho.nsa.gov>
> > Signed-off-by: Ondrej Mosnacek <omosnace at redhat.com>
> > ---
> >  include/linux/lsm_hooks.h    |  31 --
> >  security/Kconfig             |   5 -
> >  security/apparmor/lsm.c      |   6 +-
> >  security/commoncap.c         |   2 +-
> >  security/loadpin/loadpin.c   |   2 +-
> >  security/lockdown/lockdown.c |   2 +-
> >  security/security.c          |   5 +-
> >  security/selinux/Kconfig     |   6 -
> >  security/selinux/hooks.c     | 742 ++++++++++++++++++++++++++++++-----
> >  security/smack/smack_lsm.c   |   4 +-
> >  security/tomoyo/tomoyo.c     |   6 +-
> >  security/yama/yama_lsm.c     |   2 +-
> >  12 files changed, 654 insertions(+), 159 deletions(-)
>
> Please separate the changes for each LSM into separate patches (the
> __lsm_ro_after_init removal patch can be last).
>
> >  config SECURITY_SELINUX_DEVELOP
> > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> > index 47ad4db925cf..9ac2b6b69ff9 100644
> > --- a/security/selinux/hooks.c
> > +++ b/security/selinux/hooks.c
> > @@ -650,13 +650,15 @@ static int selinux_set_mnt_opts(struct super_block *sb,
> >  {
> >       const struct cred *cred = current_cred();
> >       struct superblock_security_struct *sbsec = sb->s_security;
> > -     struct dentry *root = sbsec->sb->s_root;
> >       struct selinux_mnt_opts *opts = mnt_opts;
>
> Seems like there are a bunch of unrelated cleanups mixed in here.

These are not unrelated - we need to avoid dereferencing the security
structs before checking selinux_disabled(), because they may be NULL
or uninitialized when SELinux has been diabled.

-- 
Ondrej Mosnacek <omosnace at redhat dot com>
Software Engineer, Security Technologies
Red Hat, Inc.