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@xxxxxxxxxxxxx>
> Signed-off-by: Ondrej Mosnacek <omosnace@xxxxxxxxxx>
> ---
> 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.
> - int set_fscontext = (oldsbsec->flags & FSCONTEXT_MNT);
> - int set_context = (oldsbsec->flags & CONTEXT_MNT);
> - int set_rootcontext = (oldsbsec->flags & ROOTCONTEXT_MNT);
> + set_fscontext = (oldsbsec->flags & FSCONTEXT_MNT);
> + set_context = (oldsbsec->flags & CONTEXT_MNT);
> + set_rootcontext = (oldsbsec->flags & ROOTCONTEXT_MNT);
>
...
> static int selinux_binder_set_context_mgr(struct task_struct *mgr)
> {
> - u32 mysid = current_sid();
> - u32 mgrsid = task_sid(mgr);
> + if (selinux_disabled(&selinux_state))
> + return 0;
>
> return avc_has_perm(&selinux_state,
> - mysid, mgrsid, SECCLASS_BINDER,
> + current_sid(), task_sid(mgr), SECCLASS_BINDER,
> BINDER__SET_CONTEXT_MGR, NULL);
> }
>
Ditto, etc.
Please don't do this.
--
James Morris
<jmorris@xxxxxxxxx>
