All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] selinux:Significant reduce of preempt_disable holds
@ 2018-01-17 14:55 ` peter.enderborg at sony.com
  0 siblings, 0 replies; 5+ messages in thread
From: peter.enderborg @ 2018-01-17 14:55 UTC (permalink / raw)
  To: Paul Moore, Stephen Smalley, Eric Paris, James Morris,
	Daniel Jurgens, Doug Ledford, selinux, linux-security-module,
	linux-kernel, Ingo Molnar, alsa-devel, Serge E . Hallyn
  Cc: Peter Enderborg

From: Peter Enderborg <peter.enderborg@sony.com>

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@sony.com>
Signed-off-by: Peter Enderborg <peter.enderborg@sony.com>
---
 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

^ permalink raw reply related	[flat|nested] 5+ messages in thread
* [PATCH] selinux:Significant reduce of preempt_disable holds
@ 2018-01-17 14:55 ` peter.enderborg at sony.com
  0 siblings, 0 replies; 5+ messages in thread
From: peter.enderborg at sony.com @ 2018-01-17 14:55 UTC (permalink / raw)
  To: linux-security-module

From: Peter Enderborg <peter.enderborg@sony.com>

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@sony.com>
Signed-off-by: Peter Enderborg <peter.enderborg@sony.com>
---
 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

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 5+ messages in thread
* [PATCH] selinux:Significant reduce of preempt_disable holds
@ 2018-01-17 14:55 ` peter.enderborg at sony.com
  0 siblings, 0 replies; 5+ messages in thread
From: peter.enderborg @ 2018-01-17 14:55 UTC (permalink / raw)
  To: Paul Moore, Stephen Smalley, Eric Paris, James Morris,
	Daniel Jurgens, Doug Ledford, selinux, linux-security-module,
	linux-kernel, Ingo Molnar, alsa-devel, Serge E . Hallyn
  Cc: Peter Enderborg

From: Peter Enderborg <peter.enderborg@sony.com>

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@sony.com>
Signed-off-by: Peter Enderborg <peter.enderborg@sony.com>
---
 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


^ permalink raw reply related	[flat|nested] 5+ messages in thread
* Re: [PATCH] selinux:Significant reduce of preempt_disable holds
  2018-01-17 14:55 ` peter.enderborg at sony.com
@ 2018-01-17 16:14   ` Stephen Smalley
  -1 siblings, 0 replies; 5+ messages in thread
From: Stephen Smalley @ 2018-01-17 16:14 UTC (permalink / raw)
  To: peter.enderborg, Paul Moore, Eric Paris, James Morris,
	Daniel Jurgens, Doug Ledford, selinux, linux-security-module,
	linux-kernel, Ingo Molnar, alsa-devel, Serge E . Hallyn

On Wed, 2018-01-17 at 15:55 +0100, peter.enderborg@sony.com wrote:
> From: Peter Enderborg <peter.enderborg@sony.com>
> 
> 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.

Many of these functions are called while holding spinlocks, and some of
them are called from interrupt.  Unless I misunderstand, you can't just
replace read_lock() with percpu_down_read(), which might sleep.

What you might be able to do is to convert the whole thing to RCU, but
this would require reworking how policy booleans are changed and how
policy is reloaded.

You might also try increasing your AVC size via
/sys/fs/selinux/avc/cache_threshold to reduce cache misses and thus
calls to security_compute_av().

> 
> 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@sony.com>
> Signed-off-by: Peter Enderborg <peter.enderborg@sony.com>
> ---
>  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;

^ permalink raw reply	[flat|nested] 5+ messages in thread
* [PATCH] selinux:Significant reduce of preempt_disable holds
@ 2018-01-17 16:14   ` Stephen Smalley
  0 siblings, 0 replies; 5+ messages in thread
From: Stephen Smalley @ 2018-01-17 16:14 UTC (permalink / raw)
  To: linux-security-module

On Wed, 2018-01-17 at 15:55 +0100, peter.enderborg at sony.com wrote:
> From: Peter Enderborg <peter.enderborg@sony.com>
> 
> 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.

Many of these functions are called while holding spinlocks, and some of
them are called from interrupt.  Unless I misunderstand, you can't just
replace read_lock() with percpu_down_read(), which might sleep.

What you might be able to do is to convert the whole thing to RCU, but
this would require reworking how policy booleans are changed and how
policy is reloaded.

You might also try increasing your AVC size via
/sys/fs/selinux/avc/cache_threshold to reduce cache misses and thus
calls to security_compute_av().

> 
> 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@sony.com>
> Signed-off-by: Peter Enderborg <peter.enderborg@sony.com>
> ---
> ?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;
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-01-17 16:15 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-17 14:55 [PATCH] selinux:Significant reduce of preempt_disable holds peter.enderborg
2018-01-17 14:55 ` peter.enderborg
2018-01-17 14:55 ` peter.enderborg at sony.com
2018-01-17 16:14 ` Stephen Smalley
2018-01-17 16:14   ` Stephen Smalley

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.