2018-01-26 14:34:17

by peter enderborg

[permalink] [raw]
Subject: [PATCH v2 0/5] selinux:Significant reduce of preempt_disable holds

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 RCU 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 RCY that does not hold preempt_disable.

Intel Xeon W3520 2.67 Ghz running FC27 with 4.15.0-rc9git (+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 RCU usage.

To use RCU the structure of policydb has to be accesses through a pointer.
We need 4 patches to get there.

[PATCH v2 1/5] selinux:Remove direct references to policydb.
We remove direct references and pass it through function arguments.

[PATCH v2 2/5] selinux: Move policydb to pointer structure
Move the policydb to dynamic allocated structure.

[PATCH v2 3/5] selinux: Move sidtab to pointer structure
Same as for policydb but for sidtab. They are closly related
and should be switched at the same time.

[PATCH v2 4/5] selinux: Use pointer to switch policydb and sidtab
Now we can switch rules by switching pointers.

[PATCH v2 5/5] selinux: Switch locking to RCU.
We are now ready to use RCU.

History: V1 rwsem


2018-01-26 14:34:24

by peter enderborg

[permalink] [raw]
Subject: [PATCH v2 3/5] selinux: Move sidtab to pointer structure

From: Peter Enderborg <[email protected]>

To be able to use rcu locks we need access the sidtab trough
a pointer. This moves the sittab to a dynamic allocated struture.

Signed-off-by: Peter Enderborg <[email protected]>
---
security/selinux/ss/services.c | 140 ++++++++++++++++++++++-------------------
1 file changed, 74 insertions(+), 66 deletions(-)

diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index 21400bd..2a8486c 100644
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
@@ -89,7 +89,6 @@ int selinux_policycap_nnp_nosuid_transition;

static DEFINE_RWLOCK(policy_rwlock);

-static struct sidtab sidtab;
int ss_initialized;

/*
@@ -120,6 +119,7 @@ struct shared_current_mapping {
struct selinux_mapping *current_mapping;
u16 current_mapping_size;
struct policydb policydb;
+ struct sidtab sidtab;
};

static struct shared_current_mapping *crm;
@@ -804,7 +804,7 @@ static int security_compute_validatetrans(u32 oldsid, u32 newsid, u32 tasksid,
}
tclass_datum = crm->policydb.class_val_to_struct[tclass - 1];

- ocontext = sidtab_search(&sidtab, oldsid);
+ ocontext = sidtab_search(&crm->sidtab, oldsid);
if (!ocontext) {
printk(KERN_ERR "SELinux: %s: unrecognized SID %d\n",
__func__, oldsid);
@@ -812,7 +812,7 @@ static int security_compute_validatetrans(u32 oldsid, u32 newsid, u32 tasksid,
goto out;
}

- ncontext = sidtab_search(&sidtab, newsid);
+ ncontext = sidtab_search(&crm->sidtab, newsid);
if (!ncontext) {
printk(KERN_ERR "SELinux: %s: unrecognized SID %d\n",
__func__, newsid);
@@ -820,7 +820,7 @@ static int security_compute_validatetrans(u32 oldsid, u32 newsid, u32 tasksid,
goto out;
}

- tcontext = sidtab_search(&sidtab, tasksid);
+ tcontext = sidtab_search(&crm->sidtab, tasksid);
if (!tcontext) {
printk(KERN_ERR "SELinux: %s: unrecognized SID %d\n",
__func__, tasksid);
@@ -882,7 +882,7 @@ int security_bounded_transition(u32 old_sid, u32 new_sid)
read_lock(&policy_rwlock);

rc = -EINVAL;
- old_context = sidtab_search(&sidtab, old_sid);
+ old_context = sidtab_search(&crm->sidtab, old_sid);
if (!old_context) {
printk(KERN_ERR "SELinux: %s: unrecognized SID %u\n",
__func__, old_sid);
@@ -890,7 +890,7 @@ int security_bounded_transition(u32 old_sid, u32 new_sid)
}

rc = -EINVAL;
- new_context = sidtab_search(&sidtab, new_sid);
+ new_context = sidtab_search(&crm->sidtab, new_sid);
if (!new_context) {
printk(KERN_ERR "SELinux: %s: unrecognized SID %u\n",
__func__, new_sid);
@@ -1033,14 +1033,14 @@ void security_compute_xperms_decision(u32 ssid,
if (!ss_initialized)
goto allow;

- scontext = sidtab_search(&sidtab, ssid);
+ scontext = sidtab_search(&crm->sidtab, ssid);
if (!scontext) {
printk(KERN_ERR "SELinux: %s: unrecognized SID %d\n",
__func__, ssid);
goto out;
}

- tcontext = sidtab_search(&sidtab, tsid);
+ tcontext = sidtab_search(&crm->sidtab, tsid);
if (!tcontext) {
printk(KERN_ERR "SELinux: %s: unrecognized SID %d\n",
__func__, tsid);
@@ -1116,7 +1116,7 @@ void security_compute_av(u32 ssid,
if (!ss_initialized)
goto allow;

- scontext = sidtab_search(&sidtab, ssid);
+ scontext = sidtab_search(&crm->sidtab, ssid);
if (!scontext) {
printk(KERN_ERR "SELinux: %s: unrecognized SID %d\n",
__func__, ssid);
@@ -1127,7 +1127,7 @@ void security_compute_av(u32 ssid,
if (ebitmap_get_bit(&crm->policydb.permissive_map, scontext->type))
avd->flags |= AVD_FLAGS_PERMISSIVE;

- tcontext = sidtab_search(&sidtab, tsid);
+ tcontext = sidtab_search(&crm->sidtab, tsid);
if (!tcontext) {
printk(KERN_ERR "SELinux: %s: unrecognized SID %d\n",
__func__, tsid);
@@ -1162,7 +1162,7 @@ void security_compute_av_user(u32 ssid,
if (!ss_initialized)
goto allow;

- scontext = sidtab_search(&sidtab, ssid);
+ scontext = sidtab_search(&crm->sidtab, ssid);
if (!scontext) {
printk(KERN_ERR "SELinux: %s: unrecognized SID %d\n",
__func__, ssid);
@@ -1173,7 +1173,7 @@ void security_compute_av_user(u32 ssid,
if (ebitmap_get_bit(&crm->policydb.permissive_map, scontext->type))
avd->flags |= AVD_FLAGS_PERMISSIVE;

- tcontext = sidtab_search(&sidtab, tsid);
+ tcontext = sidtab_search(&crm->sidtab, tsid);
if (!tcontext) {
printk(KERN_ERR "SELinux: %s: unrecognized SID %d\n",
__func__, tsid);
@@ -1295,9 +1295,9 @@ static int security_sid_to_context_core(u32 sid, char **scontext,
}
read_lock(&policy_rwlock);
if (force)
- context = sidtab_search_force(&sidtab, sid);
+ context = sidtab_search_force(&crm->sidtab, sid);
else
- context = sidtab_search(&sidtab, sid);
+ context = sidtab_search(&crm->sidtab, sid);
if (!context) {
printk(KERN_ERR "SELinux: %s: unrecognized SID %d\n",
__func__, sid);
@@ -1459,7 +1459,7 @@ static int security_context_to_sid_core(const char *scontext, u32 scontext_len,
}

read_lock(&policy_rwlock);
- rc = string_to_context_struct(&crm->policydb, &sidtab, scontext2,
+ rc = string_to_context_struct(&crm->policydb, &crm->sidtab, scontext2,
scontext_len, &context, def_sid);
if (rc == -EINVAL && force) {
context.str = str;
@@ -1467,7 +1467,7 @@ static int security_context_to_sid_core(const char *scontext, u32 scontext_len,
str = NULL;
} else if (rc)
goto out_unlock;
- rc = sidtab_context_to_sid(&sidtab, &context, sid);
+ rc = sidtab_context_to_sid(&crm->sidtab, &context, sid);
context_destroy(&context);
out_unlock:
read_unlock(&policy_rwlock);
@@ -1630,14 +1630,14 @@ static int security_compute_sid(u32 ssid,
sock = security_is_socket_class(map_class(tclass));
}

- scontext = sidtab_search(&sidtab, ssid);
+ scontext = sidtab_search(&crm->sidtab, ssid);
if (!scontext) {
printk(KERN_ERR "SELinux: %s: unrecognized SID %d\n",
__func__, ssid);
rc = -EINVAL;
goto out_unlock;
}
- tcontext = sidtab_search(&sidtab, tsid);
+ tcontext = sidtab_search(&crm->sidtab, tsid);
if (!tcontext) {
printk(KERN_ERR "SELinux: %s: unrecognized SID %d\n",
__func__, tsid);
@@ -1757,7 +1757,7 @@ static int security_compute_sid(u32 ssid,
goto out_unlock;
}
/* Obtain the sid for the context. */
- rc = sidtab_context_to_sid(&sidtab, &newcontext, out_sid);
+ rc = sidtab_context_to_sid(&crm->sidtab, &newcontext, out_sid);
out_unlock:
read_unlock(&policy_rwlock);
context_destroy(&newcontext);
@@ -2017,36 +2017,33 @@ static int convert_context(u32 key,
goto out;
}

-static void security_load_policycaps(void)
+static void security_load_policycaps(struct policydb *pdb)
{
unsigned int i;
struct ebitmap_node *node;

- selinux_policycap_netpeer =
- ebitmap_get_bit(&crm->policydb.policycaps,
- POLICYDB_CAPABILITY_NETPEER);
- selinux_policycap_openperm =
- ebitmap_get_bit(&crm->policydb.policycaps,
- POLICYDB_CAPABILITY_OPENPERM);
- selinux_policycap_extsockclass =
- ebitmap_get_bit(&crm->policydb.policycaps,
- POLICYDB_CAPABILITY_EXTSOCKCLASS);
+ selinux_policycap_netpeer = ebitmap_get_bit(&pdb->policycaps,
+ POLICYDB_CAPABILITY_NETPEER);
+ selinux_policycap_openperm = ebitmap_get_bit(&pdb->policycaps,
+ POLICYDB_CAPABILITY_OPENPERM);
+ selinux_policycap_extsockclass = ebitmap_get_bit(&pdb->policycaps,
+ POLICYDB_CAPABILITY_EXTSOCKCLASS);
selinux_policycap_alwaysnetwork =
- ebitmap_get_bit(&crm->policydb.policycaps,
+ ebitmap_get_bit(&pdb->policycaps,
POLICYDB_CAPABILITY_ALWAYSNETWORK);
selinux_policycap_cgroupseclabel =
- ebitmap_get_bit(&crm->policydb.policycaps,
+ ebitmap_get_bit(&pdb->policycaps,
POLICYDB_CAPABILITY_CGROUPSECLABEL);
selinux_policycap_nnp_nosuid_transition =
- ebitmap_get_bit(&crm->policydb.policycaps,
+ ebitmap_get_bit(&pdb->policycaps,
POLICYDB_CAPABILITY_NNP_NOSUID_TRANSITION);

for (i = 0; i < ARRAY_SIZE(selinux_policycap_names); i++)
pr_info("SELinux: policy capability %s=%d\n",
selinux_policycap_names[i],
- ebitmap_get_bit(&crm->policydb.policycaps, i));
+ ebitmap_get_bit(&pdb->policycaps, i));

- ebitmap_for_each_positive_bit(&crm->policydb.policycaps, node, i) {
+ ebitmap_for_each_positive_bit(&pdb->policycaps, node, i) {
if (i >= ARRAY_SIZE(selinux_policycap_names))
pr_info("SELinux: unknown policy capability %u\n",
i);
@@ -2072,6 +2069,8 @@ int security_load_policy(void *data, size_t len)
struct selinux_mapping *oldmap = NULL, *map = NULL;
struct convert_context_args args;
struct shared_current_mapping *new_mapping;
+ struct shared_current_mapping *next_rcu;
+
u32 seqno;
u16 map_size;
int rc = 0;
@@ -2089,6 +2088,11 @@ int security_load_policy(void *data, size_t len)
goto out;
}
newpolicydb = oldpolicydb + 1;
+ next_rcu = kmalloc(sizeof(struct shared_current_mapping), GFP_KERNEL);
+ if (!next_rcu) {
+ rc = -ENOMEM;
+ goto out;
+ }

if (!ss_initialized) {
crm = kzalloc(sizeof(struct shared_current_mapping),
@@ -2097,7 +2101,6 @@ int security_load_policy(void *data, size_t len)
rc = -ENOMEM;
goto out;
}
-
avtab_cache_init();
ebitmap_cache_init();
hashtab_cache_init();
@@ -2121,7 +2124,7 @@ int security_load_policy(void *data, size_t len)
goto out;
}

- rc = policydb_load_isids(&crm->policydb, &sidtab);
+ rc = policydb_load_isids(&crm->policydb, &crm->sidtab);
if (rc) {
policydb_destroy(&crm->policydb);
avtab_cache_destroy();
@@ -2130,7 +2133,7 @@ int security_load_policy(void *data, size_t len)
goto out;
}

- security_load_policycaps();
+ security_load_policycaps(&crm->policydb);
ss_initialized = 1;
seqno = ++latest_granting;
selinux_complete_init();
@@ -2143,7 +2146,7 @@ int security_load_policy(void *data, size_t len)
}

#if 0
- sidtab_hash_eval(&sidtab, "sids");
+ sidtab_hash_eval(&crm->sidtab, "sids");
#endif

rc = policydb_read(newpolicydb, fp);
@@ -2175,9 +2178,9 @@ int security_load_policy(void *data, size_t len)
}

/* Clone the SID table. */
- sidtab_shutdown(&sidtab);
+ sidtab_shutdown(&crm->sidtab);

- rc = sidtab_map(&sidtab, clone_sid, &newsidtab);
+ rc = sidtab_map(&crm->sidtab, clone_sid, &newsidtab);
if (rc)
goto err;

@@ -2197,19 +2200,21 @@ int security_load_policy(void *data, size_t len)

/* Save the old policydb and SID table to free later. */
memcpy(oldpolicydb, &crm->policydb, sizeof(struct policydb));
- sidtab_set(&oldsidtab, &sidtab);
+ sidtab_set(&oldsidtab, &crm->sidtab);

/* Install the new policydb and SID table. */
+ /* next */
write_lock_irq(&policy_rwlock);
- memcpy(&crm->policydb, newpolicydb, sizeof(struct policydb));
-
- sidtab_set(&sidtab, &newsidtab);
- security_load_policycaps();
+ memcpy(&next_rcu->policydb, newpolicydb, sizeof(struct policydb));
+ sidtab_set(&next_rcu->sidtab, &newsidtab);
+ security_load_policycaps(&next_rcu->policydb);
oldmap = crm->current_mapping;
- crm->current_mapping = map;
- crm->current_mapping_size = map_size;
+ next_rcu->current_mapping = map;
+ next_rcu->current_mapping_size = map_size;
+
seqno = ++latest_granting;
write_unlock_irq(&policy_rwlock);
+ crm = next_rcu;

/* Free the old policydb and SID table. */
policydb_destroy(oldpolicydb);
@@ -2270,7 +2275,7 @@ int security_port_sid(u8 protocol, u16 port, u32 *out_sid)

if (c) {
if (!c->sid[0]) {
- rc = sidtab_context_to_sid(&sidtab,
+ rc = sidtab_context_to_sid(&crm->sidtab,
&c->context[0],
&c->sid[0]);
if (rc)
@@ -2311,7 +2316,7 @@ int security_ib_pkey_sid(u64 subnet_prefix, u16 pkey_num, u32 *out_sid)

if (c) {
if (!c->sid[0]) {
- rc = sidtab_context_to_sid(&sidtab,
+ rc = sidtab_context_to_sid(&crm->sidtab,
&c->context[0],
&c->sid[0]);
if (rc)
@@ -2352,7 +2357,7 @@ int security_ib_endport_sid(const char *dev_name, u8 port_num, u32 *out_sid)

if (c) {
if (!c->sid[0]) {
- rc = sidtab_context_to_sid(&sidtab,
+ rc = sidtab_context_to_sid(&crm->sidtab,
&c->context[0],
&c->sid[0]);
if (rc)
@@ -2388,12 +2393,12 @@ int security_netif_sid(char *name, u32 *if_sid)

if (c) {
if (!c->sid[0] || !c->sid[1]) {
- rc = sidtab_context_to_sid(&sidtab,
+ rc = sidtab_context_to_sid(&crm->sidtab,
&c->context[0],
&c->sid[0]);
if (rc)
goto out;
- rc = sidtab_context_to_sid(&sidtab,
+ rc = sidtab_context_to_sid(&crm->sidtab,
&c->context[1],
&c->sid[1]);
if (rc)
@@ -2478,7 +2483,7 @@ int security_node_sid(u16 domain,

if (c) {
if (!c->sid[0]) {
- rc = sidtab_context_to_sid(&sidtab,
+ rc = sidtab_context_to_sid(&crm->sidtab,
&c->context[0],
&c->sid[0]);
if (rc)
@@ -2535,7 +2540,7 @@ int security_get_user_sids(u32 fromsid,
context_init(&usercon);

rc = -EINVAL;
- fromcon = sidtab_search(&sidtab, fromsid);
+ fromcon = sidtab_search(&crm->sidtab, fromsid);
if (!fromcon)
goto out_unlock;

@@ -2561,7 +2566,8 @@ int security_get_user_sids(u32 fromsid,
user, &usercon))
continue;

- rc = sidtab_context_to_sid(&sidtab, &usercon, &sid);
+ rc = sidtab_context_to_sid(&crm->sidtab, &usercon,
+ &sid);
if (rc)
goto out_unlock;
if (mynel < maxnel) {
@@ -2663,7 +2669,8 @@ static inline int __security_genfs_sid(const char *fstype,
goto out;

if (!c->sid[0]) {
- rc = sidtab_context_to_sid(&sidtab, &c->context[0], &c->sid[0]);
+ rc = sidtab_context_to_sid(&crm->sidtab, &c->context[0],
+ &c->sid[0]);
if (rc)
goto out;
}
@@ -2720,7 +2727,8 @@ int security_fs_use(struct super_block *sb)
if (c) {
sbsec->behavior = c->v.behavior;
if (!c->sid[0]) {
- rc = sidtab_context_to_sid(&sidtab, &c->context[0],
+ rc = sidtab_context_to_sid(&crm->sidtab,
+ &c->context[0],
&c->sid[0]);
if (rc)
goto out;
@@ -2911,7 +2919,7 @@ int security_sid_mls_copy(u32 sid, u32 mls_sid, u32 *new_sid)
read_lock(&policy_rwlock);

rc = -EINVAL;
- context1 = sidtab_search(&sidtab, sid);
+ context1 = sidtab_search(&crm->sidtab, sid);
if (!context1) {
printk(KERN_ERR "SELinux: %s: unrecognized SID %d\n",
__func__, sid);
@@ -2919,7 +2927,7 @@ int security_sid_mls_copy(u32 sid, u32 mls_sid, u32 *new_sid)
}

rc = -EINVAL;
- context2 = sidtab_search(&sidtab, mls_sid);
+ context2 = sidtab_search(&crm->sidtab, mls_sid);
if (!context2) {
printk(KERN_ERR "SELinux: %s: unrecognized SID %d\n",
__func__, mls_sid);
@@ -2948,7 +2956,7 @@ int security_sid_mls_copy(u32 sid, u32 mls_sid, u32 *new_sid)
}
}

- rc = sidtab_context_to_sid(&sidtab, &newcon, new_sid);
+ rc = sidtab_context_to_sid(&crm->sidtab, &newcon, new_sid);
out_unlock:
read_unlock(&policy_rwlock);
context_destroy(&newcon);
@@ -3010,14 +3018,14 @@ int security_net_peersid_resolve(u32 nlbl_sid, u32 nlbl_type,
read_lock(&policy_rwlock);

rc = -EINVAL;
- nlbl_ctx = sidtab_search(&sidtab, nlbl_sid);
+ nlbl_ctx = sidtab_search(&crm->sidtab, nlbl_sid);
if (!nlbl_ctx) {
printk(KERN_ERR "SELinux: %s: unrecognized SID %d\n",
__func__, nlbl_sid);
goto out;
}
rc = -EINVAL;
- xfrm_ctx = sidtab_search(&sidtab, xfrm_sid);
+ xfrm_ctx = sidtab_search(&crm->sidtab, xfrm_sid);
if (!xfrm_ctx) {
printk(KERN_ERR "SELinux: %s: unrecognized SID %d\n",
__func__, xfrm_sid);
@@ -3326,7 +3334,7 @@ int selinux_audit_rule_match(u32 sid, u32 field, u32 op, void *vrule,
goto out;
}

- ctxt = sidtab_search(&sidtab, sid);
+ ctxt = sidtab_search(&crm->sidtab, sid);
if (unlikely(!ctxt)) {
WARN_ONCE(1, "selinux_audit_rule_match: unrecognized SID %d\n",
sid);
@@ -3504,7 +3512,7 @@ int security_netlbl_secattr_to_sid(struct netlbl_lsm_secattr *secattr,
*sid = secattr->attr.secid;
else if (secattr->flags & NETLBL_SECATTR_MLS_LVL) {
rc = -EIDRM;
- ctx = sidtab_search(&sidtab, SECINITSID_NETMSG);
+ ctx = sidtab_search(&crm->sidtab, SECINITSID_NETMSG);
if (ctx == NULL)
goto out;

@@ -3523,7 +3531,7 @@ int security_netlbl_secattr_to_sid(struct netlbl_lsm_secattr *secattr,
if (!mls_context_isvalid(&crm->policydb, &ctx_new))
goto out_free;

- rc = sidtab_context_to_sid(&sidtab, &ctx_new, sid);
+ rc = sidtab_context_to_sid(&crm->sidtab, &ctx_new, sid);
if (rc)
goto out_free;

@@ -3563,7 +3571,7 @@ int security_netlbl_sid_to_secattr(u32 sid, struct netlbl_lsm_secattr *secattr)
read_lock(&policy_rwlock);

rc = -ENOENT;
- ctx = sidtab_search(&sidtab, sid);
+ ctx = sidtab_search(&crm->sidtab, sid);
if (ctx == NULL)
goto out;

--
2.7.4


2018-01-26 14:35:02

by peter enderborg

[permalink] [raw]
Subject: [PATCH v2 4/5] selinux: Use pointer to switch policydb and sidtab

From: Peter Enderborg <[email protected]>

This i preparation for switching to RCU locks. To be able to use
RCU we need atomic switched pointer. This adds the dynamic
memory copying to be a single pointer. It copy all the
data structures in to new ones. This is an overhead
for writing rules but the benifit is RCU.

Signed-off-by: Peter Enderborg <[email protected]>
---
security/selinux/ss/services.c | 139 +++++++++++++++++++++++------------------
1 file changed, 78 insertions(+), 61 deletions(-)

diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index 2a8486c..81c5717 100644
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
@@ -2064,76 +2064,67 @@ static int security_preserve_bools(struct policydb *p);
*/
int security_load_policy(void *data, size_t len)
{
- struct policydb *oldpolicydb, *newpolicydb;
+ struct policydb *oldpolicydb;
struct sidtab oldsidtab, newsidtab;
struct selinux_mapping *oldmap = NULL, *map = NULL;
struct convert_context_args args;
- struct shared_current_mapping *new_mapping;
struct shared_current_mapping *next_rcu;
-
+ struct shared_current_mapping *old_rcu;
u32 seqno;
u16 map_size;
int rc = 0;
struct policy_file file = { data, len }, *fp = &file;

- oldpolicydb = kzalloc(2 * sizeof(*oldpolicydb), GFP_KERNEL);
- if (!oldpolicydb) {
- rc = -ENOMEM;
- goto out;
- }
- new_mapping = kzalloc(sizeof(struct shared_current_mapping),
- GFP_KERNEL);
- if (!new_mapping) {
- rc = -ENOMEM;
- goto out;
- }
- newpolicydb = oldpolicydb + 1;
- next_rcu = kmalloc(sizeof(struct shared_current_mapping), GFP_KERNEL);
- if (!next_rcu) {
- rc = -ENOMEM;
- goto out;
- }
-
if (!ss_initialized) {
- crm = kzalloc(sizeof(struct shared_current_mapping),
- GFP_KERNEL);
- if (!crm) {
+ struct shared_current_mapping *first_mapping;
+
+ first_mapping = kzalloc(sizeof(struct shared_current_mapping),
+ GFP_KERNEL);
+ if (!first_mapping) {
rc = -ENOMEM;
goto out;
}
avtab_cache_init();
ebitmap_cache_init();
hashtab_cache_init();
- rc = policydb_read(&crm->policydb, fp);
+ rc = policydb_read(&first_mapping->policydb, fp);
if (rc) {
avtab_cache_destroy();
ebitmap_cache_destroy();
hashtab_cache_destroy();
+ kfree(first_mapping);
goto out;
}

- crm->policydb.len = len;
- rc = selinux_set_mapping(&crm->policydb, secclass_map,
- &crm->current_mapping,
- &crm->current_mapping_size);
+ first_mapping->policydb.len = len;
+ rc = selinux_set_mapping(&first_mapping->policydb, secclass_map,
+ &first_mapping->current_mapping,
+ &first_mapping->current_mapping_size);
if (rc) {
- policydb_destroy(&crm->policydb);
+ policydb_destroy(&first_mapping->policydb);
avtab_cache_destroy();
ebitmap_cache_destroy();
hashtab_cache_destroy();
+ kfree(first_mapping);
goto out;
}

- rc = policydb_load_isids(&crm->policydb, &crm->sidtab);
+ rc = policydb_load_isids(&first_mapping->policydb,
+ &first_mapping->sidtab);
if (rc) {
- policydb_destroy(&crm->policydb);
+ policydb_destroy(&first_mapping->policydb);
avtab_cache_destroy();
ebitmap_cache_destroy();
hashtab_cache_destroy();
+ kfree(first_mapping);
goto out;
}

- security_load_policycaps(&crm->policydb);
+ security_load_policycaps(&first_mapping->policydb);
+ crm = first_mapping;
+
+ smp_mb(); /* make sure that crm exist before we */
+ /* switch ss_initialized */
ss_initialized = 1;
seqno = ++latest_granting;
selinux_complete_init();
@@ -2148,30 +2139,44 @@ int security_load_policy(void *data, size_t len)
#if 0
sidtab_hash_eval(&crm->sidtab, "sids");
#endif
+ oldpolicydb = kzalloc(sizeof(*oldpolicydb), GFP_KERNEL);
+ if (!oldpolicydb) {
+ rc = -ENOMEM;
+ goto out;
+ }
+
+ next_rcu = kzalloc(sizeof(struct shared_current_mapping), GFP_KERNEL);
+ if (!next_rcu) {
+ kfree(oldpolicydb);
+ rc = -ENOMEM;
+ goto out;
+ }

- rc = policydb_read(newpolicydb, fp);
+ rc = policydb_read(&next_rcu->policydb, fp);
if (rc)
goto out;

- newpolicydb->len = len;
+ next_rcu->policydb.len = len;
+ read_lock(&policy_rwlock);
/* If switching between different policy types, log MLS status */
- if (crm->policydb.mls_enabled && !newpolicydb->mls_enabled)
+ if (crm->policydb.mls_enabled && !next_rcu->policydb.mls_enabled)
printk(KERN_INFO "SELinux: Disabling MLS support...\n");
- else if (!crm->policydb.mls_enabled && newpolicydb->mls_enabled)
+ else if (!crm->policydb.mls_enabled && next_rcu->policydb.mls_enabled)
printk(KERN_INFO "SELinux: Enabling MLS support...\n");

- rc = policydb_load_isids(newpolicydb, &newsidtab);
+ rc = policydb_load_isids(&next_rcu->policydb, &newsidtab);
if (rc) {
printk(KERN_ERR "SELinux: unable to load the initial SIDs\n");
- policydb_destroy(newpolicydb);
+ policydb_destroy(&next_rcu->policydb);
goto out;
}

- rc = selinux_set_mapping(newpolicydb, secclass_map, &map, &map_size);
+ rc = selinux_set_mapping(&next_rcu->policydb, secclass_map,
+ &map, &map_size);
if (rc)
goto err;

- rc = security_preserve_bools(newpolicydb);
+ rc = security_preserve_bools(&next_rcu->policydb);
if (rc) {
printk(KERN_ERR "SELinux: unable to preserve booleans\n");
goto err;
@@ -2189,7 +2194,7 @@ int security_load_policy(void *data, size_t len)
* in the new SID table.
*/
args.oldp = &crm->policydb;
- args.newp = newpolicydb;
+ args.newp = &next_rcu->policydb;
rc = sidtab_map(&newsidtab, convert_context, &args);
if (rc) {
printk(KERN_ERR "SELinux: unable to convert the internal"
@@ -2204,8 +2209,9 @@ int security_load_policy(void *data, size_t len)

/* Install the new policydb and SID table. */
/* next */
+ security_load_policycaps(&next_rcu->policydb);
+ read_unlock(&policy_rwlock);
write_lock_irq(&policy_rwlock);
- memcpy(&next_rcu->policydb, newpolicydb, sizeof(struct policydb));
sidtab_set(&next_rcu->sidtab, &newsidtab);
security_load_policycaps(&next_rcu->policydb);
oldmap = crm->current_mapping;
@@ -2213,8 +2219,9 @@ int security_load_policy(void *data, size_t len)
next_rcu->current_mapping_size = map_size;

seqno = ++latest_granting;
- write_unlock_irq(&policy_rwlock);
+ old_rcu = crm;
crm = next_rcu;
+ write_unlock_irq(&policy_rwlock);

/* Free the old policydb and SID table. */
policydb_destroy(oldpolicydb);
@@ -2226,17 +2233,16 @@ int security_load_policy(void *data, size_t len)
selinux_status_update_policyload(seqno);
selinux_netlbl_cache_invalidate();
selinux_xfrm_notify_policyload();
+ kfree(oldpolicydb);
+ kfree(old_rcu);

rc = 0;
goto out;
-
err:
kfree(map);
sidtab_destroy(&newsidtab);
- policydb_destroy(newpolicydb);
-
+ policydb_destroy(&next_rcu->policydb);
out:
- kfree(oldpolicydb);
return rc;
}

@@ -2795,54 +2801,65 @@ int security_get_bools(int *len, char ***names, int **values)
goto out;
}

-
int security_set_bools(int len, int *values)
{
+ struct shared_current_mapping *next_rcu, *old_rcu;
int i, rc;
int lenp, seqno = 0;
struct cond_node *cur;

- write_lock_irq(&policy_rwlock);
-
+ next_rcu = kzalloc(sizeof(struct shared_current_mapping), GFP_KERNEL);
+ read_lock(&policy_rwlock);
+ old_rcu = crm;
+ memcpy(&next_rcu->policydb, &old_rcu->policydb,
+ sizeof(struct policydb));
rc = -EFAULT;
- lenp = crm->policydb.p_bools.nprim;
+ lenp = next_rcu->policydb.p_bools.nprim;
+
if (len != lenp)
goto out;

for (i = 0; i < len; i++) {
if (!!values[i] !=
- crm->policydb.bool_val_to_struct[i]->state) {
+ next_rcu->policydb.bool_val_to_struct[i]->state) {
audit_log(current->audit_context, GFP_ATOMIC,
AUDIT_MAC_CONFIG_CHANGE,
"bool=%s val=%d old_val=%d auid=%u ses=%u",
- sym_name(&crm->policydb, SYM_BOOLS, i),
+ sym_name(&next_rcu->policydb, SYM_BOOLS, i),
!!values[i],
- crm->policydb.bool_val_to_struct[i]->state,
+ next_rcu->policydb.bool_val_to_struct[i]->state,
from_kuid(&init_user_ns, audit_get_loginuid(current)),
audit_get_sessionid(current));
}
if (values[i])
- crm->policydb.bool_val_to_struct[i]->state = 1;
+ next_rcu->policydb.bool_val_to_struct[i]->state = 1;
else
- crm->policydb.bool_val_to_struct[i]->state = 0;
+ next_rcu->policydb.bool_val_to_struct[i]->state = 0;
}

- for (cur = crm->policydb.cond_list; cur; cur = cur->next) {
- rc = evaluate_cond_node(&crm->policydb, cur);
+ for (cur = next_rcu->policydb.cond_list; cur; cur = cur->next) {
+ rc = evaluate_cond_node(&next_rcu->policydb, cur);
if (rc)
goto out;
}
+ read_unlock(&policy_rwlock);
+ rc = 0;

+ write_lock_irq(&policy_rwlock);
seqno = ++latest_granting;
- rc = 0;
-out:
+ crm = next_rcu;
write_unlock_irq(&policy_rwlock);
+out:
if (!rc) {
avc_ss_reset(seqno);
selnl_notify_policyload(seqno);
selinux_status_update_policyload(seqno);
selinux_xfrm_notify_policyload();
+ } else {
+ kfree(next_rcu);
}
+ kfree(old_rcu);
+
return rc;
}

--
2.7.4


2018-01-26 14:35:07

by peter enderborg

[permalink] [raw]
Subject: [PATCH v2 5/5] selinux: Switch locking to RCU.

From: Peter Enderborg <[email protected]>

This patch switch to using RCU locks instead of rwlocks. This has
the big advantage that it does not has preempt disable.

Signed-off-by: Peter Enderborg <[email protected]>
Reported-by: Björn Davidsson <[email protected]>
---
security/selinux/ss/services.c | 162 +++++++++++++++++++++--------------------
1 file changed, 82 insertions(+), 80 deletions(-)

diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index 81c5717..f142ef8 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);
+static DEFINE_SPINLOCK(policy_w_lock);

int ss_initialized;

@@ -115,14 +115,14 @@ struct selinux_mapping {
u32 perms[sizeof(u32) * 8];
};

-struct shared_current_mapping {
+struct shared_rcu_mapping {
struct selinux_mapping *current_mapping;
u16 current_mapping_size;
struct policydb policydb;
struct sidtab sidtab;
};

-static struct shared_current_mapping *crm;
+static struct shared_rcu_mapping *crm;

static int selinux_set_mapping(struct policydb *pol,
struct security_class_mapping *map,
@@ -791,7 +791,7 @@ static int security_compute_validatetrans(u32 oldsid, u32 newsid, u32 tasksid,
if (!ss_initialized)
return 0;

- read_lock(&policy_rwlock);
+ rcu_read_lock();

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

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

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

- read_lock(&policy_rwlock);
+ rcu_read_lock();

rc = -EINVAL;
old_context = sidtab_search(&crm->sidtab, old_sid);
@@ -941,7 +941,7 @@ int security_bounded_transition(u32 old_sid, u32 new_sid)
kfree(old_name);
}
out:
- read_unlock(&policy_rwlock);
+ rcu_read_unlock();

return rc;
}
@@ -1029,7 +1029,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);
+ rcu_read_lock();
if (!ss_initialized)
goto allow;

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

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

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

context_struct_compute_av(scontext, tcontext, tclass, avd, NULL);
out:
- read_unlock(&policy_rwlock);
+ rcu_read_unlock();
return;
allow:
avd->allowed = 0xffffffff;
@@ -1293,7 +1293,7 @@ static int security_sid_to_context_core(u32 sid, char **scontext,
rc = -EINVAL;
goto out;
}
- read_lock(&policy_rwlock);
+ rcu_read_lock();
if (force)
context = sidtab_search_force(&crm->sidtab, sid);
else
@@ -1306,7 +1306,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);
+ rcu_read_unlock();
out:
return rc;

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

- read_lock(&policy_rwlock);
+ rcu_read_lock();
rc = string_to_context_struct(&crm->policydb, &crm->sidtab, scontext2,
scontext_len, &context, def_sid);
if (rc == -EINVAL && force) {
@@ -1470,7 +1470,7 @@ static int security_context_to_sid_core(const char *scontext, u32 scontext_len,
rc = sidtab_context_to_sid(&crm->sidtab, &context, sid);
context_destroy(&context);
out_unlock:
- read_unlock(&policy_rwlock);
+ rcu_read_unlock();
out:
kfree(scontext2);
kfree(str);
@@ -1620,7 +1620,7 @@ static int security_compute_sid(u32 ssid,

context_init(&newcontext);

- read_lock(&policy_rwlock);
+ rcu_read_lock();

if (kern) {
tclass = unmap_class(orig_tclass);
@@ -1759,7 +1759,7 @@ static int security_compute_sid(u32 ssid,
/* Obtain the sid for the context. */
rc = sidtab_context_to_sid(&crm->sidtab, &newcontext, out_sid);
out_unlock:
- read_unlock(&policy_rwlock);
+ rcu_read_unlock();
context_destroy(&newcontext);
out:
return rc;
@@ -2051,7 +2051,6 @@ static void security_load_policycaps(struct policydb *pdb)
}

static int security_preserve_bools(struct policydb *p);
-
/**
* security_load_policy - Load a security policy configuration.
* @data: binary policy data
@@ -2068,17 +2067,17 @@ int security_load_policy(void *data, size_t len)
struct sidtab oldsidtab, newsidtab;
struct selinux_mapping *oldmap = NULL, *map = NULL;
struct convert_context_args args;
- struct shared_current_mapping *next_rcu;
- struct shared_current_mapping *old_rcu;
+ struct shared_rcu_mapping *next_rcu;
+ struct shared_rcu_mapping *old_rcu;
u32 seqno;
u16 map_size;
int rc = 0;
struct policy_file file = { data, len }, *fp = &file;

if (!ss_initialized) {
- struct shared_current_mapping *first_mapping;
+ struct shared_rcu_mapping *first_mapping;

- first_mapping = kzalloc(sizeof(struct shared_current_mapping),
+ first_mapping = kzalloc(sizeof(struct shared_rcu_mapping),
GFP_KERNEL);
if (!first_mapping) {
rc = -ENOMEM;
@@ -2145,7 +2144,7 @@ int security_load_policy(void *data, size_t len)
goto out;
}

- next_rcu = kzalloc(sizeof(struct shared_current_mapping), GFP_KERNEL);
+ next_rcu = kzalloc(sizeof(struct shared_rcu_mapping), GFP_KERNEL);
if (!next_rcu) {
kfree(oldpolicydb);
rc = -ENOMEM;
@@ -2157,7 +2156,7 @@ int security_load_policy(void *data, size_t len)
goto out;

next_rcu->policydb.len = len;
- read_lock(&policy_rwlock);
+ rcu_read_lock();
/* If switching between different policy types, log MLS status */
if (crm->policydb.mls_enabled && !next_rcu->policydb.mls_enabled)
printk(KERN_INFO "SELinux: Disabling MLS support...\n");
@@ -2210,19 +2209,19 @@ int security_load_policy(void *data, size_t len)
/* Install the new policydb and SID table. */
/* next */
security_load_policycaps(&next_rcu->policydb);
- read_unlock(&policy_rwlock);
- write_lock_irq(&policy_rwlock);
sidtab_set(&next_rcu->sidtab, &newsidtab);
security_load_policycaps(&next_rcu->policydb);
oldmap = crm->current_mapping;
+ rcu_read_unlock();
next_rcu->current_mapping = map;
next_rcu->current_mapping_size = map_size;

+ spin_lock(&policy_w_lock);
seqno = ++latest_granting;
old_rcu = crm;
crm = next_rcu;
- write_unlock_irq(&policy_rwlock);
-
+ spin_unlock(&policy_w_lock);
+ synchronize_rcu();
/* Free the old policydb and SID table. */
policydb_destroy(oldpolicydb);
sidtab_destroy(&oldsidtab);
@@ -2250,9 +2249,9 @@ size_t security_policydb_len(void)
{
size_t len;

- read_lock(&policy_rwlock);
+ rcu_read_lock();
len = crm->policydb.len;
- read_unlock(&policy_rwlock);
+ rcu_read_unlock();

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

- read_lock(&policy_rwlock);
+ rcu_read_lock();

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

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

@@ -2308,7 +2307,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);
+ rcu_read_lock();

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

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

@@ -2348,7 +2347,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);
+ rcu_read_lock();

c = crm->policydb.ocontexts[OCON_IBENDPORT];
while (c) {
@@ -2374,7 +2373,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);
+ rcu_read_unlock();
return rc;
}

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

- read_lock(&policy_rwlock);
+ rcu_read_lock();

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

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

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

- read_lock(&policy_rwlock);
+ rcu_read_lock();

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

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

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

- read_lock(&policy_rwlock);
+ rcu_read_lock();

context_init(&usercon);

@@ -2593,7 +2592,7 @@ int security_get_user_sids(u32 fromsid,
}
rc = 0;
out_unlock:
- read_unlock(&policy_rwlock);
+ rcu_read_unlock();
if (rc || !mynel) {
kfree(mysids);
goto out;
@@ -2704,9 +2703,9 @@ int security_genfs_sid(const char *fstype,
{
int retval;

- read_lock(&policy_rwlock);
+ rcu_read_lock();
retval = __security_genfs_sid(fstype, path, orig_sclass, sid);
- read_unlock(&policy_rwlock);
+ rcu_read_unlock();
return retval;
}

@@ -2721,7 +2720,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);
+ rcu_read_lock();

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

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

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

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

@@ -2790,7 +2789,7 @@ int security_get_bools(int *len, char ***names, int **values)
}
rc = 0;
out:
- read_unlock(&policy_rwlock);
+ rcu_read_unlock();
return rc;
err:
if (*names) {
@@ -2803,13 +2802,14 @@ int security_get_bools(int *len, char ***names, int **values)

int security_set_bools(int len, int *values)
{
- struct shared_current_mapping *next_rcu, *old_rcu;
+
+ struct shared_rcu_mapping *next_rcu, *old_rcu;
int i, rc;
int lenp, seqno = 0;
struct cond_node *cur;

- next_rcu = kzalloc(sizeof(struct shared_current_mapping), GFP_KERNEL);
- read_lock(&policy_rwlock);
+ next_rcu = kzalloc(sizeof(struct shared_rcu_mapping), GFP_KERNEL);
+ rcu_read_lock();
old_rcu = crm;
memcpy(&next_rcu->policydb, &old_rcu->policydb,
sizeof(struct policydb));
@@ -2842,13 +2842,15 @@ int security_set_bools(int len, int *values)
if (rc)
goto out;
}
- read_unlock(&policy_rwlock);
+ rcu_read_unlock();
+
rc = 0;

- write_lock_irq(&policy_rwlock);
+ spin_lock(&policy_w_lock);
seqno = ++latest_granting;
crm = next_rcu;
- write_unlock_irq(&policy_rwlock);
+ spin_unlock(&policy_w_lock);
+ synchronize_rcu();
out:
if (!rc) {
avc_ss_reset(seqno);
@@ -2868,7 +2870,7 @@ int security_get_bool_value(int index)
int rc;
int len;

- read_lock(&policy_rwlock);
+ rcu_read_lock();

rc = -EFAULT;
len = crm->policydb.p_bools.nprim;
@@ -2877,7 +2879,7 @@ int security_get_bool_value(int index)

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

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

context_init(&newcon);

- read_lock(&policy_rwlock);
+ rcu_read_lock();

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

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

- read_lock(&policy_rwlock);
+ rcu_read_lock();

rc = -EINVAL;
nlbl_ctx = sidtab_search(&crm->sidtab, nlbl_sid);
@@ -3059,7 +3061,7 @@ int security_net_peersid_resolve(u32 nlbl_sid, u32 nlbl_type,
* expressive */
*peer_sid = xfrm_sid;
out:
- read_unlock(&policy_rwlock);
+ rcu_read_unlock();
return rc;
}

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

- read_lock(&policy_rwlock);
+ rcu_read_lock();

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

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

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

- read_lock(&policy_rwlock);
+ rcu_read_lock();

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

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

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

- read_lock(&policy_rwlock);
+ rcu_read_lock();
rc = ebitmap_get_bit(&crm->policydb.policycaps, req_cap);
- read_unlock(&policy_rwlock);
+ rcu_read_unlock();

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

context_init(&tmprule->au_ctxt);

- read_lock(&policy_rwlock);
+ rcu_read_lock();

tmprule->au_seqno = latest_granting;

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

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

- read_lock(&policy_rwlock);
+ rcu_read_lock();

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

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

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

- read_lock(&policy_rwlock);
+ rcu_read_lock();

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

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

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

- read_lock(&policy_rwlock);
+ rcu_read_lock();

rc = -ENOENT;
ctx = sidtab_search(&crm->sidtab, sid);
@@ -3604,7 +3606,7 @@ int security_netlbl_sid_to_secattr(u32 sid, struct netlbl_lsm_secattr *secattr)
mls_export_netlbl_lvl(&crm->policydb, ctx, secattr);
rc = mls_export_netlbl_cat(&crm->policydb, ctx, secattr);
out:
- read_unlock(&policy_rwlock);
+ rcu_read_unlock();
return rc;
}
#endif /* CONFIG_NETLABEL */
@@ -3632,9 +3634,9 @@ int security_read_policy(void **data, size_t *len)
fp.data = *data;
fp.len = *len;

- read_lock(&policy_rwlock);
+ rcu_read_lock();
rc = policydb_write(&crm->policydb, &fp);
- read_unlock(&policy_rwlock);
+ rcu_read_unlock();

if (rc)
return rc;
--
2.7.4


2018-01-26 14:35:28

by peter enderborg

[permalink] [raw]
Subject: [PATCH v2 2/5] selinux: Move policydb to pointer structure

From: Peter Enderborg <[email protected]>

To be able to use rcu locks we seed to address the policydb
though a pointer. This patch adds a pointer structure to
repleace the static policydb.

Signed-off-by: Peter Enderborg <[email protected]>
---
security/selinux/ss/services.c | 274 ++++++++++++++++++++++-------------------
1 file changed, 149 insertions(+), 125 deletions(-)

diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index 47d8030..21400bd 100644
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
@@ -90,7 +90,6 @@ int selinux_policycap_nnp_nosuid_transition;
static DEFINE_RWLOCK(policy_rwlock);

static struct sidtab sidtab;
-static struct policydb policydb;
int ss_initialized;

/*
@@ -120,6 +119,7 @@ struct selinux_mapping {
struct shared_current_mapping {
struct selinux_mapping *current_mapping;
u16 current_mapping_size;
+ struct policydb policydb;
};

static struct shared_current_mapping *crm;
@@ -277,7 +277,7 @@ static void map_decision(u16 tclass, struct av_decision *avd,

int security_mls_enabled(void)
{
- return policydb.mls_enabled;
+ return crm->policydb.mls_enabled;
}

/*
@@ -335,8 +335,8 @@ static int constraint_expr_eval(struct context *scontext,
case CEXPR_ROLE:
val1 = scontext->role;
val2 = tcontext->role;
- r1 = policydb.role_val_to_struct[val1 - 1];
- r2 = policydb.role_val_to_struct[val2 - 1];
+ r1 = crm->policydb.role_val_to_struct[val1 - 1];
+ r2 = crm->policydb.role_val_to_struct[val2 - 1];
switch (e->op) {
case CEXPR_DOM:
s[++sp] = ebitmap_get_bit(&r1->dominates,
@@ -501,8 +501,8 @@ static void security_dump_masked_av(struct context *scontext,
if (!permissions)
return;

- tclass_name = sym_name(&policydb, SYM_CLASSES, tclass - 1);
- tclass_dat = policydb.class_val_to_struct[tclass - 1];
+ tclass_name = sym_name(&crm->policydb, SYM_CLASSES, tclass - 1);
+ tclass_dat = crm->policydb.class_val_to_struct[tclass - 1];
common_dat = tclass_dat->comdatum;

/* init permission_names */
@@ -571,14 +571,14 @@ static void type_attribute_bounds_av(struct context *scontext,
struct type_datum *target;
u32 masked = 0;

- source = flex_array_get_ptr(policydb.type_val_to_struct_array,
+ source = flex_array_get_ptr(crm->policydb.type_val_to_struct_array,
scontext->type - 1);
BUG_ON(!source);

if (!source->bounds)
return;

- target = flex_array_get_ptr(policydb.type_val_to_struct_array,
+ target = flex_array_get_ptr(crm->policydb.type_val_to_struct_array,
tcontext->type - 1);
BUG_ON(!target);

@@ -664,13 +664,13 @@ static void context_struct_compute_av(struct context *scontext,
xperms->len = 0;
}

- if (unlikely(!tclass || tclass > policydb.p_classes.nprim)) {
+ if (unlikely(!tclass || tclass > crm->policydb.p_classes.nprim)) {
if (printk_ratelimit())
printk(KERN_WARNING "SELinux: Invalid class %hu\n", tclass);
return;
}

- tclass_datum = policydb.class_val_to_struct[tclass - 1];
+ tclass_datum = crm->policydb.class_val_to_struct[tclass - 1];

/*
* If a specific type enforcement rule was defined for
@@ -678,15 +678,18 @@ static void context_struct_compute_av(struct context *scontext,
*/
avkey.target_class = tclass;
avkey.specified = AVTAB_AV | AVTAB_XPERMS;
- sattr = flex_array_get(policydb.type_attr_map_array, scontext->type - 1);
+ sattr = flex_array_get(crm->policydb.type_attr_map_array,
+ scontext->type - 1);
BUG_ON(!sattr);
- tattr = flex_array_get(policydb.type_attr_map_array, tcontext->type - 1);
+ tattr = flex_array_get(crm->policydb.type_attr_map_array,
+ tcontext->type - 1);
BUG_ON(!tattr);
ebitmap_for_each_positive_bit(sattr, snode, i) {
ebitmap_for_each_positive_bit(tattr, tnode, j) {
avkey.source_type = i + 1;
avkey.target_type = j + 1;
- for (node = avtab_search_node(&policydb.te_avtab, &avkey);
+ for (node = avtab_search_node(&crm->policydb.te_avtab,
+ &avkey);
node;
node = avtab_search_node_next(node, avkey.specified)) {
if (node->key.specified == AVTAB_ALLOWED)
@@ -700,7 +703,7 @@ static void context_struct_compute_av(struct context *scontext,
}

/* Check conditional av table for additional permissions */
- cond_compute_av(&policydb.te_cond_avtab, &avkey,
+ cond_compute_av(&crm->policydb.te_cond_avtab, &avkey,
avd, xperms);

}
@@ -725,16 +728,16 @@ static void context_struct_compute_av(struct context *scontext,
* role is changing, then check the (current_role, new_role)
* pair.
*/
- if (tclass == policydb.process_class &&
- (avd->allowed & policydb.process_trans_perms) &&
+ if (tclass == crm->policydb.process_class &&
+ (avd->allowed & crm->policydb.process_trans_perms) &&
scontext->role != tcontext->role) {
- for (ra = policydb.role_allow; ra; ra = ra->next) {
+ for (ra = crm->policydb.role_allow; ra; ra = ra->next) {
if (scontext->role == ra->role &&
tcontext->role == ra->new_role)
break;
}
if (!ra)
- avd->allowed &= ~policydb.process_trans_perms;
+ avd->allowed &= ~crm->policydb.process_trans_perms;
}

/*
@@ -763,7 +766,7 @@ static int security_validtrans_handle_fail(struct context *ocontext,
audit_log(current->audit_context, GFP_ATOMIC, AUDIT_SELINUX_ERR,
"op=security_validate_transition seresult=denied"
" oldcontext=%s newcontext=%s taskcontext=%s tclass=%s",
- o, n, t, sym_name(&policydb, SYM_CLASSES, tclass-1));
+ o, n, t, sym_name(&crm->policydb, SYM_CLASSES, tclass-1));
out:
kfree(o);
kfree(n);
@@ -795,11 +798,11 @@ static int security_compute_validatetrans(u32 oldsid, u32 newsid, u32 tasksid,
else
tclass = orig_tclass;

- if (!tclass || tclass > policydb.p_classes.nprim) {
+ if (!tclass || tclass > crm->policydb.p_classes.nprim) {
rc = -EINVAL;
goto out;
}
- tclass_datum = policydb.class_val_to_struct[tclass - 1];
+ tclass_datum = crm->policydb.class_val_to_struct[tclass - 1];

ocontext = sidtab_search(&sidtab, oldsid);
if (!ocontext) {
@@ -901,7 +904,7 @@ int security_bounded_transition(u32 old_sid, u32 new_sid)

index = new_context->type;
while (true) {
- type = flex_array_get_ptr(policydb.type_val_to_struct_array,
+ type = flex_array_get_ptr(crm->policydb.type_val_to_struct_array,
index - 1);
BUG_ON(!type);

@@ -1046,35 +1049,36 @@ void security_compute_xperms_decision(u32 ssid,

tclass = unmap_class(orig_tclass);
if (unlikely(orig_tclass && !tclass)) {
- if (policydb.allow_unknown)
+ if (crm->policydb.allow_unknown)
goto allow;
goto out;
}


- if (unlikely(!tclass || tclass > policydb.p_classes.nprim)) {
+ if (unlikely(!tclass || tclass > crm->policydb.p_classes.nprim)) {
pr_warn_ratelimited("SELinux: Invalid class %hu\n", tclass);
goto out;
}

avkey.target_class = tclass;
avkey.specified = AVTAB_XPERMS;
- sattr = flex_array_get(policydb.type_attr_map_array,
+ sattr = flex_array_get(crm->policydb.type_attr_map_array,
scontext->type - 1);
BUG_ON(!sattr);
- tattr = flex_array_get(policydb.type_attr_map_array,
+ tattr = flex_array_get(crm->policydb.type_attr_map_array,
tcontext->type - 1);
BUG_ON(!tattr);
ebitmap_for_each_positive_bit(sattr, snode, i) {
ebitmap_for_each_positive_bit(tattr, tnode, j) {
avkey.source_type = i + 1;
avkey.target_type = j + 1;
- for (node = avtab_search_node(&policydb.te_avtab, &avkey);
+ for (node = avtab_search_node(&crm->policydb.te_avtab,
+ &avkey);
node;
node = avtab_search_node_next(node, avkey.specified))
services_compute_xperms_decision(xpermd, node);

- cond_compute_xperms(&policydb.te_cond_avtab,
+ cond_compute_xperms(&crm->policydb.te_cond_avtab,
&avkey, xpermd);
}
}
@@ -1120,7 +1124,7 @@ void security_compute_av(u32 ssid,
}

/* permissive domain? */
- if (ebitmap_get_bit(&policydb.permissive_map, scontext->type))
+ if (ebitmap_get_bit(&crm->policydb.permissive_map, scontext->type))
avd->flags |= AVD_FLAGS_PERMISSIVE;

tcontext = sidtab_search(&sidtab, tsid);
@@ -1132,12 +1136,12 @@ void security_compute_av(u32 ssid,

tclass = unmap_class(orig_tclass);
if (unlikely(orig_tclass && !tclass)) {
- if (policydb.allow_unknown)
+ if (crm->policydb.allow_unknown)
goto allow;
goto out;
}
context_struct_compute_av(scontext, tcontext, tclass, avd, xperms);
- map_decision(orig_tclass, avd, policydb.allow_unknown);
+ map_decision(orig_tclass, avd, crm->policydb.allow_unknown);
out:
read_unlock(&policy_rwlock);
return;
@@ -1166,7 +1170,7 @@ void security_compute_av_user(u32 ssid,
}

/* permissive domain? */
- if (ebitmap_get_bit(&policydb.permissive_map, scontext->type))
+ if (ebitmap_get_bit(&crm->policydb.permissive_map, scontext->type))
avd->flags |= AVD_FLAGS_PERMISSIVE;

tcontext = sidtab_search(&sidtab, tsid);
@@ -1177,7 +1181,7 @@ void security_compute_av_user(u32 ssid,
}

if (unlikely(!tclass)) {
- if (policydb.allow_unknown)
+ if (crm->policydb.allow_unknown)
goto allow;
goto out;
}
@@ -1217,10 +1221,13 @@ static int context_struct_to_string(struct context *context, char **scontext, u3
}

/* Compute the size of the context. */
- *scontext_len += strlen(sym_name(&policydb, SYM_USERS, context->user - 1)) + 1;
- *scontext_len += strlen(sym_name(&policydb, SYM_ROLES, context->role - 1)) + 1;
- *scontext_len += strlen(sym_name(&policydb, SYM_TYPES, context->type - 1)) + 1;
- *scontext_len += mls_compute_context_len(&policydb, context);
+ *scontext_len += strlen(sym_name(&crm->policydb, SYM_USERS,
+ context->user - 1)) + 1;
+ *scontext_len += strlen(sym_name(&crm->policydb, SYM_ROLES,
+ context->role - 1)) + 1;
+ *scontext_len += strlen(sym_name(&crm->policydb, SYM_TYPES,
+ context->type - 1)) + 1;
+ *scontext_len += mls_compute_context_len(&crm->policydb, context);

if (!scontext)
return 0;
@@ -1235,11 +1242,11 @@ static int context_struct_to_string(struct context *context, char **scontext, u3
* Copy the user name, role name and type name into the context.
*/
scontextp += sprintf(scontextp, "%s:%s:%s",
- sym_name(&policydb, SYM_USERS, context->user - 1),
- sym_name(&policydb, SYM_ROLES, context->role - 1),
- sym_name(&policydb, SYM_TYPES, context->type - 1));
+ sym_name(&crm->policydb, SYM_USERS, context->user - 1),
+ sym_name(&crm->policydb, SYM_ROLES, context->role - 1),
+ sym_name(&crm->policydb, SYM_TYPES, context->type - 1));

- mls_sid_to_context(&policydb, context, &scontextp);
+ mls_sid_to_context(&crm->policydb, context, &scontextp);

*scontextp = 0;

@@ -1452,7 +1459,7 @@ static int security_context_to_sid_core(const char *scontext, u32 scontext_len,
}

read_lock(&policy_rwlock);
- rc = string_to_context_struct(&policydb, &sidtab, scontext2,
+ rc = string_to_context_struct(&crm->policydb, &sidtab, scontext2,
scontext_len, &context, def_sid);
if (rc == -EINVAL && force) {
context.str = str;
@@ -1546,7 +1553,7 @@ static int compute_sid_handle_invalid_context(
" scontext=%s"
" tcontext=%s"
" tclass=%s",
- n, s, t, sym_name(&policydb, SYM_CLASSES, tclass-1));
+ n, s, t, sym_name(&crm->policydb, SYM_CLASSES, tclass-1));
out:
kfree(s);
kfree(t);
@@ -1638,8 +1645,8 @@ static int security_compute_sid(u32 ssid,
goto out_unlock;
}

- if (tclass && tclass <= policydb.p_classes.nprim)
- cladatum = policydb.class_val_to_struct[tclass - 1];
+ if (tclass && tclass <= crm->policydb.p_classes.nprim)
+ cladatum = crm->policydb.class_val_to_struct[tclass - 1];

/* Set the user identity. */
switch (specified) {
@@ -1665,7 +1672,7 @@ static int security_compute_sid(u32 ssid,
} else if (cladatum && cladatum->default_role == DEFAULT_TARGET) {
newcontext.role = tcontext->role;
} else {
- if ((tclass == policydb.process_class) || (sock == true))
+ if ((tclass == crm->policydb.process_class) || (sock == true))
newcontext.role = scontext->role;
else
newcontext.role = OBJECT_R_VAL;
@@ -1677,7 +1684,8 @@ static int security_compute_sid(u32 ssid,
} else if (cladatum && cladatum->default_type == DEFAULT_TARGET) {
newcontext.type = tcontext->type;
} else {
- if ((tclass == policydb.process_class) || (sock == true)) {
+ if ((tclass == crm->policydb.process_class) ||
+ (sock == true)) {
/* Use the type of process. */
newcontext.type = scontext->type;
} else {
@@ -1691,11 +1699,12 @@ static int security_compute_sid(u32 ssid,
avkey.target_type = tcontext->type;
avkey.target_class = tclass;
avkey.specified = specified;
- avdatum = avtab_search(&policydb.te_avtab, &avkey);
+ avdatum = avtab_search(&crm->policydb.te_avtab, &avkey);

/* If no permanent rule, also check for enabled conditional rules */
if (!avdatum) {
- node = avtab_search_node(&policydb.te_cond_avtab, &avkey);
+ node = avtab_search_node(&crm->policydb.te_cond_avtab,
+ &avkey);
for (; node; node = avtab_search_node_next(node, specified)) {
if (node->key.specified & AVTAB_ENABLED) {
avdatum = &node->datum;
@@ -1711,13 +1720,16 @@ static int security_compute_sid(u32 ssid,

/* if we have a objname this is a file trans check so check those rules */
if (objname)
- filename_compute_type(&policydb, &newcontext, scontext->type,
- tcontext->type, tclass, objname);
+ filename_compute_type(&crm->policydb, &newcontext,
+ scontext->type, tcontext->type,
+ tclass, objname);

/* Check for class-specific changes. */
if (specified & AVTAB_TRANSITION) {
/* Look for a role transition rule. */
- for (roletr = policydb.role_tr; roletr; roletr = roletr->next) {
+ for (roletr = crm->policydb.role_tr;
+ roletr;
+ roletr = roletr->next) {
if ((roletr->role == scontext->role) &&
(roletr->type == tcontext->type) &&
(roletr->tclass == tclass)) {
@@ -1730,13 +1742,13 @@ static int security_compute_sid(u32 ssid,

/* Set the MLS attributes.
This is done last because it may allocate memory. */
- rc = mls_compute_sid(&policydb, scontext, tcontext, tclass, specified,
- &newcontext, sock);
+ rc = mls_compute_sid(&crm->policydb, scontext, tcontext, tclass,
+ specified, &newcontext, sock);
if (rc)
goto out_unlock;

/* Check the validity of the context. */
- if (!policydb_context_isvalid(&policydb, &newcontext)) {
+ if (!policydb_context_isvalid(&crm->policydb, &newcontext)) {
rc = compute_sid_handle_invalid_context(scontext,
tcontext,
tclass,
@@ -1944,7 +1956,8 @@ static int convert_context(u32 key,

/* Convert the MLS fields if dealing with MLS policies */
if (args->oldp->mls_enabled && args->newp->mls_enabled) {
- rc = mls_convert_context(&policydb, args->oldp, args->newp, c);
+ rc = mls_convert_context(&crm->policydb, args->oldp,
+ args->newp, c);
if (rc)
goto bad;
} else if (args->oldp->mls_enabled && !args->newp->mls_enabled) {
@@ -2009,27 +2022,31 @@ static void security_load_policycaps(void)
unsigned int i;
struct ebitmap_node *node;

- selinux_policycap_netpeer = ebitmap_get_bit(&policydb.policycaps,
- POLICYDB_CAPABILITY_NETPEER);
- selinux_policycap_openperm = ebitmap_get_bit(&policydb.policycaps,
- POLICYDB_CAPABILITY_OPENPERM);
- selinux_policycap_extsockclass = ebitmap_get_bit(&policydb.policycaps,
- POLICYDB_CAPABILITY_EXTSOCKCLASS);
- selinux_policycap_alwaysnetwork = ebitmap_get_bit(&policydb.policycaps,
- POLICYDB_CAPABILITY_ALWAYSNETWORK);
+ selinux_policycap_netpeer =
+ ebitmap_get_bit(&crm->policydb.policycaps,
+ POLICYDB_CAPABILITY_NETPEER);
+ selinux_policycap_openperm =
+ ebitmap_get_bit(&crm->policydb.policycaps,
+ POLICYDB_CAPABILITY_OPENPERM);
+ selinux_policycap_extsockclass =
+ ebitmap_get_bit(&crm->policydb.policycaps,
+ POLICYDB_CAPABILITY_EXTSOCKCLASS);
+ selinux_policycap_alwaysnetwork =
+ ebitmap_get_bit(&crm->policydb.policycaps,
+ POLICYDB_CAPABILITY_ALWAYSNETWORK);
selinux_policycap_cgroupseclabel =
- ebitmap_get_bit(&policydb.policycaps,
+ ebitmap_get_bit(&crm->policydb.policycaps,
POLICYDB_CAPABILITY_CGROUPSECLABEL);
selinux_policycap_nnp_nosuid_transition =
- ebitmap_get_bit(&policydb.policycaps,
+ ebitmap_get_bit(&crm->policydb.policycaps,
POLICYDB_CAPABILITY_NNP_NOSUID_TRANSITION);

for (i = 0; i < ARRAY_SIZE(selinux_policycap_names); i++)
pr_info("SELinux: policy capability %s=%d\n",
selinux_policycap_names[i],
- ebitmap_get_bit(&policydb.policycaps, i));
+ ebitmap_get_bit(&crm->policydb.policycaps, i));

- ebitmap_for_each_positive_bit(&policydb.policycaps, node, i) {
+ ebitmap_for_each_positive_bit(&crm->policydb.policycaps, node, i) {
if (i >= ARRAY_SIZE(selinux_policycap_names))
pr_info("SELinux: unknown policy capability %u\n",
i);
@@ -2084,7 +2101,7 @@ int security_load_policy(void *data, size_t len)
avtab_cache_init();
ebitmap_cache_init();
hashtab_cache_init();
- rc = policydb_read(&policydb, fp);
+ rc = policydb_read(&crm->policydb, fp);
if (rc) {
avtab_cache_destroy();
ebitmap_cache_destroy();
@@ -2092,21 +2109,21 @@ int security_load_policy(void *data, size_t len)
goto out;
}

- policydb.len = len;
- rc = selinux_set_mapping(&policydb, secclass_map,
+ crm->policydb.len = len;
+ rc = selinux_set_mapping(&crm->policydb, secclass_map,
&crm->current_mapping,
&crm->current_mapping_size);
if (rc) {
- policydb_destroy(&policydb);
+ policydb_destroy(&crm->policydb);
avtab_cache_destroy();
ebitmap_cache_destroy();
hashtab_cache_destroy();
goto out;
}

- rc = policydb_load_isids(&policydb, &sidtab);
+ rc = policydb_load_isids(&crm->policydb, &sidtab);
if (rc) {
- policydb_destroy(&policydb);
+ policydb_destroy(&crm->policydb);
avtab_cache_destroy();
ebitmap_cache_destroy();
hashtab_cache_destroy();
@@ -2135,9 +2152,9 @@ int security_load_policy(void *data, size_t len)

newpolicydb->len = len;
/* If switching between different policy types, log MLS status */
- if (policydb.mls_enabled && !newpolicydb->mls_enabled)
+ if (crm->policydb.mls_enabled && !newpolicydb->mls_enabled)
printk(KERN_INFO "SELinux: Disabling MLS support...\n");
- else if (!policydb.mls_enabled && newpolicydb->mls_enabled)
+ else if (!crm->policydb.mls_enabled && newpolicydb->mls_enabled)
printk(KERN_INFO "SELinux: Enabling MLS support...\n");

rc = policydb_load_isids(newpolicydb, &newsidtab);
@@ -2168,7 +2185,7 @@ int security_load_policy(void *data, size_t len)
* Convert the internal representations of contexts
* in the new SID table.
*/
- args.oldp = &policydb;
+ args.oldp = &crm->policydb;
args.newp = newpolicydb;
rc = sidtab_map(&newsidtab, convert_context, &args);
if (rc) {
@@ -2179,12 +2196,13 @@ int security_load_policy(void *data, size_t len)
}

/* Save the old policydb and SID table to free later. */
- memcpy(oldpolicydb, &policydb, sizeof(policydb));
+ memcpy(oldpolicydb, &crm->policydb, sizeof(struct policydb));
sidtab_set(&oldsidtab, &sidtab);

/* Install the new policydb and SID table. */
write_lock_irq(&policy_rwlock);
- memcpy(&policydb, newpolicydb, sizeof(policydb));
+ memcpy(&crm->policydb, newpolicydb, sizeof(struct policydb));
+
sidtab_set(&sidtab, &newsidtab);
security_load_policycaps();
oldmap = crm->current_mapping;
@@ -2222,7 +2240,7 @@ size_t security_policydb_len(void)
size_t len;

read_lock(&policy_rwlock);
- len = policydb.len;
+ len = crm->policydb.len;
read_unlock(&policy_rwlock);

return len;
@@ -2241,7 +2259,7 @@ int security_port_sid(u8 protocol, u16 port, u32 *out_sid)

read_lock(&policy_rwlock);

- c = policydb.ocontexts[OCON_PORT];
+ c = crm->policydb.ocontexts[OCON_PORT];
while (c) {
if (c->u.port.protocol == protocol &&
c->u.port.low_port <= port &&
@@ -2281,7 +2299,7 @@ int security_ib_pkey_sid(u64 subnet_prefix, u16 pkey_num, u32 *out_sid)

read_lock(&policy_rwlock);

- c = policydb.ocontexts[OCON_IBPKEY];
+ c = crm->policydb.ocontexts[OCON_IBPKEY];
while (c) {
if (c->u.ibpkey.low_pkey <= pkey_num &&
c->u.ibpkey.high_pkey >= pkey_num &&
@@ -2321,7 +2339,7 @@ int security_ib_endport_sid(const char *dev_name, u8 port_num, u32 *out_sid)

read_lock(&policy_rwlock);

- c = policydb.ocontexts[OCON_IBENDPORT];
+ c = crm->policydb.ocontexts[OCON_IBENDPORT];
while (c) {
if (c->u.ibendport.port == port_num &&
!strncmp(c->u.ibendport.dev_name,
@@ -2361,7 +2379,7 @@ int security_netif_sid(char *name, u32 *if_sid)

read_lock(&policy_rwlock);

- c = policydb.ocontexts[OCON_NETIF];
+ c = crm->policydb.ocontexts[OCON_NETIF];
while (c) {
if (strcmp(name, c->u.name) == 0)
break;
@@ -2430,7 +2448,7 @@ int security_node_sid(u16 domain,

addr = *((u32 *)addrp);

- c = policydb.ocontexts[OCON_NODE];
+ c = crm->policydb.ocontexts[OCON_NODE];
while (c) {
if (c->u.node.addr == (addr & c->u.node.mask))
break;
@@ -2443,7 +2461,7 @@ int security_node_sid(u16 domain,
rc = -EINVAL;
if (addrlen != sizeof(u64) * 2)
goto out;
- c = policydb.ocontexts[OCON_NODE6];
+ c = crm->policydb.ocontexts[OCON_NODE6];
while (c) {
if (match_ipv6_addrmask(addrp, c->u.node6.addr,
c->u.node6.mask))
@@ -2522,7 +2540,7 @@ int security_get_user_sids(u32 fromsid,
goto out_unlock;

rc = -EINVAL;
- user = hashtab_search(policydb.p_users.table, username);
+ user = hashtab_search(crm->policydb.p_users.table, username);
if (!user)
goto out_unlock;

@@ -2534,12 +2552,12 @@ int security_get_user_sids(u32 fromsid,
goto out_unlock;

ebitmap_for_each_positive_bit(&user->roles, rnode, i) {
- role = policydb.role_val_to_struct[i];
+ role = crm->policydb.role_val_to_struct[i];
usercon.role = i + 1;
ebitmap_for_each_positive_bit(&role->types, tnode, j) {
usercon.type = j + 1;

- if (mls_setup_user_range(&policydb, fromcon,
+ if (mls_setup_user_range(&crm->policydb, fromcon,
user, &usercon))
continue;

@@ -2623,7 +2641,7 @@ static inline int __security_genfs_sid(const char *fstype,
sclass = unmap_class(orig_sclass);
*sid = SECINITSID_UNLABELED;

- for (genfs = policydb.genfs; genfs; genfs = genfs->next) {
+ for (genfs = crm->policydb.genfs; genfs; genfs = genfs->next) {
cmp = strcmp(fstype, genfs->fstype);
if (cmp <= 0)
break;
@@ -2692,7 +2710,7 @@ int security_fs_use(struct super_block *sb)

read_lock(&policy_rwlock);

- c = policydb.ocontexts[OCON_FSUSE];
+ c = crm->policydb.ocontexts[OCON_FSUSE];
while (c) {
if (strcmp(fstype, c->u.name) == 0)
break;
@@ -2733,7 +2751,7 @@ int security_get_bools(int *len, char ***names, int **values)
*values = NULL;

rc = 0;
- *len = policydb.p_bools.nprim;
+ *len = crm->policydb.p_bools.nprim;
if (!*len)
goto out;

@@ -2748,10 +2766,11 @@ int security_get_bools(int *len, char ***names, int **values)
goto err;

for (i = 0; i < *len; i++) {
- (*values)[i] = policydb.bool_val_to_struct[i]->state;
+ (*values)[i] = crm->policydb.bool_val_to_struct[i]->state;

rc = -ENOMEM;
- (*names)[i] = kstrdup(sym_name(&policydb, SYM_BOOLS, i), GFP_ATOMIC);
+ (*names)[i] = kstrdup(sym_name(&crm->policydb, SYM_BOOLS, i),
+ GFP_ATOMIC);
if (!(*names)[i])
goto err;
}
@@ -2778,29 +2797,30 @@ int security_set_bools(int len, int *values)
write_lock_irq(&policy_rwlock);

rc = -EFAULT;
- lenp = policydb.p_bools.nprim;
+ lenp = crm->policydb.p_bools.nprim;
if (len != lenp)
goto out;

for (i = 0; i < len; i++) {
- if (!!values[i] != policydb.bool_val_to_struct[i]->state) {
+ if (!!values[i] !=
+ crm->policydb.bool_val_to_struct[i]->state) {
audit_log(current->audit_context, GFP_ATOMIC,
AUDIT_MAC_CONFIG_CHANGE,
"bool=%s val=%d old_val=%d auid=%u ses=%u",
- sym_name(&policydb, SYM_BOOLS, i),
+ sym_name(&crm->policydb, SYM_BOOLS, i),
!!values[i],
- policydb.bool_val_to_struct[i]->state,
+ crm->policydb.bool_val_to_struct[i]->state,
from_kuid(&init_user_ns, audit_get_loginuid(current)),
audit_get_sessionid(current));
}
if (values[i])
- policydb.bool_val_to_struct[i]->state = 1;
+ crm->policydb.bool_val_to_struct[i]->state = 1;
else
- policydb.bool_val_to_struct[i]->state = 0;
+ crm->policydb.bool_val_to_struct[i]->state = 0;
}

- for (cur = policydb.cond_list; cur; cur = cur->next) {
- rc = evaluate_cond_node(&policydb, cur);
+ for (cur = crm->policydb.cond_list; cur; cur = cur->next) {
+ rc = evaluate_cond_node(&crm->policydb, cur);
if (rc)
goto out;
}
@@ -2826,11 +2846,11 @@ int security_get_bool_value(int index)
read_lock(&policy_rwlock);

rc = -EFAULT;
- len = policydb.p_bools.nprim;
+ len = crm->policydb.p_bools.nprim;
if (index >= len)
goto out;

- rc = policydb.bool_val_to_struct[index]->state;
+ rc = crm->policydb.bool_val_to_struct[index]->state;
out:
read_unlock(&policy_rwlock);
return rc;
@@ -2881,7 +2901,7 @@ int security_sid_mls_copy(u32 sid, u32 mls_sid, u32 *new_sid)
int rc;

rc = 0;
- if (!ss_initialized || !policydb.mls_enabled) {
+ if (!ss_initialized || !crm->policydb.mls_enabled) {
*new_sid = sid;
goto out;
}
@@ -2914,7 +2934,7 @@ int security_sid_mls_copy(u32 sid, u32 mls_sid, u32 *new_sid)
goto out_unlock;

/* Check the validity of the new context. */
- if (!policydb_context_isvalid(&policydb, &newcon)) {
+ if (!policydb_context_isvalid(&crm->policydb, &newcon)) {
rc = convert_context_handle_invalid_context(&newcon);
if (rc) {
if (!context_struct_to_string(&newcon, &s, &len)) {
@@ -2984,7 +3004,7 @@ int security_net_peersid_resolve(u32 nlbl_sid, u32 nlbl_type,
/* we don't need to check ss_initialized here since the only way both
* nlbl_sid and xfrm_sid are not equal to SECSID_NULL would be if the
* security server was initialized and ss_initialized was true */
- if (!policydb.mls_enabled)
+ if (!crm->policydb.mls_enabled)
return 0;

read_lock(&policy_rwlock);
@@ -3038,12 +3058,12 @@ int security_get_classes(char ***classes, int *nclasses)
read_lock(&policy_rwlock);

rc = -ENOMEM;
- *nclasses = policydb.p_classes.nprim;
+ *nclasses = crm->policydb.p_classes.nprim;
*classes = kcalloc(*nclasses, sizeof(**classes), GFP_ATOMIC);
if (!*classes)
goto out;

- rc = hashtab_map(policydb.p_classes.table, get_classes_callback,
+ rc = hashtab_map(crm->policydb.p_classes.table, get_classes_callback,
*classes);
if (rc) {
int i;
@@ -3078,7 +3098,7 @@ int security_get_permissions(char *class, char ***perms, int *nperms)
read_lock(&policy_rwlock);

rc = -EINVAL;
- match = hashtab_search(policydb.p_classes.table, class);
+ match = hashtab_search(crm->policydb.p_classes.table, class);
if (!match) {
printk(KERN_ERR "SELinux: %s: unrecognized class %s\n",
__func__, class);
@@ -3117,12 +3137,12 @@ int security_get_permissions(char *class, char ***perms, int *nperms)

int security_get_reject_unknown(void)
{
- return policydb.reject_unknown;
+ return crm->policydb.reject_unknown;
}

int security_get_allow_unknown(void)
{
- return policydb.allow_unknown;
+ return crm->policydb.allow_unknown;
}

/**
@@ -3140,7 +3160,7 @@ int security_policycap_supported(unsigned int req_cap)
int rc;

read_lock(&policy_rwlock);
- rc = ebitmap_get_bit(&policydb.policycaps, req_cap);
+ rc = ebitmap_get_bit(&crm->policydb.policycaps, req_cap);
read_unlock(&policy_rwlock);

return rc;
@@ -3213,7 +3233,8 @@ int selinux_audit_rule_init(u32 field, u32 op, char *rulestr, void **vrule)
case AUDIT_SUBJ_USER:
case AUDIT_OBJ_USER:
rc = -EINVAL;
- userdatum = hashtab_search(policydb.p_users.table, rulestr);
+ userdatum = hashtab_search(crm->policydb.p_users.table,
+ rulestr);
if (!userdatum)
goto out;
tmprule->au_ctxt.user = userdatum->value;
@@ -3221,7 +3242,8 @@ int selinux_audit_rule_init(u32 field, u32 op, char *rulestr, void **vrule)
case AUDIT_SUBJ_ROLE:
case AUDIT_OBJ_ROLE:
rc = -EINVAL;
- roledatum = hashtab_search(policydb.p_roles.table, rulestr);
+ roledatum = hashtab_search(crm->policydb.p_roles.table,
+ rulestr);
if (!roledatum)
goto out;
tmprule->au_ctxt.role = roledatum->value;
@@ -3229,7 +3251,8 @@ int selinux_audit_rule_init(u32 field, u32 op, char *rulestr, void **vrule)
case AUDIT_SUBJ_TYPE:
case AUDIT_OBJ_TYPE:
rc = -EINVAL;
- typedatum = hashtab_search(policydb.p_types.table, rulestr);
+ typedatum = hashtab_search(crm->policydb.p_types.table,
+ rulestr);
if (!typedatum)
goto out;
tmprule->au_ctxt.type = typedatum->value;
@@ -3238,8 +3261,8 @@ int selinux_audit_rule_init(u32 field, u32 op, char *rulestr, void **vrule)
case AUDIT_SUBJ_CLR:
case AUDIT_OBJ_LEV_LOW:
case AUDIT_OBJ_LEV_HIGH:
- rc = mls_from_string(&policydb, rulestr, &tmprule->au_ctxt,
- GFP_ATOMIC);
+ rc = mls_from_string(&crm->policydb, rulestr,
+ &tmprule->au_ctxt, GFP_ATOMIC);
if (rc)
goto out;
break;
@@ -3489,15 +3512,15 @@ int security_netlbl_secattr_to_sid(struct netlbl_lsm_secattr *secattr,
ctx_new.user = ctx->user;
ctx_new.role = ctx->role;
ctx_new.type = ctx->type;
- mls_import_netlbl_lvl(&policydb, &ctx_new, secattr);
+ mls_import_netlbl_lvl(&crm->policydb, &ctx_new, secattr);
if (secattr->flags & NETLBL_SECATTR_MLS_CAT) {
- rc = mls_import_netlbl_cat(&policydb, &ctx_new,
+ rc = mls_import_netlbl_cat(&crm->policydb, &ctx_new,
secattr);
if (rc)
goto out;
}
rc = -EIDRM;
- if (!mls_context_isvalid(&policydb, &ctx_new))
+ if (!mls_context_isvalid(&crm->policydb, &ctx_new))
goto out_free;

rc = sidtab_context_to_sid(&sidtab, &ctx_new, sid);
@@ -3545,15 +3568,16 @@ int security_netlbl_sid_to_secattr(u32 sid, struct netlbl_lsm_secattr *secattr)
goto out;

rc = -ENOMEM;
- secattr->domain = kstrdup(sym_name(&policydb, SYM_TYPES, ctx->type - 1),
+ secattr->domain = kstrdup(sym_name(&crm->policydb,
+ SYM_TYPES, ctx->type - 1),
GFP_ATOMIC);
if (secattr->domain == NULL)
goto out;

secattr->attr.secid = sid;
secattr->flags |= NETLBL_SECATTR_DOMAIN_CPY | NETLBL_SECATTR_SECID;
- mls_export_netlbl_lvl(&policydb, ctx, secattr);
- rc = mls_export_netlbl_cat(&policydb, ctx, secattr);
+ mls_export_netlbl_lvl(&crm->policydb, ctx, secattr);
+ rc = mls_export_netlbl_cat(&crm->policydb, ctx, secattr);
out:
read_unlock(&policy_rwlock);
return rc;
@@ -3584,7 +3608,7 @@ int security_read_policy(void **data, size_t *len)
fp.len = *len;

read_lock(&policy_rwlock);
- rc = policydb_write(&policydb, &fp);
+ rc = policydb_write(&crm->policydb, &fp);
read_unlock(&policy_rwlock);

if (rc)
--
2.7.4


2018-01-26 14:36:26

by peter enderborg

[permalink] [raw]
Subject: [PATCH v2 1/5] selinux:Remove direct references to policydb.

From: Peter Enderborg <[email protected]>

To be able to use rcu locks we seed to address the policydb
though a pointer. This preparation removes the export of the
policydb and send pointers to it through parameter agruments.

Signed-off-by: Peter Enderborg <[email protected]>
---
security/selinux/ss/mls.c | 69 ++++++++++++++++----------------
security/selinux/ss/mls.h | 37 +++++++++--------
security/selinux/ss/services.c | 90 +++++++++++++++++++++++++++---------------
security/selinux/ss/services.h | 3 --
4 files changed, 114 insertions(+), 85 deletions(-)

diff --git a/security/selinux/ss/mls.c b/security/selinux/ss/mls.c
index ad982ce..b1f35d3 100644
--- a/security/selinux/ss/mls.c
+++ b/security/selinux/ss/mls.c
@@ -33,20 +33,20 @@
* Return the length in bytes for the MLS fields of the
* security context string representation of `context'.
*/
-int mls_compute_context_len(struct context *context)
+int mls_compute_context_len(struct policydb *p, struct context *context)
{
int i, l, len, head, prev;
char *nm;
struct ebitmap *e;
struct ebitmap_node *node;

- if (!policydb.mls_enabled)
+ if (!p->mls_enabled)
return 0;

len = 1; /* for the beginning ":" */
for (l = 0; l < 2; l++) {
int index_sens = context->range.level[l].sens;
- len += strlen(sym_name(&policydb, SYM_LEVELS, index_sens - 1));
+ len += strlen(sym_name(p, SYM_LEVELS, index_sens - 1));

/* categories */
head = -2;
@@ -56,17 +56,17 @@ int mls_compute_context_len(struct context *context)
if (i - prev > 1) {
/* one or more negative bits are skipped */
if (head != prev) {
- nm = sym_name(&policydb, SYM_CATS, prev);
+ nm = sym_name(p, SYM_CATS, prev);
len += strlen(nm) + 1;
}
- nm = sym_name(&policydb, SYM_CATS, i);
+ nm = sym_name(p, SYM_CATS, i);
len += strlen(nm) + 1;
head = i;
}
prev = i;
}
if (prev != head) {
- nm = sym_name(&policydb, SYM_CATS, prev);
+ nm = sym_name(p, SYM_CATS, prev);
len += strlen(nm) + 1;
}
if (l == 0) {
@@ -86,7 +86,7 @@ int mls_compute_context_len(struct context *context)
* the MLS fields of `context' into the string `*scontext'.
* Update `*scontext' to point to the end of the MLS fields.
*/
-void mls_sid_to_context(struct context *context,
+void mls_sid_to_context(struct policydb *p, struct context *context,
char **scontext)
{
char *scontextp, *nm;
@@ -94,7 +94,7 @@ void mls_sid_to_context(struct context *context,
struct ebitmap *e;
struct ebitmap_node *node;

- if (!policydb.mls_enabled)
+ if (!p->mls_enabled)
return;

scontextp = *scontext;
@@ -103,7 +103,7 @@ void mls_sid_to_context(struct context *context,
scontextp++;

for (l = 0; l < 2; l++) {
- strcpy(scontextp, sym_name(&policydb, SYM_LEVELS,
+ strcpy(scontextp, sym_name(p, SYM_LEVELS,
context->range.level[l].sens - 1));
scontextp += strlen(scontextp);

@@ -119,7 +119,7 @@ void mls_sid_to_context(struct context *context,
*scontextp++ = '.';
else
*scontextp++ = ',';
- nm = sym_name(&policydb, SYM_CATS, prev);
+ nm = sym_name(p, SYM_CATS, prev);
strcpy(scontextp, nm);
scontextp += strlen(nm);
}
@@ -127,7 +127,7 @@ void mls_sid_to_context(struct context *context,
*scontextp++ = ':';
else
*scontextp++ = ',';
- nm = sym_name(&policydb, SYM_CATS, i);
+ nm = sym_name(p, SYM_CATS, i);
strcpy(scontextp, nm);
scontextp += strlen(nm);
head = i;
@@ -140,7 +140,7 @@ void mls_sid_to_context(struct context *context,
*scontextp++ = '.';
else
*scontextp++ = ',';
- nm = sym_name(&policydb, SYM_CATS, prev);
+ nm = sym_name(p, SYM_CATS, prev);
strcpy(scontextp, nm);
scontextp += strlen(nm);
}
@@ -375,12 +375,13 @@ int mls_context_to_sid(struct policydb *pol,
* the string `str'. This function will allocate temporary memory with the
* given constraints of gfp_mask.
*/
-int mls_from_string(char *str, struct context *context, gfp_t gfp_mask)
+int mls_from_string(struct policydb *p, char *str, struct context *context,
+ gfp_t gfp_mask)
{
char *tmpstr, *freestr;
int rc;

- if (!policydb.mls_enabled)
+ if (!p->mls_enabled)
return -EINVAL;

/* we need freestr because mls_context_to_sid will change
@@ -389,7 +390,7 @@ int mls_from_string(char *str, struct context *context, gfp_t gfp_mask)
if (!tmpstr) {
rc = -ENOMEM;
} else {
- rc = mls_context_to_sid(&policydb, ':', &tmpstr, context,
+ rc = mls_context_to_sid(p, ':', &tmpstr, context,
NULL, SECSID_NULL);
kfree(freestr);
}
@@ -417,10 +418,10 @@ int mls_range_set(struct context *context,
return rc;
}

-int mls_setup_user_range(struct context *fromcon, struct user_datum *user,
- struct context *usercon)
+int mls_setup_user_range(struct policydb *p, struct context *fromcon,
+ struct user_datum *user, struct context *usercon)
{
- if (policydb.mls_enabled) {
+ if (p->mls_enabled) {
struct mls_level *fromcon_sen = &(fromcon->range.level[0]);
struct mls_level *fromcon_clr = &(fromcon->range.level[1]);
struct mls_level *user_low = &(user->range.level[0]);
@@ -460,7 +461,7 @@ int mls_setup_user_range(struct context *fromcon, struct user_datum *user,
* structure `c' from the values specified in the
* policy `oldp' to the values specified in the policy `newp'.
*/
-int mls_convert_context(struct policydb *oldp,
+int mls_convert_context(struct policydb *p, struct policydb *oldp,
struct policydb *newp,
struct context *c)
{
@@ -470,7 +471,7 @@ int mls_convert_context(struct policydb *oldp,
struct ebitmap_node *node;
int l, i;

- if (!policydb.mls_enabled)
+ if (!p->mls_enabled)
return 0;

for (l = 0; l < 2; l++) {
@@ -503,7 +504,7 @@ int mls_convert_context(struct policydb *oldp,
return 0;
}

-int mls_compute_sid(struct context *scontext,
+int mls_compute_sid(struct policydb *p, struct context *scontext,
struct context *tcontext,
u16 tclass,
u32 specified,
@@ -515,7 +516,7 @@ int mls_compute_sid(struct context *scontext,
struct class_datum *cladatum;
int default_range = 0;

- if (!policydb.mls_enabled)
+ if (!p->mls_enabled)
return 0;

switch (specified) {
@@ -524,12 +525,12 @@ int mls_compute_sid(struct context *scontext,
rtr.source_type = scontext->type;
rtr.target_type = tcontext->type;
rtr.target_class = tclass;
- r = hashtab_search(policydb.range_tr, &rtr);
+ r = hashtab_search(p->range_tr, &rtr);
if (r)
return mls_range_set(newcontext, r);

- if (tclass && tclass <= policydb.p_classes.nprim) {
- cladatum = policydb.class_val_to_struct[tclass - 1];
+ if (tclass && tclass <= p->p_classes.nprim) {
+ cladatum = p->class_val_to_struct[tclass - 1];
if (cladatum)
default_range = cladatum->default_range;
}
@@ -551,7 +552,7 @@ int mls_compute_sid(struct context *scontext,

/* Fallthrough */
case AVTAB_CHANGE:
- if ((tclass == policydb.process_class) || (sock == true))
+ if ((tclass == p->process_class) || (sock == true))
/* Use the process MLS attributes. */
return mls_context_cpy(newcontext, scontext);
else
@@ -577,10 +578,10 @@ int mls_compute_sid(struct context *scontext,
* NetLabel MLS sensitivity level field.
*
*/
-void mls_export_netlbl_lvl(struct context *context,
+void mls_export_netlbl_lvl(struct policydb *p, struct context *context,
struct netlbl_lsm_secattr *secattr)
{
- if (!policydb.mls_enabled)
+ if (!p->mls_enabled)
return;

secattr->attr.mls.lvl = context->range.level[0].sens - 1;
@@ -597,10 +598,10 @@ void mls_export_netlbl_lvl(struct context *context,
* NetLabel MLS sensitivity level into the context.
*
*/
-void mls_import_netlbl_lvl(struct context *context,
+void mls_import_netlbl_lvl(struct policydb *p, struct context *context,
struct netlbl_lsm_secattr *secattr)
{
- if (!policydb.mls_enabled)
+ if (!p->mls_enabled)
return;

context->range.level[0].sens = secattr->attr.mls.lvl + 1;
@@ -617,12 +618,12 @@ void mls_import_netlbl_lvl(struct context *context,
* MLS category field. Returns zero on success, negative values on failure.
*
*/
-int mls_export_netlbl_cat(struct context *context,
+int mls_export_netlbl_cat(struct policydb *p, struct context *context,
struct netlbl_lsm_secattr *secattr)
{
int rc;

- if (!policydb.mls_enabled)
+ if (!p->mls_enabled)
return 0;

rc = ebitmap_netlbl_export(&context->range.level[0].cat,
@@ -645,12 +646,12 @@ int mls_export_netlbl_cat(struct context *context,
* negative values on failure.
*
*/
-int mls_import_netlbl_cat(struct context *context,
+int mls_import_netlbl_cat(struct policydb *p, struct context *context,
struct netlbl_lsm_secattr *secattr)
{
int rc;

- if (!policydb.mls_enabled)
+ if (!p->mls_enabled)
return 0;

rc = ebitmap_netlbl_import(&context->range.level[0].cat,
diff --git a/security/selinux/ss/mls.h b/security/selinux/ss/mls.h
index 131d762..cb039c0 100644
--- a/security/selinux/ss/mls.h
+++ b/security/selinux/ss/mls.h
@@ -25,8 +25,9 @@
#include "context.h"
#include "policydb.h"

-int mls_compute_context_len(struct context *context);
-void mls_sid_to_context(struct context *context, char **scontext);
+int mls_compute_context_len(struct policydb *p, struct context *context);
+void mls_sid_to_context(struct policydb *p, struct context *context,
+ char **scontext);
int mls_context_isvalid(struct policydb *p, struct context *c);
int mls_range_isvalid(struct policydb *p, struct mls_range *r);
int mls_level_isvalid(struct policydb *p, struct mls_level *l);
@@ -38,50 +39,55 @@ int mls_context_to_sid(struct policydb *p,
struct sidtab *s,
u32 def_sid);

-int mls_from_string(char *str, struct context *context, gfp_t gfp_mask);
+int mls_from_string(struct policydb *p, char *str, struct context *context,
+ gfp_t gfp_mask);

int mls_range_set(struct context *context, struct mls_range *range);

-int mls_convert_context(struct policydb *oldp,
+int mls_convert_context(struct policydb *p, struct policydb *oldp,
struct policydb *newp,
struct context *context);

-int mls_compute_sid(struct context *scontext,
+int mls_compute_sid(struct policydb *p, struct context *scontext,
struct context *tcontext,
u16 tclass,
u32 specified,
struct context *newcontext,
bool sock);

-int mls_setup_user_range(struct context *fromcon, struct user_datum *user,
- struct context *usercon);
+int mls_setup_user_range(struct policydb *p, struct context *fromcon,
+ struct user_datum *user, struct context *usercon);

#ifdef CONFIG_NETLABEL
-void mls_export_netlbl_lvl(struct context *context,
+void mls_export_netlbl_lvl(struct policydb *p, struct context *context,
struct netlbl_lsm_secattr *secattr);
-void mls_import_netlbl_lvl(struct context *context,
+void mls_import_netlbl_lvl(struct policydb *p, struct context *context,
struct netlbl_lsm_secattr *secattr);
-int mls_export_netlbl_cat(struct context *context,
+int mls_export_netlbl_cat(struct policydb *p, struct context *context,
struct netlbl_lsm_secattr *secattr);
-int mls_import_netlbl_cat(struct context *context,
+int mls_import_netlbl_cat(struct policydb *p, struct context *context,
struct netlbl_lsm_secattr *secattr);
#else
-static inline void mls_export_netlbl_lvl(struct context *context,
+static inline void mls_export_netlbl_lvl(struct policydb *p,
+ struct context *context,
struct netlbl_lsm_secattr *secattr)
{
return;
}
-static inline void mls_import_netlbl_lvl(struct context *context,
+static inline void mls_import_netlbl_lvl(struct policydb *p,
+ struct context *context,
struct netlbl_lsm_secattr *secattr)
{
return;
}
-static inline int mls_export_netlbl_cat(struct context *context,
+static inline int mls_export_netlbl_cat(struct policydb *p,
+ struct context *context,
struct netlbl_lsm_secattr *secattr)
{
return -ENOMEM;
}
-static inline int mls_import_netlbl_cat(struct context *context,
+static inline int mls_import_netlbl_cat(struct policydb *p,
+ struct context *context,
struct netlbl_lsm_secattr *secattr)
{
return -ENOMEM;
@@ -89,4 +95,3 @@ static inline int mls_import_netlbl_cat(struct context *context,
#endif

#endif /* _SS_MLS_H */
-
diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index 33cfe5d..47d8030 100644
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
@@ -90,7 +90,7 @@ int selinux_policycap_nnp_nosuid_transition;
static DEFINE_RWLOCK(policy_rwlock);

static struct sidtab sidtab;
-struct policydb policydb;
+static struct policydb policydb;
int ss_initialized;

/*
@@ -117,8 +117,12 @@ struct selinux_mapping {
u32 perms[sizeof(u32) * 8];
};

-static struct selinux_mapping *current_mapping;
-static u16 current_mapping_size;
+struct shared_current_mapping {
+ struct selinux_mapping *current_mapping;
+ u16 current_mapping_size;
+};
+
+static struct shared_current_mapping *crm;

static int selinux_set_mapping(struct policydb *pol,
struct security_class_mapping *map,
@@ -208,8 +212,8 @@ static int selinux_set_mapping(struct policydb *pol,

static u16 unmap_class(u16 tclass)
{
- if (tclass < current_mapping_size)
- return current_mapping[tclass].value;
+ if (tclass < crm->current_mapping_size)
+ return crm->current_mapping[tclass].value;

return tclass;
}
@@ -221,8 +225,8 @@ static u16 map_class(u16 pol_value)
{
u16 i;

- for (i = 1; i < current_mapping_size; i++) {
- if (current_mapping[i].value == pol_value)
+ for (i = 1; i < crm->current_mapping_size; i++) {
+ if (crm->current_mapping[i].value == pol_value)
return i;
}

@@ -232,27 +236,32 @@ static u16 map_class(u16 pol_value)
static void map_decision(u16 tclass, struct av_decision *avd,
int allow_unknown)
{
- if (tclass < current_mapping_size) {
- unsigned i, n = current_mapping[tclass].num_perms;
+ if (tclass < crm->current_mapping_size) {
+ unsigned int i, n = crm->current_mapping[tclass].num_perms;
u32 result;

for (i = 0, result = 0; i < n; i++) {
- if (avd->allowed & current_mapping[tclass].perms[i])
+ if (avd->allowed &
+ crm->current_mapping[tclass].perms[i])
result |= 1<<i;
- if (allow_unknown && !current_mapping[tclass].perms[i])
+ if (allow_unknown &&
+ !crm->current_mapping[tclass].perms[i])
result |= 1<<i;
}
avd->allowed = result;

for (i = 0, result = 0; i < n; i++)
- if (avd->auditallow & current_mapping[tclass].perms[i])
+ if (avd->auditallow &
+ crm->current_mapping[tclass].perms[i])
result |= 1<<i;
avd->auditallow = result;

for (i = 0, result = 0; i < n; i++) {
- if (avd->auditdeny & current_mapping[tclass].perms[i])
+ if (avd->auditdeny &
+ crm->current_mapping[tclass].perms[i])
result |= 1<<i;
- if (!allow_unknown && !current_mapping[tclass].perms[i])
+ if (!allow_unknown &&
+ !crm->current_mapping[tclass].perms[i])
result |= 1<<i;
}
/*
@@ -1211,7 +1220,7 @@ static int context_struct_to_string(struct context *context, char **scontext, u3
*scontext_len += strlen(sym_name(&policydb, SYM_USERS, context->user - 1)) + 1;
*scontext_len += strlen(sym_name(&policydb, SYM_ROLES, context->role - 1)) + 1;
*scontext_len += strlen(sym_name(&policydb, SYM_TYPES, context->type - 1)) + 1;
- *scontext_len += mls_compute_context_len(context);
+ *scontext_len += mls_compute_context_len(&policydb, context);

if (!scontext)
return 0;
@@ -1230,7 +1239,7 @@ static int context_struct_to_string(struct context *context, char **scontext, u3
sym_name(&policydb, SYM_ROLES, context->role - 1),
sym_name(&policydb, SYM_TYPES, context->type - 1));

- mls_sid_to_context(context, &scontextp);
+ mls_sid_to_context(&policydb, context, &scontextp);

*scontextp = 0;

@@ -1721,7 +1730,7 @@ static int security_compute_sid(u32 ssid,

/* Set the MLS attributes.
This is done last because it may allocate memory. */
- rc = mls_compute_sid(scontext, tcontext, tclass, specified,
+ rc = mls_compute_sid(&policydb, scontext, tcontext, tclass, specified,
&newcontext, sock);
if (rc)
goto out_unlock;
@@ -1935,7 +1944,7 @@ static int convert_context(u32 key,

/* Convert the MLS fields if dealing with MLS policies */
if (args->oldp->mls_enabled && args->newp->mls_enabled) {
- rc = mls_convert_context(args->oldp, args->newp, c);
+ rc = mls_convert_context(&policydb, args->oldp, args->newp, c);
if (rc)
goto bad;
} else if (args->oldp->mls_enabled && !args->newp->mls_enabled) {
@@ -2043,8 +2052,9 @@ int security_load_policy(void *data, size_t len)
{
struct policydb *oldpolicydb, *newpolicydb;
struct sidtab oldsidtab, newsidtab;
- struct selinux_mapping *oldmap, *map = NULL;
+ struct selinux_mapping *oldmap = NULL, *map = NULL;
struct convert_context_args args;
+ struct shared_current_mapping *new_mapping;
u32 seqno;
u16 map_size;
int rc = 0;
@@ -2055,9 +2065,22 @@ int security_load_policy(void *data, size_t len)
rc = -ENOMEM;
goto out;
}
+ new_mapping = kzalloc(sizeof(struct shared_current_mapping),
+ GFP_KERNEL);
+ if (!new_mapping) {
+ rc = -ENOMEM;
+ goto out;
+ }
newpolicydb = oldpolicydb + 1;

if (!ss_initialized) {
+ crm = kzalloc(sizeof(struct shared_current_mapping),
+ GFP_KERNEL);
+ if (!crm) {
+ rc = -ENOMEM;
+ goto out;
+ }
+
avtab_cache_init();
ebitmap_cache_init();
hashtab_cache_init();
@@ -2071,8 +2094,8 @@ int security_load_policy(void *data, size_t len)

policydb.len = len;
rc = selinux_set_mapping(&policydb, secclass_map,
- &current_mapping,
- &current_mapping_size);
+ &crm->current_mapping,
+ &crm->current_mapping_size);
if (rc) {
policydb_destroy(&policydb);
avtab_cache_destroy();
@@ -2164,9 +2187,9 @@ int security_load_policy(void *data, size_t len)
memcpy(&policydb, newpolicydb, sizeof(policydb));
sidtab_set(&sidtab, &newsidtab);
security_load_policycaps();
- oldmap = current_mapping;
- current_mapping = map;
- current_mapping_size = map_size;
+ oldmap = crm->current_mapping;
+ crm->current_mapping = map;
+ crm->current_mapping_size = map_size;
seqno = ++latest_granting;
write_unlock_irq(&policy_rwlock);

@@ -2516,7 +2539,8 @@ int security_get_user_sids(u32 fromsid,
ebitmap_for_each_positive_bit(&role->types, tnode, j) {
usercon.type = j + 1;

- if (mls_setup_user_range(fromcon, user, &usercon))
+ if (mls_setup_user_range(&policydb, fromcon,
+ user, &usercon))
continue;

rc = sidtab_context_to_sid(&sidtab, &usercon, &sid);
@@ -2580,7 +2604,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 hold rcu before calling this function.
*/
static inline int __security_genfs_sid(const char *fstype,
char *path,
@@ -2639,7 +2663,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
+ * Hold rcu before calling __security_genfs_sid() and release
* it afterward.
*/
int security_genfs_sid(const char *fstype,
@@ -3214,7 +3238,8 @@ int selinux_audit_rule_init(u32 field, u32 op, char *rulestr, void **vrule)
case AUDIT_SUBJ_CLR:
case AUDIT_OBJ_LEV_LOW:
case AUDIT_OBJ_LEV_HIGH:
- rc = mls_from_string(rulestr, &tmprule->au_ctxt, GFP_ATOMIC);
+ rc = mls_from_string(&policydb, rulestr, &tmprule->au_ctxt,
+ GFP_ATOMIC);
if (rc)
goto out;
break;
@@ -3464,9 +3489,10 @@ int security_netlbl_secattr_to_sid(struct netlbl_lsm_secattr *secattr,
ctx_new.user = ctx->user;
ctx_new.role = ctx->role;
ctx_new.type = ctx->type;
- mls_import_netlbl_lvl(&ctx_new, secattr);
+ mls_import_netlbl_lvl(&policydb, &ctx_new, secattr);
if (secattr->flags & NETLBL_SECATTR_MLS_CAT) {
- rc = mls_import_netlbl_cat(&ctx_new, secattr);
+ rc = mls_import_netlbl_cat(&policydb, &ctx_new,
+ secattr);
if (rc)
goto out;
}
@@ -3526,8 +3552,8 @@ int security_netlbl_sid_to_secattr(u32 sid, struct netlbl_lsm_secattr *secattr)

secattr->attr.secid = sid;
secattr->flags |= NETLBL_SECATTR_DOMAIN_CPY | NETLBL_SECATTR_SECID;
- mls_export_netlbl_lvl(ctx, secattr);
- rc = mls_export_netlbl_cat(ctx, secattr);
+ mls_export_netlbl_lvl(&policydb, ctx, secattr);
+ rc = mls_export_netlbl_cat(&policydb, ctx, secattr);
out:
read_unlock(&policy_rwlock);
return rc;
diff --git a/security/selinux/ss/services.h b/security/selinux/ss/services.h
index 356bdd3..50c7ceb 100644
--- a/security/selinux/ss/services.h
+++ b/security/selinux/ss/services.h
@@ -10,8 +10,6 @@
#include "policydb.h"
#include "sidtab.h"

-extern struct policydb policydb;
-
void services_compute_xperms_drivers(struct extended_perms *xperms,
struct avtab_node *node);

@@ -19,4 +17,3 @@ void services_compute_xperms_decision(struct extended_perms_decision *xpermd,
struct avtab_node *node);

#endif /* _SS_SERVICES_H_ */
-
--
2.7.4


2018-01-30 14:21:17

by Stephen Smalley

[permalink] [raw]
Subject: Re: [PATCH v2 1/5] selinux:Remove direct references to policydb.

On Fri, 2018-01-26 at 15:32 +0100, [email protected] wrote:
> From: Peter Enderborg <[email protected]>
>
> To be able to use rcu locks we seed to address the policydb
> though a pointer. This preparation removes the export of the
> policydb and send pointers to it through parameter agruments.

Just for reference, I have a patch series that does this not only for
the policydb, sidtab, and class/perm mapping, but for all of the
SELinux global state, see:
https://github.com/stephensmalley/selinux-kernel/tree/selinuxns
and in particular
https://github.com/stephensmalley/selinux-kernel/commit/c10d90b43cd720c8f8aab51007e805bf7c4f10d2
https://github.com/stephensmalley/selinux-kernel/commit/ec038a64173d56a331423b6d1564b801f0915afc
https://github.com/stephensmalley/selinux-kernel/commit/97aa5d7a05e4458bc4562c47d8f7bc4f56fbfefd

Those first three patches should have no effect on SELinux behavior.
They need to be re-based to latest selinux next branch (some minor
conflict resolution required) but I was waiting for that to advance to
something 4.15-rcX based. I could however re-base it now if desired.

>
> Signed-off-by: Peter Enderborg <[email protected]>
> ---
> security/selinux/ss/mls.c | 69 ++++++++++++++++----------------
> security/selinux/ss/mls.h | 37 +++++++++--------
> security/selinux/ss/services.c | 90 +++++++++++++++++++++++++++-----
> ----------
> security/selinux/ss/services.h | 3 --
> 4 files changed, 114 insertions(+), 85 deletions(-)
>
> diff --git a/security/selinux/ss/mls.c b/security/selinux/ss/mls.c
> index ad982ce..b1f35d3 100644
> --- a/security/selinux/ss/mls.c
> +++ b/security/selinux/ss/mls.c
> @@ -33,20 +33,20 @@
> * Return the length in bytes for the MLS fields of the
> * security context string representation of `context'.
> */
> -int mls_compute_context_len(struct context *context)
> +int mls_compute_context_len(struct policydb *p, struct context
> *context)
> {
> int i, l, len, head, prev;
> char *nm;
> struct ebitmap *e;
> struct ebitmap_node *node;
>
> - if (!policydb.mls_enabled)
> + if (!p->mls_enabled)
> return 0;
>
> len = 1; /* for the beginning ":" */
> for (l = 0; l < 2; l++) {
> int index_sens = context->range.level[l].sens;
> - len += strlen(sym_name(&policydb, SYM_LEVELS,
> index_sens - 1));
> + len += strlen(sym_name(p, SYM_LEVELS, index_sens -
> 1));
>
> /* categories */
> head = -2;
> @@ -56,17 +56,17 @@ int mls_compute_context_len(struct context
> *context)
> if (i - prev > 1) {
> /* one or more negative bits are
> skipped */
> if (head != prev) {
> - nm = sym_name(&policydb,
> SYM_CATS, prev);
> + nm = sym_name(p, SYM_CATS,
> prev);
> len += strlen(nm) + 1;
> }
> - nm = sym_name(&policydb, SYM_CATS,
> i);
> + nm = sym_name(p, SYM_CATS, i);
> len += strlen(nm) + 1;
> head = i;
> }
> prev = i;
> }
> if (prev != head) {
> - nm = sym_name(&policydb, SYM_CATS, prev);
> + nm = sym_name(p, SYM_CATS, prev);
> len += strlen(nm) + 1;
> }
> if (l == 0) {
> @@ -86,7 +86,7 @@ int mls_compute_context_len(struct context
> *context)
> * the MLS fields of `context' into the string `*scontext'.
> * Update `*scontext' to point to the end of the MLS fields.
> */
> -void mls_sid_to_context(struct context *context,
> +void mls_sid_to_context(struct policydb *p, struct context *context,
> char **scontext)
> {
> char *scontextp, *nm;
> @@ -94,7 +94,7 @@ void mls_sid_to_context(struct context *context,
> struct ebitmap *e;
> struct ebitmap_node *node;
>
> - if (!policydb.mls_enabled)
> + if (!p->mls_enabled)
> return;
>
> scontextp = *scontext;
> @@ -103,7 +103,7 @@ void mls_sid_to_context(struct context *context,
> scontextp++;
>
> for (l = 0; l < 2; l++) {
> - strcpy(scontextp, sym_name(&policydb, SYM_LEVELS,
> + strcpy(scontextp, sym_name(p, SYM_LEVELS,
> context-
> >range.level[l].sens - 1));
> scontextp += strlen(scontextp);
>
> @@ -119,7 +119,7 @@ void mls_sid_to_context(struct context *context,
> *scontextp++ = '.';
> else
> *scontextp++ = ',';
> - nm = sym_name(&policydb,
> SYM_CATS, prev);
> + nm = sym_name(p, SYM_CATS,
> prev);
> strcpy(scontextp, nm);
> scontextp += strlen(nm);
> }
> @@ -127,7 +127,7 @@ void mls_sid_to_context(struct context *context,
> *scontextp++ = ':';
> else
> *scontextp++ = ',';
> - nm = sym_name(&policydb, SYM_CATS,
> i);
> + nm = sym_name(p, SYM_CATS, i);
> strcpy(scontextp, nm);
> scontextp += strlen(nm);
> head = i;
> @@ -140,7 +140,7 @@ void mls_sid_to_context(struct context *context,
> *scontextp++ = '.';
> else
> *scontextp++ = ',';
> - nm = sym_name(&policydb, SYM_CATS, prev);
> + nm = sym_name(p, SYM_CATS, prev);
> strcpy(scontextp, nm);
> scontextp += strlen(nm);
> }
> @@ -375,12 +375,13 @@ int mls_context_to_sid(struct policydb *pol,
> * the string `str'. This function will allocate temporary memory
> with the
> * given constraints of gfp_mask.
> */
> -int mls_from_string(char *str, struct context *context, gfp_t
> gfp_mask)
> +int mls_from_string(struct policydb *p, char *str, struct context
> *context,
> + gfp_t gfp_mask)
> {
> char *tmpstr, *freestr;
> int rc;
>
> - if (!policydb.mls_enabled)
> + if (!p->mls_enabled)
> return -EINVAL;
>
> /* we need freestr because mls_context_to_sid will change
> @@ -389,7 +390,7 @@ int mls_from_string(char *str, struct context
> *context, gfp_t gfp_mask)
> if (!tmpstr) {
> rc = -ENOMEM;
> } else {
> - rc = mls_context_to_sid(&policydb, ':', &tmpstr,
> context,
> + rc = mls_context_to_sid(p, ':', &tmpstr, context,
> NULL, SECSID_NULL);
> kfree(freestr);
> }
> @@ -417,10 +418,10 @@ int mls_range_set(struct context *context,
> return rc;
> }
>
> -int mls_setup_user_range(struct context *fromcon, struct user_datum
> *user,
> - struct context *usercon)
> +int mls_setup_user_range(struct policydb *p, struct context
> *fromcon,
> + struct user_datum *user, struct context
> *usercon)
> {
> - if (policydb.mls_enabled) {
> + if (p->mls_enabled) {
> struct mls_level *fromcon_sen = &(fromcon-
> >range.level[0]);
> struct mls_level *fromcon_clr = &(fromcon-
> >range.level[1]);
> struct mls_level *user_low = &(user-
> >range.level[0]);
> @@ -460,7 +461,7 @@ int mls_setup_user_range(struct context *fromcon,
> struct user_datum *user,
> * structure `c' from the values specified in the
> * policy `oldp' to the values specified in the policy `newp'.
> */
> -int mls_convert_context(struct policydb *oldp,
> +int mls_convert_context(struct policydb *p, struct policydb *oldp,
> struct policydb *newp,
> struct context *c)
> {
> @@ -470,7 +471,7 @@ int mls_convert_context(struct policydb *oldp,
> struct ebitmap_node *node;
> int l, i;
>
> - if (!policydb.mls_enabled)
> + if (!p->mls_enabled)
> return 0;
>
> for (l = 0; l < 2; l++) {
> @@ -503,7 +504,7 @@ int mls_convert_context(struct policydb *oldp,
> return 0;
> }
>
> -int mls_compute_sid(struct context *scontext,
> +int mls_compute_sid(struct policydb *p, struct context *scontext,
> struct context *tcontext,
> u16 tclass,
> u32 specified,
> @@ -515,7 +516,7 @@ int mls_compute_sid(struct context *scontext,
> struct class_datum *cladatum;
> int default_range = 0;
>
> - if (!policydb.mls_enabled)
> + if (!p->mls_enabled)
> return 0;
>
> switch (specified) {
> @@ -524,12 +525,12 @@ int mls_compute_sid(struct context *scontext,
> rtr.source_type = scontext->type;
> rtr.target_type = tcontext->type;
> rtr.target_class = tclass;
> - r = hashtab_search(policydb.range_tr, &rtr);
> + r = hashtab_search(p->range_tr, &rtr);
> if (r)
> return mls_range_set(newcontext, r);
>
> - if (tclass && tclass <= policydb.p_classes.nprim) {
> - cladatum =
> policydb.class_val_to_struct[tclass - 1];
> + if (tclass && tclass <= p->p_classes.nprim) {
> + cladatum = p->class_val_to_struct[tclass -
> 1];
> if (cladatum)
> default_range = cladatum-
> >default_range;
> }
> @@ -551,7 +552,7 @@ int mls_compute_sid(struct context *scontext,
>
> /* Fallthrough */
> case AVTAB_CHANGE:
> - if ((tclass == policydb.process_class) || (sock ==
> true))
> + if ((tclass == p->process_class) || (sock == true))
> /* Use the process MLS attributes. */
> return mls_context_cpy(newcontext,
> scontext);
> else
> @@ -577,10 +578,10 @@ int mls_compute_sid(struct context *scontext,
> * NetLabel MLS sensitivity level field.
> *
> */
> -void mls_export_netlbl_lvl(struct context *context,
> +void mls_export_netlbl_lvl(struct policydb *p, struct context
> *context,
> struct netlbl_lsm_secattr *secattr)
> {
> - if (!policydb.mls_enabled)
> + if (!p->mls_enabled)
> return;
>
> secattr->attr.mls.lvl = context->range.level[0].sens - 1;
> @@ -597,10 +598,10 @@ void mls_export_netlbl_lvl(struct context
> *context,
> * NetLabel MLS sensitivity level into the context.
> *
> */
> -void mls_import_netlbl_lvl(struct context *context,
> +void mls_import_netlbl_lvl(struct policydb *p, struct context
> *context,
> struct netlbl_lsm_secattr *secattr)
> {
> - if (!policydb.mls_enabled)
> + if (!p->mls_enabled)
> return;
>
> context->range.level[0].sens = secattr->attr.mls.lvl + 1;
> @@ -617,12 +618,12 @@ void mls_import_netlbl_lvl(struct context
> *context,
> * MLS category field. Returns zero on success, negative values on
> failure.
> *
> */
> -int mls_export_netlbl_cat(struct context *context,
> +int mls_export_netlbl_cat(struct policydb *p, struct context
> *context,
> struct netlbl_lsm_secattr *secattr)
> {
> int rc;
>
> - if (!policydb.mls_enabled)
> + if (!p->mls_enabled)
> return 0;
>
> rc = ebitmap_netlbl_export(&context->range.level[0].cat,
> @@ -645,12 +646,12 @@ int mls_export_netlbl_cat(struct context
> *context,
> * negative values on failure.
> *
> */
> -int mls_import_netlbl_cat(struct context *context,
> +int mls_import_netlbl_cat(struct policydb *p, struct context
> *context,
> struct netlbl_lsm_secattr *secattr)
> {
> int rc;
>
> - if (!policydb.mls_enabled)
> + if (!p->mls_enabled)
> return 0;
>
> rc = ebitmap_netlbl_import(&context->range.level[0].cat,
> diff --git a/security/selinux/ss/mls.h b/security/selinux/ss/mls.h
> index 131d762..cb039c0 100644
> --- a/security/selinux/ss/mls.h
> +++ b/security/selinux/ss/mls.h
> @@ -25,8 +25,9 @@
> #include "context.h"
> #include "policydb.h"
>
> -int mls_compute_context_len(struct context *context);
> -void mls_sid_to_context(struct context *context, char **scontext);
> +int mls_compute_context_len(struct policydb *p, struct context
> *context);
> +void mls_sid_to_context(struct policydb *p, struct context *context,
> + char **scontext);
> int mls_context_isvalid(struct policydb *p, struct context *c);
> int mls_range_isvalid(struct policydb *p, struct mls_range *r);
> int mls_level_isvalid(struct policydb *p, struct mls_level *l);
> @@ -38,50 +39,55 @@ int mls_context_to_sid(struct policydb *p,
> struct sidtab *s,
> u32 def_sid);
>
> -int mls_from_string(char *str, struct context *context, gfp_t
> gfp_mask);
> +int mls_from_string(struct policydb *p, char *str, struct context
> *context,
> + gfp_t gfp_mask);
>
> int mls_range_set(struct context *context, struct mls_range *range);
>
> -int mls_convert_context(struct policydb *oldp,
> +int mls_convert_context(struct policydb *p, struct policydb *oldp,
> struct policydb *newp,
> struct context *context);
>
> -int mls_compute_sid(struct context *scontext,
> +int mls_compute_sid(struct policydb *p, struct context *scontext,
> struct context *tcontext,
> u16 tclass,
> u32 specified,
> struct context *newcontext,
> bool sock);
>
> -int mls_setup_user_range(struct context *fromcon, struct user_datum
> *user,
> - struct context *usercon);
> +int mls_setup_user_range(struct policydb *p, struct context
> *fromcon,
> + struct user_datum *user, struct context
> *usercon);
>
> #ifdef CONFIG_NETLABEL
> -void mls_export_netlbl_lvl(struct context *context,
> +void mls_export_netlbl_lvl(struct policydb *p, struct context
> *context,
> struct netlbl_lsm_secattr *secattr);
> -void mls_import_netlbl_lvl(struct context *context,
> +void mls_import_netlbl_lvl(struct policydb *p, struct context
> *context,
> struct netlbl_lsm_secattr *secattr);
> -int mls_export_netlbl_cat(struct context *context,
> +int mls_export_netlbl_cat(struct policydb *p, struct context
> *context,
> struct netlbl_lsm_secattr *secattr);
> -int mls_import_netlbl_cat(struct context *context,
> +int mls_import_netlbl_cat(struct policydb *p, struct context
> *context,
> struct netlbl_lsm_secattr *secattr);
> #else
> -static inline void mls_export_netlbl_lvl(struct context *context,
> +static inline void mls_export_netlbl_lvl(struct policydb *p,
> + struct context *context,
> struct netlbl_lsm_secattr
> *secattr)
> {
> return;
> }
> -static inline void mls_import_netlbl_lvl(struct context *context,
> +static inline void mls_import_netlbl_lvl(struct policydb *p,
> + struct context *context,
> struct netlbl_lsm_secattr
> *secattr)
> {
> return;
> }
> -static inline int mls_export_netlbl_cat(struct context *context,
> +static inline int mls_export_netlbl_cat(struct policydb *p,
> + struct context *context,
> struct netlbl_lsm_secattr
> *secattr)
> {
> return -ENOMEM;
> }
> -static inline int mls_import_netlbl_cat(struct context *context,
> +static inline int mls_import_netlbl_cat(struct policydb *p,
> + struct context *context,
> struct netlbl_lsm_secattr
> *secattr)
> {
> return -ENOMEM;
> @@ -89,4 +95,3 @@ static inline int mls_import_netlbl_cat(struct
> context *context,
> #endif
>
> #endif /* _SS_MLS_H */
> -
> diff --git a/security/selinux/ss/services.c
> b/security/selinux/ss/services.c
> index 33cfe5d..47d8030 100644
> --- a/security/selinux/ss/services.c
> +++ b/security/selinux/ss/services.c
> @@ -90,7 +90,7 @@ int selinux_policycap_nnp_nosuid_transition;
> static DEFINE_RWLOCK(policy_rwlock);
>
> static struct sidtab sidtab;
> -struct policydb policydb;
> +static struct policydb policydb;
> int ss_initialized;
>
> /*
> @@ -117,8 +117,12 @@ struct selinux_mapping {
> u32 perms[sizeof(u32) * 8];
> };
>
> -static struct selinux_mapping *current_mapping;
> -static u16 current_mapping_size;
> +struct shared_current_mapping {
> + struct selinux_mapping *current_mapping;
> + u16 current_mapping_size;
> +};
> +
> +static struct shared_current_mapping *crm;
>
> static int selinux_set_mapping(struct policydb *pol,
> struct security_class_mapping *map,
> @@ -208,8 +212,8 @@ static int selinux_set_mapping(struct policydb
> *pol,
>
> static u16 unmap_class(u16 tclass)
> {
> - if (tclass < current_mapping_size)
> - return current_mapping[tclass].value;
> + if (tclass < crm->current_mapping_size)
> + return crm->current_mapping[tclass].value;
>
> return tclass;
> }
> @@ -221,8 +225,8 @@ static u16 map_class(u16 pol_value)
> {
> u16 i;
>
> - for (i = 1; i < current_mapping_size; i++) {
> - if (current_mapping[i].value == pol_value)
> + for (i = 1; i < crm->current_mapping_size; i++) {
> + if (crm->current_mapping[i].value == pol_value)
> return i;
> }
>
> @@ -232,27 +236,32 @@ static u16 map_class(u16 pol_value)
> static void map_decision(u16 tclass, struct av_decision *avd,
> int allow_unknown)
> {
> - if (tclass < current_mapping_size) {
> - unsigned i, n = current_mapping[tclass].num_perms;
> + if (tclass < crm->current_mapping_size) {
> + unsigned int i, n = crm-
> >current_mapping[tclass].num_perms;
> u32 result;
>
> for (i = 0, result = 0; i < n; i++) {
> - if (avd->allowed &
> current_mapping[tclass].perms[i])
> + if (avd->allowed &
> + crm->current_mapping[tclass].perms[i])
> result |= 1<<i;
> - if (allow_unknown &&
> !current_mapping[tclass].perms[i])
> + if (allow_unknown &&
> + !crm->current_mapping[tclass].perms[i])
> result |= 1<<i;
> }
> avd->allowed = result;
>
> for (i = 0, result = 0; i < n; i++)
> - if (avd->auditallow &
> current_mapping[tclass].perms[i])
> + if (avd->auditallow &
> + crm->current_mapping[tclass].perms[i])
> result |= 1<<i;
> avd->auditallow = result;
>
> for (i = 0, result = 0; i < n; i++) {
> - if (avd->auditdeny &
> current_mapping[tclass].perms[i])
> + if (avd->auditdeny &
> + crm->current_mapping[tclass].perms[i])
> result |= 1<<i;
> - if (!allow_unknown &&
> !current_mapping[tclass].perms[i])
> + if (!allow_unknown &&
> + !crm->current_mapping[tclass].perms[i])
> result |= 1<<i;
> }
> /*
> @@ -1211,7 +1220,7 @@ static int context_struct_to_string(struct
> context *context, char **scontext, u3
> *scontext_len += strlen(sym_name(&policydb, SYM_USERS,
> context->user - 1)) + 1;
> *scontext_len += strlen(sym_name(&policydb, SYM_ROLES,
> context->role - 1)) + 1;
> *scontext_len += strlen(sym_name(&policydb, SYM_TYPES,
> context->type - 1)) + 1;
> - *scontext_len += mls_compute_context_len(context);
> + *scontext_len += mls_compute_context_len(&policydb,
> context);
>
> if (!scontext)
> return 0;
> @@ -1230,7 +1239,7 @@ static int context_struct_to_string(struct
> context *context, char **scontext, u3
> sym_name(&policydb, SYM_ROLES, context->role - 1),
> sym_name(&policydb, SYM_TYPES, context->type - 1));
>
> - mls_sid_to_context(context, &scontextp);
> + mls_sid_to_context(&policydb, context, &scontextp);
>
> *scontextp = 0;
>
> @@ -1721,7 +1730,7 @@ static int security_compute_sid(u32 ssid,
>
> /* Set the MLS attributes.
> This is done last because it may allocate memory. */
> - rc = mls_compute_sid(scontext, tcontext, tclass, specified,
> + rc = mls_compute_sid(&policydb, scontext, tcontext, tclass,
> specified,
> &newcontext, sock);
> if (rc)
> goto out_unlock;
> @@ -1935,7 +1944,7 @@ static int convert_context(u32 key,
>
> /* Convert the MLS fields if dealing with MLS policies */
> if (args->oldp->mls_enabled && args->newp->mls_enabled) {
> - rc = mls_convert_context(args->oldp, args->newp, c);
> + rc = mls_convert_context(&policydb, args->oldp,
> args->newp, c);
> if (rc)
> goto bad;
> } else if (args->oldp->mls_enabled && !args->newp-
> >mls_enabled) {
> @@ -2043,8 +2052,9 @@ int security_load_policy(void *data, size_t
> len)
> {
> struct policydb *oldpolicydb, *newpolicydb;
> struct sidtab oldsidtab, newsidtab;
> - struct selinux_mapping *oldmap, *map = NULL;
> + struct selinux_mapping *oldmap = NULL, *map = NULL;
> struct convert_context_args args;
> + struct shared_current_mapping *new_mapping;
> u32 seqno;
> u16 map_size;
> int rc = 0;
> @@ -2055,9 +2065,22 @@ int security_load_policy(void *data, size_t
> len)
> rc = -ENOMEM;
> goto out;
> }
> + new_mapping = kzalloc(sizeof(struct shared_current_mapping),
> + GFP_KERNEL);
> + if (!new_mapping) {
> + rc = -ENOMEM;
> + goto out;
> + }
> newpolicydb = oldpolicydb + 1;
>
> if (!ss_initialized) {
> + crm = kzalloc(sizeof(struct shared_current_mapping),
> + GFP_KERNEL);
> + if (!crm) {
> + rc = -ENOMEM;
> + goto out;
> + }
> +
> avtab_cache_init();
> ebitmap_cache_init();
> hashtab_cache_init();
> @@ -2071,8 +2094,8 @@ int security_load_policy(void *data, size_t
> len)
>
> policydb.len = len;
> rc = selinux_set_mapping(&policydb, secclass_map,
> - &current_mapping,
> - &current_mapping_size);
> + &crm->current_mapping,
> + &crm-
> >current_mapping_size);
> if (rc) {
> policydb_destroy(&policydb);
> avtab_cache_destroy();
> @@ -2164,9 +2187,9 @@ int security_load_policy(void *data, size_t
> len)
> memcpy(&policydb, newpolicydb, sizeof(policydb));
> sidtab_set(&sidtab, &newsidtab);
> security_load_policycaps();
> - oldmap = current_mapping;
> - current_mapping = map;
> - current_mapping_size = map_size;
> + oldmap = crm->current_mapping;
> + crm->current_mapping = map;
> + crm->current_mapping_size = map_size;
> seqno = ++latest_granting;
> write_unlock_irq(&policy_rwlock);
>
> @@ -2516,7 +2539,8 @@ int security_get_user_sids(u32 fromsid,
> ebitmap_for_each_positive_bit(&role->types, tnode,
> j) {
> usercon.type = j + 1;
>
> - if (mls_setup_user_range(fromcon, user,
> &usercon))
> + if (mls_setup_user_range(&policydb, fromcon,
> + user, &usercon))
> continue;
>
> rc = sidtab_context_to_sid(&sidtab,
> &usercon, &sid);
> @@ -2580,7 +2604,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 hold rcu before calling this function.
> */
> static inline int __security_genfs_sid(const char *fstype,
> char *path,
> @@ -2639,7 +2663,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
> + * Hold rcu before calling __security_genfs_sid() and release
> * it afterward.
> */
> int security_genfs_sid(const char *fstype,
> @@ -3214,7 +3238,8 @@ int selinux_audit_rule_init(u32 field, u32 op,
> char *rulestr, void **vrule)
> case AUDIT_SUBJ_CLR:
> case AUDIT_OBJ_LEV_LOW:
> case AUDIT_OBJ_LEV_HIGH:
> - rc = mls_from_string(rulestr, &tmprule->au_ctxt,
> GFP_ATOMIC);
> + rc = mls_from_string(&policydb, rulestr, &tmprule-
> >au_ctxt,
> + GFP_ATOMIC);
> if (rc)
> goto out;
> break;
> @@ -3464,9 +3489,10 @@ int security_netlbl_secattr_to_sid(struct
> netlbl_lsm_secattr *secattr,
> ctx_new.user = ctx->user;
> ctx_new.role = ctx->role;
> ctx_new.type = ctx->type;
> - mls_import_netlbl_lvl(&ctx_new, secattr);
> + mls_import_netlbl_lvl(&policydb, &ctx_new, secattr);
> if (secattr->flags & NETLBL_SECATTR_MLS_CAT) {
> - rc = mls_import_netlbl_cat(&ctx_new,
> secattr);
> + rc = mls_import_netlbl_cat(&policydb,
> &ctx_new,
> + secattr);
> if (rc)
> goto out;
> }
> @@ -3526,8 +3552,8 @@ int security_netlbl_sid_to_secattr(u32 sid,
> struct netlbl_lsm_secattr *secattr)
>
> secattr->attr.secid = sid;
> secattr->flags |= NETLBL_SECATTR_DOMAIN_CPY |
> NETLBL_SECATTR_SECID;
> - mls_export_netlbl_lvl(ctx, secattr);
> - rc = mls_export_netlbl_cat(ctx, secattr);
> + mls_export_netlbl_lvl(&policydb, ctx, secattr);
> + rc = mls_export_netlbl_cat(&policydb, ctx, secattr);
> out:
> read_unlock(&policy_rwlock);
> return rc;
> diff --git a/security/selinux/ss/services.h
> b/security/selinux/ss/services.h
> index 356bdd3..50c7ceb 100644
> --- a/security/selinux/ss/services.h
> +++ b/security/selinux/ss/services.h
> @@ -10,8 +10,6 @@
> #include "policydb.h"
> #include "sidtab.h"
>
> -extern struct policydb policydb;
> -
> void services_compute_xperms_drivers(struct extended_perms *xperms,
> struct avtab_node *node);
>
> @@ -19,4 +17,3 @@ void services_compute_xperms_decision(struct
> extended_perms_decision *xpermd,
> struct avtab_node *node);
>
> #endif /* _SS_SERVICES_H_ */
> -

2018-01-30 14:21:26

by Stephen Smalley

[permalink] [raw]
Subject: Re: [PATCH v2 0/5] selinux:Significant reduce of preempt_disable holds

On Fri, 2018-01-26 at 15:32 +0100, [email protected] wrote:
> 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 RCU instead of rwlock we
> have an efficient locking and less preemption interference.

NB: rcu_read_lock() may disable preemption as well if
CONFIG_PREEMPT_COUNT=y. I assume you aren't concerned with that
configuration?

>
> Selinux uses a lot of read_locks. This patch replaces the rwlock
> with RCY that does not hold preempt_disable.
>
> Intel Xeon W3520 2.67 Ghz running FC27 with 4.15.0-rc9git
> (+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 RCU usage.
>
> To use RCU the structure of policydb has to be accesses through a
> pointer.
> We need 4 patches to get there.
>
> [PATCH v2 1/5] selinux:Remove direct references to policydb.
> We remove direct references and pass it through function arguments.
>
> [PATCH v2 2/5] selinux: Move policydb to pointer structure
> Move the policydb to dynamic allocated structure.
>
> [PATCH v2 3/5] selinux: Move sidtab to pointer structure
> Same as for policydb but for sidtab. They are closly related
> and should be switched at the same time.
>
> [PATCH v2 4/5] selinux: Use pointer to switch policydb and sidtab
> Now we can switch rules by switching pointers.
>
> [PATCH v2 5/5] selinux: Switch locking to RCU.
> We are now ready to use RCU.
>
> History: V1 rwsem
>

2018-01-30 14:42:02

by Stephen Smalley

[permalink] [raw]
Subject: Re: [PATCH v2 4/5] selinux: Use pointer to switch policydb and sidtab

On Fri, 2018-01-26 at 15:32 +0100, [email protected] wrote:
> From: Peter Enderborg <[email protected]>
>
> This i preparation for switching to RCU locks. To be able to use
> RCU we need atomic switched pointer. This adds the dynamic
> memory copying to be a single pointer. It copy all the
> data structures in to new ones. This is an overhead
> for writing rules but the benifit is RCU.
>
> Signed-off-by: Peter Enderborg <[email protected]>
> ---
> security/selinux/ss/services.c | 139 +++++++++++++++++++++++------
> ------------
> 1 file changed, 78 insertions(+), 61 deletions(-)
>
> diff --git a/security/selinux/ss/services.c
> b/security/selinux/ss/services.c
> index 2a8486c..81c5717 100644
> --- a/security/selinux/ss/services.c
> +++ b/security/selinux/ss/services.c
> @@ -2064,76 +2064,67 @@ static int security_preserve_bools(struct
> policydb *p);
> */
> int security_load_policy(void *data, size_t len)
> {
> - struct policydb *oldpolicydb, *newpolicydb;
> + struct policydb *oldpolicydb;
> struct sidtab oldsidtab, newsidtab;
> struct selinux_mapping *oldmap = NULL, *map = NULL;
> struct convert_context_args args;
> - struct shared_current_mapping *new_mapping;
> struct shared_current_mapping *next_rcu;
> -
> + struct shared_current_mapping *old_rcu;
> u32 seqno;
> u16 map_size;
> int rc = 0;
> struct policy_file file = { data, len }, *fp = &file;
>
> - oldpolicydb = kzalloc(2 * sizeof(*oldpolicydb), GFP_KERNEL);
> - if (!oldpolicydb) {
> - rc = -ENOMEM;
> - goto out;
> - }
> - new_mapping = kzalloc(sizeof(struct shared_current_mapping),
> - GFP_KERNEL);
> - if (!new_mapping) {
> - rc = -ENOMEM;
> - goto out;
> - }
> - newpolicydb = oldpolicydb + 1;
> - next_rcu = kmalloc(sizeof(struct shared_current_mapping),
> GFP_KERNEL);
> - if (!next_rcu) {
> - rc = -ENOMEM;
> - goto out;
> - }
> -
> if (!ss_initialized) {
> - crm = kzalloc(sizeof(struct shared_current_mapping),
> - GFP_KERNEL);
> - if (!crm) {
> + struct shared_current_mapping *first_mapping;
> +
> + first_mapping = kzalloc(sizeof(struct
> shared_current_mapping),
> + GFP_KERNEL);
> + if (!first_mapping) {
> rc = -ENOMEM;
> goto out;
> }
> avtab_cache_init();
> ebitmap_cache_init();
> hashtab_cache_init();
> - rc = policydb_read(&crm->policydb, fp);
> + rc = policydb_read(&first_mapping->policydb, fp);
> if (rc) {
> avtab_cache_destroy();
> ebitmap_cache_destroy();
> hashtab_cache_destroy();
> + kfree(first_mapping);
> goto out;
> }
>
> - crm->policydb.len = len;
> - rc = selinux_set_mapping(&crm->policydb,
> secclass_map,
> - &crm->current_mapping,
> - &crm-
> >current_mapping_size);
> + first_mapping->policydb.len = len;
> + rc = selinux_set_mapping(&first_mapping->policydb,
> secclass_map,
> + &first_mapping-
> >current_mapping,
> + &first_mapping-
> >current_mapping_size);
> if (rc) {
> - policydb_destroy(&crm->policydb);
> + policydb_destroy(&first_mapping->policydb);
> avtab_cache_destroy();
> ebitmap_cache_destroy();
> hashtab_cache_destroy();
> + kfree(first_mapping);
> goto out;
> }
>
> - rc = policydb_load_isids(&crm->policydb, &crm-
> >sidtab);
> + rc = policydb_load_isids(&first_mapping->policydb,
> + &first_mapping->sidtab);
> if (rc) {
> - policydb_destroy(&crm->policydb);
> + policydb_destroy(&first_mapping->policydb);
> avtab_cache_destroy();
> ebitmap_cache_destroy();
> hashtab_cache_destroy();
> + kfree(first_mapping);
> goto out;
> }
>
> - security_load_policycaps(&crm->policydb);
> + security_load_policycaps(&first_mapping->policydb);
> + crm = first_mapping;
> +
> + smp_mb(); /* make sure that crm exist before we */
> + /* switch ss_initialized */
> ss_initialized = 1;
> seqno = ++latest_granting;
> selinux_complete_init();
> @@ -2148,30 +2139,44 @@ int security_load_policy(void *data, size_t
> len)
> #if 0
> sidtab_hash_eval(&crm->sidtab, "sids");
> #endif
> + oldpolicydb = kzalloc(sizeof(*oldpolicydb), GFP_KERNEL);
> + if (!oldpolicydb) {
> + rc = -ENOMEM;
> + goto out;
> + }
> +
> + next_rcu = kzalloc(sizeof(struct shared_current_mapping),
> GFP_KERNEL);
> + if (!next_rcu) {
> + kfree(oldpolicydb);
> + rc = -ENOMEM;
> + goto out;
> + }
>
> - rc = policydb_read(newpolicydb, fp);
> + rc = policydb_read(&next_rcu->policydb, fp);
> if (rc)
> goto out;
>
> - newpolicydb->len = len;
> + next_rcu->policydb.len = len;
> + read_lock(&policy_rwlock);
> /* If switching between different policy types, log MLS
> status */
> - if (crm->policydb.mls_enabled && !newpolicydb->mls_enabled)
> + if (crm->policydb.mls_enabled && !next_rcu-
> >policydb.mls_enabled)
> printk(KERN_INFO "SELinux: Disabling MLS
> support...\n");
> - else if (!crm->policydb.mls_enabled && newpolicydb-
> >mls_enabled)
> + else if (!crm->policydb.mls_enabled && next_rcu-
> >policydb.mls_enabled)
> printk(KERN_INFO "SELinux: Enabling MLS
> support...\n");
>
> - rc = policydb_load_isids(newpolicydb, &newsidtab);
> + rc = policydb_load_isids(&next_rcu->policydb, &newsidtab);
> if (rc) {
> printk(KERN_ERR "SELinux: unable to load the
> initial SIDs\n");
> - policydb_destroy(newpolicydb);
> + policydb_destroy(&next_rcu->policydb);
> goto out;
> }
>
> - rc = selinux_set_mapping(newpolicydb, secclass_map, &map,
> &map_size);
> + rc = selinux_set_mapping(&next_rcu->policydb, secclass_map,
> + &map, &map_size);
> if (rc)
> goto err;
>
> - rc = security_preserve_bools(newpolicydb);
> + rc = security_preserve_bools(&next_rcu->policydb);
> if (rc) {
> printk(KERN_ERR "SELinux: unable to preserve
> booleans\n");
> goto err;

Most of this shouldn't need to be under the read lock.

> @@ -2189,7 +2194,7 @@ int security_load_policy(void *data, size_t
> len)
> * in the new SID table.
> */
> args.oldp = &crm->policydb;
> - args.newp = newpolicydb;
> + args.newp = &next_rcu->policydb;
> rc = sidtab_map(&newsidtab, convert_context, &args);
> if (rc) {
> printk(KERN_ERR "SELinux: unable to convert the
> internal"
> @@ -2204,8 +2209,9 @@ int security_load_policy(void *data, size_t
> len)
>
> /* Install the new policydb and SID table. */
> /* next */
> + security_load_policycaps(&next_rcu->policydb);

This cannot be done outside of the write lock; it has to be atomic with
the policy switch.

> + read_unlock(&policy_rwlock);
> write_lock_irq(&policy_rwlock);
> - memcpy(&next_rcu->policydb, newpolicydb, sizeof(struct
> policydb));
> sidtab_set(&next_rcu->sidtab, &newsidtab);
> security_load_policycaps(&next_rcu->policydb);
> oldmap = crm->current_mapping;
> @@ -2213,8 +2219,9 @@ int security_load_policy(void *data, size_t
> len)
> next_rcu->current_mapping_size = map_size;
>
> seqno = ++latest_granting;
> - write_unlock_irq(&policy_rwlock);
> + old_rcu = crm;
> crm = next_rcu;
> + write_unlock_irq(&policy_rwlock);
>
> /* Free the old policydb and SID table. */
> policydb_destroy(oldpolicydb);
> @@ -2226,17 +2233,16 @@ int security_load_policy(void *data, size_t
> len)
> selinux_status_update_policyload(seqno);
> selinux_netlbl_cache_invalidate();
> selinux_xfrm_notify_policyload();
> + kfree(oldpolicydb);
> + kfree(old_rcu);
>
> rc = 0;
> goto out;
> -
> err:
> kfree(map);
> sidtab_destroy(&newsidtab);
> - policydb_destroy(newpolicydb);
> -
> + policydb_destroy(&next_rcu->policydb);
> out:
> - kfree(oldpolicydb);
> return rc;
> }
>
> @@ -2795,54 +2801,65 @@ int security_get_bools(int *len, char
> ***names, int **values)
> goto out;
> }
>
> -
> int security_set_bools(int len, int *values)
> {
> + struct shared_current_mapping *next_rcu, *old_rcu;
> int i, rc;
> int lenp, seqno = 0;
> struct cond_node *cur;
>
> - write_lock_irq(&policy_rwlock);
> -
> + next_rcu = kzalloc(sizeof(struct shared_current_mapping),
> GFP_KERNEL);
> + read_lock(&policy_rwlock);
> + old_rcu = crm;
> + memcpy(&next_rcu->policydb, &old_rcu->policydb,
> + sizeof(struct policydb));

You are only doing a "shallow" copy of the policydb here, which
contains pointers to other structures. So then below when you modify
state, you are modifying the original, not just the copy. And you'll
end up double freeing if you free them both.

For reference, attached is a very old attempt to convert the policy
rwlock to RCU from KaiGai Kohei. It may provide some insight into what
is needed here.

> rc = -EFAULT;
> - lenp = crm->policydb.p_bools.nprim;
> + lenp = next_rcu->policydb.p_bools.nprim;
> +
> if (len != lenp)
> goto out;
>
> for (i = 0; i < len; i++) {
> if (!!values[i] !=
> - crm->policydb.bool_val_to_struct[i]->state) {
> + next_rcu->policydb.bool_val_to_struct[i]->state)
> {
> audit_log(current->audit_context,
> GFP_ATOMIC,
> AUDIT_MAC_CONFIG_CHANGE,
> "bool=%s val=%d old_val=%d auid=%u
> ses=%u",
> - sym_name(&crm->policydb, SYM_BOOLS,
> i),
> + sym_name(&next_rcu->policydb,
> SYM_BOOLS, i),
> !!values[i],
> - crm->policydb.bool_val_to_struct[i]-
> >state,
> + next_rcu-
> >policydb.bool_val_to_struct[i]->state,
> from_kuid(&init_user_ns,
> audit_get_loginuid(current)),
> audit_get_sessionid(current));
> }
> if (values[i])
> - crm->policydb.bool_val_to_struct[i]->state =
> 1;
> + next_rcu->policydb.bool_val_to_struct[i]-
> >state = 1;
> else
> - crm->policydb.bool_val_to_struct[i]->state =
> 0;
> + next_rcu->policydb.bool_val_to_struct[i]-
> >state = 0;
> }
>
> - for (cur = crm->policydb.cond_list; cur; cur = cur->next) {
> - rc = evaluate_cond_node(&crm->policydb, cur);
> + for (cur = next_rcu->policydb.cond_list; cur; cur = cur-
> >next) {
> + rc = evaluate_cond_node(&next_rcu->policydb, cur);
> if (rc)
> goto out;
> }
> + read_unlock(&policy_rwlock);
> + rc = 0;
>
> + write_lock_irq(&policy_rwlock);
> seqno = ++latest_granting;
> - rc = 0;
> -out:
> + crm = next_rcu;
> write_unlock_irq(&policy_rwlock);
> +out:
> if (!rc) {
> avc_ss_reset(seqno);
> selnl_notify_policyload(seqno);
> selinux_status_update_policyload(seqno);
> selinux_xfrm_notify_policyload();
> + } else {
> + kfree(next_rcu);
> }
> + kfree(old_rcu);
> +
> return rc;
> }
>


Attachments:
selinux-nolock-policydb-041110.patch (42.69 kB)