In the previous change to convert the policy rwlock to RCU, the update code was using rcu_dereference_check(..., 1) with a comment to explain why it was safe without taking rcu_read_lock() since the mutex used to provide exclusion was taken at a higher level in selinuxfs. This change passes the mutex down to the necessary functions and replaces rcu_dereference_check(..., 1) with rcu_dereference_protected(..., lockdep_is_held(mutex)) so that lockdep checking is correctly applied and the dependency is made explicit in the code rather than relying on comments. Signed-off-by: Stephen Smalley <stephen.smalley.work@xxxxxxxxx> --- This is relative to the convert policy read-write lock to RCU patch, https://patchwork.kernel.org/patch/11724997/. security/selinux/include/conditional.h | 3 +- security/selinux/include/security.h | 6 ++-- security/selinux/selinuxfs.c | 12 ++++--- security/selinux/ss/services.c | 45 ++++++++------------------ 4 files changed, 26 insertions(+), 40 deletions(-) diff --git a/security/selinux/include/conditional.h b/security/selinux/include/conditional.h index b09343346e3f..4659193dc49d 100644 --- a/security/selinux/include/conditional.h +++ b/security/selinux/include/conditional.h @@ -16,7 +16,8 @@ int security_get_bools(struct selinux_policy *policy, u32 *len, char ***names, int **values); -int security_set_bools(struct selinux_state *state, u32 len, int *values); +int security_set_bools(struct selinux_state *state, struct mutex *mutex, + u32 len, int *values); int security_get_bool_value(struct selinux_state *state, u32 index); diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h index 505e51264d51..87eac1f2e6ed 100644 --- a/security/selinux/include/security.h +++ b/security/selinux/include/security.h @@ -209,12 +209,12 @@ static inline bool selinux_policycap_genfs_seclabel_symlinks(void) } int security_mls_enabled(struct selinux_state *state); -int security_load_policy(struct selinux_state *state, +int security_load_policy(struct selinux_state *state, struct mutex *mutex, void *data, size_t len, struct selinux_policy **newpolicyp); -void selinux_policy_commit(struct selinux_state *state, +void selinux_policy_commit(struct selinux_state *state, struct mutex *mutex, struct selinux_policy *newpolicy); -void selinux_policy_cancel(struct selinux_state *state, +void selinux_policy_cancel(struct selinux_state *state, struct mutex *mutex, struct selinux_policy *policy); int security_read_policy(struct selinux_state *state, void **data, size_t *len); diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c index 131816878e50..a054683359dd 100644 --- a/security/selinux/selinuxfs.c +++ b/security/selinux/selinuxfs.c @@ -560,7 +560,8 @@ static ssize_t sel_write_load(struct file *file, const char __user *buf, if (copy_from_user(data, buf, count) != 0) goto out; - length = security_load_policy(fsi->state, data, count, &newpolicy); + length = security_load_policy(fsi->state, &fsi->mutex, data, count, + &newpolicy); if (length) { pr_warn_ratelimited("SELinux: failed to load policy\n"); goto out; @@ -568,11 +569,11 @@ static ssize_t sel_write_load(struct file *file, const char __user *buf, length = sel_make_policy_nodes(fsi, newpolicy); if (length) { - selinux_policy_cancel(fsi->state, newpolicy); + selinux_policy_cancel(fsi->state, &fsi->mutex, newpolicy); goto out1; } - selinux_policy_commit(fsi->state, newpolicy); + selinux_policy_commit(fsi->state, &fsi->mutex, newpolicy); length = count; @@ -1309,8 +1310,9 @@ static ssize_t sel_commit_bools_write(struct file *filep, length = 0; if (new_value && fsi->bool_pending_values) - length = security_set_bools(fsi->state, fsi->bool_num, - fsi->bool_pending_values); + length = security_set_bools(fsi->state, &fsi->mutex, + fsi->bool_num, + fsi->bool_pending_values); if (!length) length = count; diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c index 838161462756..a9fff3592768 100644 --- a/security/selinux/ss/services.c +++ b/security/selinux/ss/services.c @@ -2159,17 +2159,13 @@ static void selinux_policy_cond_free(struct selinux_policy *policy) } void selinux_policy_cancel(struct selinux_state *state, + struct mutex *mutex, struct selinux_policy *policy) { 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(mutex)); sidtab_cancel_convert(oldpolicy->sidtab); selinux_policy_free(policy); @@ -2187,18 +2183,14 @@ static void selinux_notify_policy_change(struct selinux_state *state, } void selinux_policy_commit(struct selinux_state *state, + struct mutex *mutex, struct selinux_policy *newpolicy) { 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(mutex)); /* If switching between different policy types, log MLS status */ if (oldpolicy) { @@ -2249,7 +2241,8 @@ void selinux_policy_commit(struct selinux_state *state, * This function will flush the access vector cache after * loading the new policy. */ -int security_load_policy(struct selinux_state *state, void *data, size_t len, +int security_load_policy(struct selinux_state *state, struct mutex *mutex, + void *data, size_t len, struct selinux_policy **newpolicyp) { struct selinux_policy *newpolicy, *oldpolicy; @@ -2289,13 +2282,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(mutex)); /* Preserve active boolean values from the old policy */ rc = security_preserve_bools(oldpolicy, newpolicy); @@ -2992,7 +2980,8 @@ int security_get_bools(struct selinux_policy *policy, } -int security_set_bools(struct selinux_state *state, u32 len, int *values) +int security_set_bools(struct selinux_state *state, struct mutex *mutex, + u32 len, int *values) { struct selinux_policy *newpolicy, *oldpolicy; int rc; @@ -3001,14 +2990,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(mutex)); /* Consistency check on number of booleans, should never fail */ if (WARN_ON(len != oldpolicy->policydb.p_bools.nprim)) -- 2.25.1
