[PATCH] selinux:Significant reduce of preempt_disable holds

From: peter.enderborg
Date: Wed Jan 17 2018 - 09:57:08 EST

From: Peter Enderborg <peter.enderborg@xxxxxxxx>

Holding the preempt_disable is very bad for low latency tasks
as audio and therefore we need to break out the rule-set dependent
part from this disable. By using a rwsem instead of rwlock we
have an efficient locking and less preemption interference.

Selinux uses a lot of read_locks. This patch replaces the rwlock
with rwsem/percpu_down_read() that does not hold preempt_disable.

Intel Xeon W3520 2.67 Ghz running FC27 with 4.15.0-rc8git (+measurement)
I get preempt_disable in worst case for 1.2ms in security_compute_av().
With the patch I get 960us as the longest security_compute_av()
without preempt disabeld. It very much noise in the measurement
but it is not likely a degrade.

And the preempt_disable times is also very dependent on the selinux
rule-set.

In security_get_user_sids() we have two nested for-loops and the
inner part calls sittab_context_to_sid() that calls
sidtab_search_context() that has a for loop() over a while() where
the loops is dependent on the rules.

On the test system the average lookup time is 60us and does
not change with the rwsem usage.

Reported-by: BjÃrn Davidsson <bjorn.davidsson@xxxxxxxx>
Signed-off-by: Peter Enderborg <peter.enderborg@xxxxxxxx>
---
security/selinux/ss/services.c | 134 ++++++++++++++++++++---------------------
1 file changed, 67 insertions(+), 67 deletions(-)

diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index 33cfe5d..a3daaf2 100644
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
@@ -87,7 +87,7 @@ int selinux_policycap_alwaysnetwork;
int selinux_policycap_cgroupseclabel;
int selinux_policycap_nnp_nosuid_transition;

-static DEFINE_RWLOCK(policy_rwlock);
+DEFINE_STATIC_PERCPU_RWSEM(policy_rwsem);

static struct sidtab sidtab;
struct policydb policydb;
@@ -779,7 +779,7 @@ static int security_compute_validatetrans(u32 oldsid, u32 newsid, u32 tasksid,
if (!ss_initialized)
return 0;

- read_lock(&policy_rwlock);
+ percpu_down_read(&policy_rwsem);

if (!user)
tclass = unmap_class(orig_tclass);
@@ -833,7 +833,7 @@ static int security_compute_validatetrans(u32 oldsid, u32 newsid, u32 tasksid,
}

out:
- read_unlock(&policy_rwlock);
+ percpu_up_read(&policy_rwsem);
return rc;
}

@@ -867,7 +867,7 @@ int security_bounded_transition(u32 old_sid, u32 new_sid)
int index;
int rc;

- read_lock(&policy_rwlock);
+ percpu_down_read(&policy_rwsem);

rc = -EINVAL;
old_context = sidtab_search(&sidtab, old_sid);
@@ -929,7 +929,7 @@ int security_bounded_transition(u32 old_sid, u32 new_sid)
kfree(old_name);
}
out:
- read_unlock(&policy_rwlock);
+ percpu_up_read(&policy_rwsem);

