From 9ff9abc4c6be27ff27b6df625501a46711730520 Mon Sep 17 00:00:00 2001 From: Stephen Smalley Date: Wed, 26 Aug 2020 13:28:53 -0400 Subject: [PATCH] selinux: move policy mutex to selinux_state, use in lockdep checks Move the mutex used to synchronize policy changes (reloads and setting of booleans) from selinux_fs_info to selinux_state and use it in lockdep checks for rcu_dereference_protected() calls in the security server functions. This makes the dependency on the mutex explicit in the code rather than relying on comments. Signed-off-by: Stephen Smalley Reviewed-by: Ondrej Mosnacek Signed-off-by: Paul Moore --- security/selinux/hooks.c | 1 + security/selinux/include/security.h | 1 + security/selinux/selinuxfs.c | 26 ++++++++++++-------------- security/selinux/ss/services.c | 37 ++++++++----------------------------- 4 files changed, 22 insertions(+), 43 deletions(-) diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 6f30ba1..6210e98 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -7237,6 +7237,7 @@ static __init int selinux_init(void) selinux_state.checkreqprot = selinux_checkreqprot_boot; selinux_avc_init(&selinux_state.avc); mutex_init(&selinux_state.status_lock); + mutex_init(&selinux_state.policy_mutex); /* Set the security state for the initial task. */ cred_init_security(); diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h index 505e512..bbbf714 100644 --- a/security/selinux/include/security.h +++ b/security/selinux/include/security.h @@ -103,6 +103,7 @@ struct selinux_state { struct selinux_avc *avc; struct selinux_policy __rcu *policy; + struct mutex policy_mutex; } __randomize_layout; void selinux_avc_init(struct selinux_avc **avc); diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c index d1872ad..29567ac 100644 --- a/security/selinux/selinuxfs.c +++ b/security/selinux/selinuxfs.c @@ -75,7 +75,6 @@ struct selinux_fs_info { unsigned long last_class_ino; bool policy_opened; struct dentry *policycap_dir; - struct mutex mutex; unsigned long last_ino; struct selinux_state *state; struct super_block *sb; @@ -89,7 +88,6 @@ static int selinux_fs_info_create(struct super_block *sb) if (!fsi) return -ENOMEM; - mutex_init(&fsi->mutex); fsi->last_ino = SEL_INO_NEXT - 1; fsi->state = &selinux_state; fsi->sb = sb; @@ -400,7 +398,7 @@ static int sel_open_policy(struct inode *inode, struct file *filp) BUG_ON(filp->private_data); - mutex_lock(&fsi->mutex); + mutex_lock(&fsi->state->policy_mutex); rc = avc_has_perm(&selinux_state, current_sid(), SECINITSID_SECURITY, @@ -431,11 +429,11 @@ static int sel_open_policy(struct inode *inode, struct file *filp) filp->private_data = plm; - mutex_unlock(&fsi->mutex); + mutex_unlock(&fsi->state->policy_mutex); return 0; err: - mutex_unlock(&fsi->mutex); + mutex_unlock(&fsi->state->policy_mutex); if (plm) vfree(plm->data); @@ -622,7 +620,7 @@ static ssize_t sel_write_load(struct file *file, const char __user *buf, ssize_t length; void *data = NULL; - mutex_lock(&fsi->mutex); + mutex_lock(&fsi->state->policy_mutex); length = avc_has_perm(&selinux_state, current_sid(), SECINITSID_SECURITY, @@ -666,7 +664,7 @@ out1: from_kuid(&init_user_ns, audit_get_loginuid(current)), audit_get_sessionid(current)); out: - mutex_unlock(&fsi->mutex); + mutex_unlock(&fsi->state->policy_mutex); vfree(data); return length; } @@ -1271,7 +1269,7 @@ static ssize_t sel_read_bool(struct file *filep, char __user *buf, unsigned index = file_inode(filep)->i_ino & SEL_INO_MASK; const char *name = filep->f_path.dentry->d_name.name; - mutex_lock(&fsi->mutex); + mutex_lock(&fsi->state->policy_mutex); ret = -EINVAL; if (index >= fsi->bool_num || strcmp(name, @@ -1290,14 +1288,14 @@ static ssize_t sel_read_bool(struct file *filep, char __user *buf, } length = scnprintf(page, PAGE_SIZE, "%d %d", cur_enforcing, fsi->bool_pending_values[index]); - mutex_unlock(&fsi->mutex); + mutex_unlock(&fsi->state->policy_mutex); ret = simple_read_from_buffer(buf, count, ppos, page, length); out_free: free_page((unsigned long)page); return ret; out_unlock: - mutex_unlock(&fsi->mutex); + mutex_unlock(&fsi->state->policy_mutex); goto out_free; } @@ -1322,7 +1320,7 @@ static ssize_t sel_write_bool(struct file *filep, const char __user *buf, if (IS_ERR(page)) return PTR_ERR(page); - mutex_lock(&fsi->mutex); + mutex_lock(&fsi->state->policy_mutex); length = avc_has_perm(&selinux_state, current_sid(), SECINITSID_SECURITY, @@ -1347,7 +1345,7 @@ static ssize_t sel_write_bool(struct file *filep, const char __user *buf, length = count; out: - mutex_unlock(&fsi->mutex); + mutex_unlock(&fsi->state->policy_mutex); kfree(page); return length; } @@ -1378,7 +1376,7 @@ static ssize_t sel_commit_bools_write(struct file *filep, if (IS_ERR(page)) return PTR_ERR(page); - mutex_lock(&fsi->mutex); + mutex_lock(&fsi->state->policy_mutex); length = avc_has_perm(&selinux_state, current_sid(), SECINITSID_SECURITY, @@ -1400,7 +1398,7 @@ static ssize_t sel_commit_bools_write(struct file *filep, length = count; out: - mutex_unlock(&fsi->mutex); + mutex_unlock(&fsi->state->policy_mutex); kfree(page); return length; } diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c index e730204..85cfd46 100644 --- a/security/selinux/ss/services.c +++ b/security/selinux/ss/services.c @@ -2163,13 +2163,8 @@ void selinux_policy_cancel(struct selinux_state *state, { struct selinux_policy *oldpolicy; - /* - * NOTE: We do not need to take the rcu read lock - * around the code below because other policy-modifying - * operations are already excluded by selinuxfs via - * fsi->mutex. - */ - oldpolicy = rcu_dereference_check(state->policy, 1); + oldpolicy = rcu_dereference_protected(state->policy, + lockdep_is_held(&state->policy_mutex)); sidtab_cancel_convert(oldpolicy->sidtab); selinux_policy_free(policy); @@ -2192,13 +2187,8 @@ void selinux_policy_commit(struct selinux_state *state, struct selinux_policy *oldpolicy; u32 seqno; - /* - * NOTE: We do not need to take the rcu read lock - * around the code below because other policy-modifying - * operations are already excluded by selinuxfs via - * fsi->mutex. - */ - oldpolicy = rcu_dereference_check(state->policy, 1); + oldpolicy = rcu_dereference_protected(state->policy, + lockdep_is_held(&state->policy_mutex)); /* If switching between different policy types, log MLS status */ if (oldpolicy) { @@ -2291,13 +2281,8 @@ int security_load_policy(struct selinux_state *state, void *data, size_t len, return 0; } - /* - * NOTE: We do not need to take the rcu read lock - * around the code below because other policy-modifying - * operations are already excluded by selinuxfs via - * fsi->mutex. - */ - oldpolicy = rcu_dereference_check(state->policy, 1); + oldpolicy = rcu_dereference_protected(state->policy, + lockdep_is_held(&state->policy_mutex)); /* Preserve active boolean values from the old policy */ rc = security_preserve_bools(oldpolicy, newpolicy); @@ -3013,14 +2998,8 @@ int security_set_bools(struct selinux_state *state, u32 len, int *values) if (!selinux_initialized(state)) return -EINVAL; - /* - * NOTE: We do not need to take the rcu read lock - * around the code below because other policy-modifying - * operations are already excluded by selinuxfs via - * fsi->mutex. - */ - - oldpolicy = rcu_dereference_check(state->policy, 1); + oldpolicy = rcu_dereference_protected(state->policy, + lockdep_is_held(&state->policy_mutex)); /* Consistency check on number of booleans, should never fail */ if (WARN_ON(len != oldpolicy->policydb.p_bools.nprim)) -- 2.7.4