Skip to content

Commit

Permalink
selinux: move policy mutex to selinux_state, use in lockdep checks
Browse files Browse the repository at this point in the history 
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 <[email protected]>
Reviewed-by: Ondrej Mosnacek <[email protected]>
Signed-off-by: Paul Moore <[email protected]>
  • Loading branch information
@stephensmalley @pcmoore
stephensmalley authored and pcmoore committed Aug 27, 2020
1 parent 0256b0a commit 9ff9abc
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 43 deletions.
1 change: 1 addition & 0 deletions security/selinux/hooks.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
1 change: 1 addition & 0 deletions security/selinux/include/security.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
26 changes: 12 additions & 14 deletions security/selinux/selinuxfs.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -666,7 +664,7 @@ static ssize_t sel_write_load(struct file *file, const char __user *buf,
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;
}
Expand Down Expand Up @@ -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,
Expand All @@ -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;
}

Expand All @@ -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,
Expand All @@ -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;
}
Expand Down Expand Up @@ -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,
Expand All @@ -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;
}
Expand Down
37 changes: 8 additions & 29 deletions security/selinux/ss/services.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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) {
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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))
Expand Down

0 comments on commit 9ff9abc

Please sign in to comment.