return rc;
}
@@ -1017,7 +1017,7 @@ void security_compute_xperms_decision(u32 ssid,
memset(xpermd->auditallow->p, 0, sizeof(xpermd->auditallow->p));
memset(xpermd->dontaudit->p, 0, sizeof(xpermd->dontaudit->p));

- read_lock(&policy_rwlock);
+ percpu_down_read(&policy_rwsem);
if (!ss_initialized)
goto allow;

@@ -1070,7 +1070,7 @@ void security_compute_xperms_decision(u32 ssid,
}
}
out:
- read_unlock(&policy_rwlock);
+ percpu_up_read(&policy_rwsem);
return;
allow:
memset(xpermd->allowed->p, 0xff, sizeof(xpermd->allowed->p));
@@ -1097,7 +1097,7 @@ void security_compute_av(u32 ssid,
u16 tclass;
struct context *scontext = NULL, *tcontext = NULL;

- read_lock(&policy_rwlock);
+ percpu_down_read(&policy_rwsem);
avd_init(avd);
xperms->len = 0;
if (!ss_initialized)
@@ -1130,7 +1130,7 @@ void security_compute_av(u32 ssid,
context_struct_compute_av(scontext, tcontext, tclass, avd, xperms);
map_decision(orig_tclass, avd, policydb.allow_unknown);
out:
- read_unlock(&policy_rwlock);
+ percpu_up_read(&policy_rwsem);
return;
allow:
avd->allowed = 0xffffffff;
@@ -1144,7 +1144,7 @@ void security_compute_av_user(u32 ssid,
{
struct context *scontext = NULL, *tcontext = NULL;

- read_lock(&policy_rwlock);
+ percpu_down_read(&policy_rwsem);
avd_init(avd);
if (!ss_initialized)
goto allow;
@@ -1175,7 +1175,7 @@ void security_compute_av_user(u32 ssid,

context_struct_compute_av(scontext, tcontext, tclass, avd, NULL);
out:
- read_unlock(&policy_rwlock);
+ percpu_up_read(&policy_rwsem);
return;
allow:
avd->allowed = 0xffffffff;
@@ -1277,7 +1277,7 @@ static int security_sid_to_context_core(u32 sid, char **scontext,
rc = -EINVAL;
goto out;
}
- read_lock(&policy_rwlock);
+ percpu_down_read(&policy_rwsem);
if (force)
context = sidtab_search_force(&sidtab, sid);
else
@@ -1290,7 +1290,7 @@ static int security_sid_to_context_core(u32 sid, char **scontext,
}
rc = context_struct_to_string(context, scontext, scontext_len);
out_unlock:
- read_unlock(&policy_rwlock);
+ percpu_up_read(&policy_rwsem);
out:
return rc;

@@ -1442,7 +1442,7 @@ static int security_context_to_sid_core(const char *scontext, u32 scontext_len,
goto out;
}

- read_lock(&policy_rwlock);
+ percpu_down_read(&policy_rwsem);
rc = string_to_context_struct(&policydb, &sidtab, scontext2,
scontext_len, &context, def_sid);
if (rc == -EINVAL && force) {
@@ -1454,7 +1454,7 @@ static int security_context_to_sid_core(const char *scontext, u32 scontext_len,
rc = sidtab_context_to_sid(&sidtab, &context, sid);
context_destroy(&context);
out_unlock:
- read_unlock(&policy_rwlock);
+ percpu_up_read(&policy_rwsem);
out:
kfree(scontext2);
kfree(str);
@@ -1604,7 +1604,7 @@ static int security_compute_sid(u32 ssid,

context_init(&newcontext);

- read_lock(&policy_rwlock);
+ percpu_down_read(&policy_rwsem);

if (kern) {
tclass = unmap_class(orig_tclass);
@@ -1738,7 +1738,7 @@ static int security_compute_sid(u32 ssid,
/* Obtain the sid for the context. */
rc = sidtab_context_to_sid(&sidtab, &newcontext, out_sid);
out_unlock:
- read_unlock(&policy_rwlock);
+ percpu_up_read(&policy_rwsem);
context_destroy(&newcontext);
out:
return rc;
@@ -2160,7 +2160,7 @@ int security_load_policy(void *data, size_t len)
sidtab_set(&oldsidtab, &sidtab);

/* Install the new policydb and SID table. */
- write_lock_irq(&policy_rwlock);
+ percpu_down_write(&policy_rwsem);
memcpy(&policydb, newpolicydb, sizeof(policydb));
sidtab_set(&sidtab, &newsidtab);
security_load_policycaps();
@@ -2168,7 +2168,7 @@ int security_load_policy(void *data, size_t len)
current_mapping = map;
current_mapping_size = map_size;
seqno = ++latest_granting;
- write_unlock_irq(&policy_rwlock);
+ percpu_up_write(&policy_rwsem);

/* Free the old policydb and SID table. */
policydb_destroy(oldpolicydb);
@@ -2198,9 +2198,9 @@ size_t security_policydb_len(void)
{
size_t len;

- read_lock(&policy_rwlock);
+ percpu_down_read(&policy_rwsem);
len = policydb.len;
- read_unlock(&policy_rwlock);
+ percpu_up_read(&policy_rwsem);

return len;
}
@@ -2216,7 +2216,7 @@ int security_port_sid(u8 protocol, u16 port, u32 *out_sid)
struct ocontext *c;
int rc = 0;

- read_lock(&policy_rwlock);
+ percpu_down_read(&policy_rwsem);

c = policydb.ocontexts[OCON_PORT];
while (c) {
@@ -2241,7 +2241,7 @@ int security_port_sid(u8 protocol, u16 port, u32 *out_sid)
}

out:
- read_unlock(&policy_rwlock);
+ percpu_up_read(&policy_rwsem);
return rc;
}

@@ -2256,7 +2256,7 @@ int security_ib_pkey_sid(u64 subnet_prefix, u16 pkey_num, u32 *out_sid)
struct ocontext *c;
int rc = 0;

- read_lock(&policy_rwlock);
+ percpu_down_read(&policy_rwsem);

c = policydb.ocontexts[OCON_IBPKEY];
while (c) {
@@ -2281,7 +2281,7 @@ int security_ib_pkey_sid(u64 subnet_prefix, u16 pkey_num, u32 *out_sid)
*out_sid = SECINITSID_UNLABELED;

out:
- read_unlock(&policy_rwlock);
+ percpu_up_read(&policy_rwsem);
return rc;
}

@@ -2296,7 +2296,7 @@ int security_ib_endport_sid(const char *dev_name, u8 port_num, u32 *out_sid)
struct ocontext *c;
int rc = 0;

- read_lock(&policy_rwlock);
+ percpu_down_read(&policy_rwsem);

c = policydb.ocontexts[OCON_IBENDPORT];
while (c) {
@@ -2322,7 +2322,7 @@ int security_ib_endport_sid(const char *dev_name, u8 port_num, u32 *out_sid)
*out_sid = SECINITSID_UNLABELED;

out:
- read_unlock(&policy_rwlock);
+ percpu_up_read(&policy_rwsem);
return rc;
}

@@ -2336,7 +2336,7 @@ int security_netif_sid(char *name, u32 *if_sid)
int rc = 0;
struct ocontext *c;

- read_lock(&policy_rwlock);
+ percpu_down_read(&policy_rwsem);

c = policydb.ocontexts[OCON_NETIF];
while (c) {
@@ -2363,7 +2363,7 @@ int security_netif_sid(char *name, u32 *if_sid)
*if_sid = SECINITSID_NETIF;

out:
- read_unlock(&policy_rwlock);
+ percpu_up_read(&policy_rwsem);
return rc;
}

@@ -2395,7 +2395,7 @@ int security_node_sid(u16 domain,
int rc;
struct ocontext *c;

- read_lock(&policy_rwlock);
+ percpu_down_read(&policy_rwsem);

switch (domain) {
case AF_INET: {
@@ -2450,7 +2450,7 @@ int security_node_sid(u16 domain,

rc = 0;
out:
- read_unlock(&policy_rwlock);
+ percpu_up_read(&policy_rwsem);
return rc;
}

@@ -2489,7 +2489,7 @@ int security_get_user_sids(u32 fromsid,
if (!ss_initialized)
goto out;

- read_lock(&policy_rwlock);
+ percpu_down_read(&policy_rwsem);

context_init(&usercon);

@@ -2539,7 +2539,7 @@ int security_get_user_sids(u32 fromsid,
}
rc = 0;
out_unlock:
- read_unlock(&policy_rwlock);
+ percpu_up_read(&policy_rwsem);
if (rc || !mynel) {
kfree(mysids);
goto out;
@@ -2580,7 +2580,7 @@ int security_get_user_sids(u32 fromsid,
* cannot support xattr or use a fixed labeling behavior like
* transition SIDs or task SIDs.
*
- * The caller must acquire the policy_rwlock before calling this function.
+ * The caller must acquire the policy_rwsem before calling this function.
*/
static inline int __security_genfs_sid(const char *fstype,
char *path,
@@ -2639,7 +2639,7 @@ static inline int __security_genfs_sid(const char *fstype,
* @sclass: file security class
* @sid: SID for path
*
- * Acquire policy_rwlock before calling __security_genfs_sid() and release
+ * Acquire policy_rwsem before calling __security_genfs_sid() and release
* it afterward.
*/
int security_genfs_sid(const char *fstype,
@@ -2649,9 +2649,9 @@ int security_genfs_sid(const char *fstype,
{
int retval;

- read_lock(&policy_rwlock);
+ percpu_down_read(&policy_rwsem);
retval = __security_genfs_sid(fstype, path, orig_sclass, sid);
- read_unlock(&policy_rwlock);
+ percpu_up_read(&policy_rwsem);
return retval;
}

@@ -2666,7 +2666,7 @@ int security_fs_use(struct super_block *sb)
struct superblock_security_struct *sbsec = sb->s_security;
const char *fstype = sb->s_type->name;

- read_lock(&policy_rwlock);
+ percpu_down_read(&policy_rwsem);

c = policydb.ocontexts[OCON_FSUSE];
while (c) {
@@ -2696,7 +2696,7 @@ int security_fs_use(struct super_block *sb)
}

out:
- read_unlock(&policy_rwlock);
+ percpu_up_read(&policy_rwsem);
return rc;
}

@@ -2704,7 +2704,7 @@ int security_get_bools(int *len, char ***names, int **values)
{
int i, rc;

- read_lock(&policy_rwlock);
+ percpu_down_read(&policy_rwsem);
*names = NULL;
*values = NULL;

@@ -2733,7 +2733,7 @@ int security_get_bools(int *len, char ***names, int **values)
}
rc = 0;
out:
- read_unlock(&policy_rwlock);
+ percpu_up_read(&policy_rwsem);
return rc;
err:
if (*names) {
@@ -2751,7 +2751,7 @@ int security_set_bools(int len, int *values)
int lenp, seqno = 0;
struct cond_node *cur;

- write_lock_irq(&policy_rwlock);
+ percpu_down_write(&policy_rwsem);

rc = -EFAULT;
lenp = policydb.p_bools.nprim;
@@ -2784,7 +2784,7 @@ int security_set_bools(int len, int *values)
seqno = ++latest_granting;
rc = 0;
out:
- write_unlock_irq(&policy_rwlock);
+ percpu_up_write(&policy_rwsem);
if (!rc) {
avc_ss_reset(seqno);
selnl_notify_policyload(seqno);
@@ -2799,7 +2799,7 @@ int security_get_bool_value(int index)
int rc;
int len;

- read_lock(&policy_rwlock);
+ percpu_down_read(&policy_rwsem);

rc = -EFAULT;
len = policydb.p_bools.nprim;
@@ -2808,7 +2808,7 @@ int security_get_bool_value(int index)

rc = policydb.bool_val_to_struct[index]->state;
out:
- read_unlock(&policy_rwlock);
+ percpu_up_read(&policy_rwsem);
return rc;
}

@@ -2864,7 +2864,7 @@ int security_sid_mls_copy(u32 sid, u32 mls_sid, u32 *new_sid)

context_init(&newcon);

- read_lock(&policy_rwlock);
+ percpu_down_read(&policy_rwsem);

rc = -EINVAL;
context1 = sidtab_search(&sidtab, sid);
@@ -2906,7 +2906,7 @@ int security_sid_mls_copy(u32 sid, u32 mls_sid, u32 *new_sid)

rc = sidtab_context_to_sid(&sidtab, &newcon, new_sid);
out_unlock:
- read_unlock(&policy_rwlock);
+ percpu_up_read(&policy_rwsem);
context_destroy(&newcon);
out:
return rc;
@@ -2963,7 +2963,7 @@ int security_net_peersid_resolve(u32 nlbl_sid, u32 nlbl_type,
if (!policydb.mls_enabled)
return 0;

- read_lock(&policy_rwlock);
+ percpu_down_read(&policy_rwsem);

rc = -EINVAL;
nlbl_ctx = sidtab_search(&sidtab, nlbl_sid);
@@ -2990,7 +2990,7 @@ int security_net_peersid_resolve(u32 nlbl_sid, u32 nlbl_type,
* expressive */
*peer_sid = xfrm_sid;
out:
- read_unlock(&policy_rwlock);
+ percpu_up_read(&policy_rwsem);
return rc;
}

@@ -3011,7 +3011,7 @@ int security_get_classes(char ***classes, int *nclasses)
{
int rc;

- read_lock(&policy_rwlock);
+ percpu_down_read(&policy_rwsem);

rc = -ENOMEM;
*nclasses = policydb.p_classes.nprim;
@@ -3029,7 +3029,7 @@ int security_get_classes(char ***classes, int *nclasses)
}

out:
- read_unlock(&policy_rwlock);
+ percpu_up_read(&policy_rwsem);
return rc;
}

@@ -3051,7 +3051,7 @@ int security_get_permissions(char *class, char ***perms, int *nperms)
int rc, i;
struct class_datum *match;

- read_lock(&policy_rwlock);
+ percpu_down_read(&policy_rwsem);

rc = -EINVAL;
match = hashtab_search(policydb.p_classes.table, class);
@@ -3080,11 +3080,11 @@ int security_get_permissions(char *class, char ***perms, int *nperms)
goto err;

out:
- read_unlock(&policy_rwlock);
+ percpu_up_read(&policy_rwsem);
return rc;

err:
- read_unlock(&policy_rwlock);
+ percpu_up_read(&policy_rwsem);
for (i = 0; i < *nperms; i++)
kfree((*perms)[i]);
kfree(*perms);
@@ -3115,9 +3115,9 @@ int security_policycap_supported(unsigned int req_cap)
{
int rc;

- read_lock(&policy_rwlock);
+ percpu_down_read(&policy_rwsem);
rc = ebitmap_get_bit(&policydb.policycaps, req_cap);
- read_unlock(&policy_rwlock);
+ percpu_up_read(&policy_rwsem);

return rc;
}
@@ -3181,7 +3181,7 @@ int selinux_audit_rule_init(u32 field, u32 op, char *rulestr, void **vrule)

context_init(&tmprule->au_ctxt);

- read_lock(&policy_rwlock);
+ percpu_down_read(&policy_rwsem);

tmprule->au_seqno = latest_granting;

@@ -3221,7 +3221,7 @@ int selinux_audit_rule_init(u32 field, u32 op, char *rulestr, void **vrule)
}
rc = 0;
out:
- read_unlock(&policy_rwlock);
+ percpu_up_read(&policy_rwsem);

if (rc) {
selinux_audit_rule_free(tmprule);
@@ -3271,7 +3271,7 @@ int selinux_audit_rule_match(u32 sid, u32 field, u32 op, void *vrule,
return -ENOENT;
}

- read_lock(&policy_rwlock);
+ percpu_down_read(&policy_rwsem);

if (rule->au_seqno < latest_granting) {
match = -ESTALE;
@@ -3362,7 +3362,7 @@ int selinux_audit_rule_match(u32 sid, u32 field, u32 op, void *vrule,
}

out:
- read_unlock(&policy_rwlock);
+ percpu_up_read(&policy_rwsem);
return match;
}

@@ -3448,7 +3448,7 @@ int security_netlbl_secattr_to_sid(struct netlbl_lsm_secattr *secattr,
return 0;
}

- read_lock(&policy_rwlock);
+ percpu_down_read(&policy_rwsem);

if (secattr->flags & NETLBL_SECATTR_CACHE)
*sid = *(u32 *)secattr->cache->data;
@@ -3484,12 +3484,12 @@ int security_netlbl_secattr_to_sid(struct netlbl_lsm_secattr *secattr,
} else
*sid = SECSID_NULL;

- read_unlock(&policy_rwlock);
+ percpu_up_read(&policy_rwsem);
return 0;
out_free:
ebitmap_destroy(&ctx_new.range.level[0].cat);
out:
- read_unlock(&policy_rwlock);
+ percpu_up_read(&policy_rwsem);
return rc;
}

@@ -3511,7 +3511,7 @@ int security_netlbl_sid_to_secattr(u32 sid, struct netlbl_lsm_secattr *secattr)
if (!ss_initialized)
return 0;

- read_lock(&policy_rwlock);
+ percpu_down_read(&policy_rwsem);

rc = -ENOENT;
ctx = sidtab_search(&sidtab, sid);
@@ -3529,7 +3529,7 @@ int security_netlbl_sid_to_secattr(u32 sid, struct netlbl_lsm_secattr *secattr)
mls_export_netlbl_lvl(ctx, secattr);
rc = mls_export_netlbl_cat(ctx, secattr);
out:
- read_unlock(&policy_rwlock);
+ percpu_up_read(&policy_rwsem);
return rc;
}
#endif /* CONFIG_NETLABEL */
@@ -3557,9 +3557,9 @@ int security_read_policy(void **data, size_t *len)
fp.data = *data;
fp.len = *len;

- read_lock(&policy_rwlock);
+ percpu_down_read(&policy_rwsem);
rc = policydb_write(&policydb, &fp);
- read_unlock(&policy_rwlock);
+ percpu_up_read(&policy_rwsem);

if (rc)
return rc;
--
2.7.4