From 339949be25863ac15e24659c2ab4b01185e1234a Mon Sep 17 00:00:00 2001 From: Stephen Smalley Date: Thu, 6 Aug 2020 14:34:18 -0400 Subject: scripts/selinux,selinux: update mdp to enable policy capabilities Presently mdp does not enable any SELinux policy capabilities in the dummy policy it generates. Thus, policies derived from it will by default lack various features commonly used in modern policies such as open permission, extended socket classes, network peer controls, etc. Split the policy capability definitions out into their own headers so that we can include them into mdp without pulling in other kernel headers and extend mdp generate policycap statements for the policy capabilities known to the kernel. Policy authors may wish to selectively remove some of these from the generated policy. Signed-off-by: Stephen Smalley Signed-off-by: Paul Moore --- security/selinux/include/policycap.h | 20 ++++++++++++++++++++ security/selinux/include/policycap_names.h | 18 ++++++++++++++++++ security/selinux/include/security.h | 16 +--------------- security/selinux/ss/services.c | 12 +----------- 4 files changed, 40 insertions(+), 26 deletions(-) create mode 100644 security/selinux/include/policycap.h create mode 100644 security/selinux/include/policycap_names.h (limited to 'security') diff --git a/security/selinux/include/policycap.h b/security/selinux/include/policycap.h new file mode 100644 index 000000000000..2ec038efbb03 --- /dev/null +++ b/security/selinux/include/policycap.h @@ -0,0 +1,20 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#ifndef _SELINUX_POLICYCAP_H_ +#define _SELINUX_POLICYCAP_H_ + +/* Policy capabilities */ +enum { + POLICYDB_CAPABILITY_NETPEER, + POLICYDB_CAPABILITY_OPENPERM, + POLICYDB_CAPABILITY_EXTSOCKCLASS, + POLICYDB_CAPABILITY_ALWAYSNETWORK, + POLICYDB_CAPABILITY_CGROUPSECLABEL, + POLICYDB_CAPABILITY_NNP_NOSUID_TRANSITION, + POLICYDB_CAPABILITY_GENFS_SECLABEL_SYMLINKS, + __POLICYDB_CAPABILITY_MAX +}; +#define POLICYDB_CAPABILITY_MAX (__POLICYDB_CAPABILITY_MAX - 1) + +extern const char *selinux_policycap_names[__POLICYDB_CAPABILITY_MAX]; + +#endif /* _SELINUX_POLICYCAP_H_ */ diff --git a/security/selinux/include/policycap_names.h b/security/selinux/include/policycap_names.h new file mode 100644 index 000000000000..b89289f092c9 --- /dev/null +++ b/security/selinux/include/policycap_names.h @@ -0,0 +1,18 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#ifndef _SELINUX_POLICYCAP_NAMES_H_ +#define _SELINUX_POLICYCAP_NAMES_H_ + +#include "policycap.h" + +/* Policy capability names */ +const char *selinux_policycap_names[__POLICYDB_CAPABILITY_MAX] = { + "network_peer_controls", + "open_perms", + "extended_socket_class", + "always_check_network", + "cgroup_seclabel", + "nnp_nosuid_transition", + "genfs_seclabel_symlinks" +}; + +#endif /* _SELINUX_POLICYCAP_NAMES_H_ */ diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h index b0e02cfe3ce1..02dd91c12235 100644 --- a/security/selinux/include/security.h +++ b/security/selinux/include/security.h @@ -16,6 +16,7 @@ #include #include #include "flask.h" +#include "policycap.h" #define SECSID_NULL 0x00000000 /* unspecified SID */ #define SECSID_WILD 0xffffffff /* wildcard SID */ @@ -72,21 +73,6 @@ struct netlbl_lsm_secattr; extern int selinux_enabled_boot; -/* Policy capabilities */ -enum { - POLICYDB_CAPABILITY_NETPEER, - POLICYDB_CAPABILITY_OPENPERM, - POLICYDB_CAPABILITY_EXTSOCKCLASS, - POLICYDB_CAPABILITY_ALWAYSNETWORK, - POLICYDB_CAPABILITY_CGROUPSECLABEL, - POLICYDB_CAPABILITY_NNP_NOSUID_TRANSITION, - POLICYDB_CAPABILITY_GENFS_SECLABEL_SYMLINKS, - __POLICYDB_CAPABILITY_MAX -}; -#define POLICYDB_CAPABILITY_MAX (__POLICYDB_CAPABILITY_MAX - 1) - -extern const char *selinux_policycap_names[__POLICYDB_CAPABILITY_MAX]; - /* * type_datum properties * available at the kernel policy version >= POLICYDB_VERSION_BOUNDARY diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c index 1caf4e603309..676550103fac 100644 --- a/security/selinux/ss/services.c +++ b/security/selinux/ss/services.c @@ -64,17 +64,7 @@ #include "xfrm.h" #include "ebitmap.h" #include "audit.h" - -/* Policy capability names */ -const char *selinux_policycap_names[__POLICYDB_CAPABILITY_MAX] = { - "network_peer_controls", - "open_perms", - "extended_socket_class", - "always_check_network", - "cgroup_seclabel", - "nnp_nosuid_transition", - "genfs_seclabel_symlinks" -}; +#include "policycap_names.h" static struct selinux_ss selinux_ss; -- cgit v1.2.3 From 461698026ffa740253b3114891d86eeb54a58bbc Mon Sep 17 00:00:00 2001 From: Stephen Smalley Date: Fri, 7 Aug 2020 09:29:33 -0400 Subject: selinux: encapsulate policy state, refactor policy load Encapsulate the policy state in its own structure (struct selinux_policy) that is separately allocated but referenced from the selinux_ss structure. The policy state includes the SID table (particularly the context structures), the policy database, and the mapping between the kernel classes/permissions and the policy values. Refactor the security server portion of the policy load logic to cleanly separate loading of the new structures from committing the new policy. Unify the initial policy load and reload code paths as much as possible, avoiding duplicated code. Make sure we are taking the policy read-lock prior to any dereferencing of the policy. Move the copying of the policy capability booleans into the state structure outside of the policy write-lock because they are separate from the policy and are read outside of any policy lock; possibly they should be using at least READ_ONCE/WRITE_ONCE or smp_load_acquire/store_release. These changes simplify the policy loading logic, reduce the size of the critical section while holding the policy write-lock, and should facilitate future changes to e.g. refactor the entire policy reload logic including the selinuxfs code to make the updating of the policy and the selinuxfs directory tree atomic and/or to convert the policy read-write lock to RCU. Signed-off-by: Stephen Smalley Signed-off-by: Paul Moore --- security/selinux/ss/services.c | 403 ++++++++++++++++++++++------------------- security/selinux/ss/services.h | 10 +- 2 files changed, 221 insertions(+), 192 deletions(-) (limited to 'security') diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c index 676550103fac..137d9396742a 100644 --- a/security/selinux/ss/services.c +++ b/security/selinux/ss/services.c @@ -238,9 +238,15 @@ static void map_decision(struct selinux_map *map, int security_mls_enabled(struct selinux_state *state) { - struct policydb *p = &state->ss->policydb; + int mls_enabled; - return p->mls_enabled; + if (!selinux_initialized(state)) + return 0; + + read_lock(&state->ss->policy_rwlock); + mls_enabled = state->ss->policy->policydb.mls_enabled; + read_unlock(&state->ss->policy_rwlock); + return mls_enabled; } /* @@ -716,8 +722,8 @@ static int security_validtrans_handle_fail(struct selinux_state *state, struct sidtab_entry *tentry, u16 tclass) { - struct policydb *p = &state->ss->policydb; - struct sidtab *sidtab = state->ss->sidtab; + struct policydb *p = &state->ss->policy->policydb; + struct sidtab *sidtab = &state->ss->policy->sidtab; char *o = NULL, *n = NULL, *t = NULL; u32 olen, nlen, tlen; @@ -761,11 +767,11 @@ static int security_compute_validatetrans(struct selinux_state *state, read_lock(&state->ss->policy_rwlock); - policydb = &state->ss->policydb; - sidtab = state->ss->sidtab; + policydb = &state->ss->policy->policydb; + sidtab = &state->ss->policy->sidtab; if (!user) - tclass = unmap_class(&state->ss->map, orig_tclass); + tclass = unmap_class(&state->ss->policy->map, orig_tclass); else tclass = orig_tclass; @@ -862,8 +868,8 @@ int security_bounded_transition(struct selinux_state *state, read_lock(&state->ss->policy_rwlock); - policydb = &state->ss->policydb; - sidtab = state->ss->sidtab; + policydb = &state->ss->policy->policydb; + sidtab = &state->ss->policy->sidtab; rc = -EINVAL; old_entry = sidtab_search_entry(sidtab, old_sid); @@ -1019,8 +1025,8 @@ void security_compute_xperms_decision(struct selinux_state *state, if (!selinux_initialized(state)) goto allow; - policydb = &state->ss->policydb; - sidtab = state->ss->sidtab; + policydb = &state->ss->policy->policydb; + sidtab = &state->ss->policy->sidtab; scontext = sidtab_search(sidtab, ssid); if (!scontext) { @@ -1036,7 +1042,7 @@ void security_compute_xperms_decision(struct selinux_state *state, goto out; } - tclass = unmap_class(&state->ss->map, orig_tclass); + tclass = unmap_class(&state->ss->policy->map, orig_tclass); if (unlikely(orig_tclass && !tclass)) { if (policydb->allow_unknown) goto allow; @@ -1104,8 +1110,8 @@ void security_compute_av(struct selinux_state *state, if (!selinux_initialized(state)) goto allow; - policydb = &state->ss->policydb; - sidtab = state->ss->sidtab; + policydb = &state->ss->policy->policydb; + sidtab = &state->ss->policy->sidtab; scontext = sidtab_search(sidtab, ssid); if (!scontext) { @@ -1125,7 +1131,7 @@ void security_compute_av(struct selinux_state *state, goto out; } - tclass = unmap_class(&state->ss->map, orig_tclass); + tclass = unmap_class(&state->ss->policy->map, orig_tclass); if (unlikely(orig_tclass && !tclass)) { if (policydb->allow_unknown) goto allow; @@ -1133,7 +1139,7 @@ void security_compute_av(struct selinux_state *state, } context_struct_compute_av(policydb, scontext, tcontext, tclass, avd, xperms); - map_decision(&state->ss->map, orig_tclass, avd, + map_decision(&state->ss->policy->map, orig_tclass, avd, policydb->allow_unknown); out: read_unlock(&state->ss->policy_rwlock); @@ -1158,8 +1164,8 @@ void security_compute_av_user(struct selinux_state *state, if (!selinux_initialized(state)) goto allow; - policydb = &state->ss->policydb; - sidtab = state->ss->sidtab; + policydb = &state->ss->policy->policydb; + sidtab = &state->ss->policy->sidtab; scontext = sidtab_search(sidtab, ssid); if (!scontext) { @@ -1282,7 +1288,7 @@ int security_sidtab_hash_stats(struct selinux_state *state, char *page) } read_lock(&state->ss->policy_rwlock); - rc = sidtab_hash_stats(state->ss->sidtab, page); + rc = sidtab_hash_stats(&state->ss->policy->sidtab, page); read_unlock(&state->ss->policy_rwlock); return rc; @@ -1330,8 +1336,8 @@ static int security_sid_to_context_core(struct selinux_state *state, return -EINVAL; } read_lock(&state->ss->policy_rwlock); - policydb = &state->ss->policydb; - sidtab = state->ss->sidtab; + policydb = &state->ss->policy->policydb; + sidtab = &state->ss->policy->sidtab; if (force) entry = sidtab_search_entry_force(sidtab, sid); @@ -1524,8 +1530,8 @@ static int security_context_to_sid_core(struct selinux_state *state, goto out; } read_lock(&state->ss->policy_rwlock); - policydb = &state->ss->policydb; - sidtab = state->ss->sidtab; + policydb = &state->ss->policy->policydb; + sidtab = &state->ss->policy->sidtab; rc = string_to_context_struct(policydb, sidtab, scontext2, &context, def_sid); if (rc == -EINVAL && force) { @@ -1612,8 +1618,8 @@ static int compute_sid_handle_invalid_context( u16 tclass, struct context *newcontext) { - struct policydb *policydb = &state->ss->policydb; - struct sidtab *sidtab = state->ss->sidtab; + struct policydb *policydb = &state->ss->policy->policydb; + struct sidtab *sidtab = &state->ss->policy->sidtab; char *s = NULL, *t = NULL, *n = NULL; u32 slen, tlen, nlen; struct audit_buffer *ab; @@ -1709,16 +1715,16 @@ static int security_compute_sid(struct selinux_state *state, read_lock(&state->ss->policy_rwlock); if (kern) { - tclass = unmap_class(&state->ss->map, orig_tclass); + tclass = unmap_class(&state->ss->policy->map, orig_tclass); sock = security_is_socket_class(orig_tclass); } else { tclass = orig_tclass; - sock = security_is_socket_class(map_class(&state->ss->map, + sock = security_is_socket_class(map_class(&state->ss->policy->map, tclass)); } - policydb = &state->ss->policydb; - sidtab = state->ss->sidtab; + policydb = &state->ss->policy->policydb; + sidtab = &state->ss->policy->sidtab; sentry = sidtab_search_entry(sidtab, ssid); if (!sentry) { @@ -1935,7 +1941,7 @@ static inline int convert_context_handle_invalid_context( struct selinux_state *state, struct context *context) { - struct policydb *policydb = &state->ss->policydb; + struct policydb *policydb = &state->ss->policy->policydb; char *s; u32 len; @@ -2088,10 +2094,14 @@ bad: static void security_load_policycaps(struct selinux_state *state) { - struct policydb *p = &state->ss->policydb; + struct policydb *p; unsigned int i; struct ebitmap_node *node; + read_lock(&state->ss->policy_rwlock); + + p = &state->ss->policy->policydb; + for (i = 0; i < ARRAY_SIZE(state->policycap); i++) state->policycap[i] = ebitmap_get_bit(&p->policycaps, i); @@ -2105,11 +2115,76 @@ static void security_load_policycaps(struct selinux_state *state) pr_info("SELinux: unknown policy capability %u\n", i); } + + read_unlock(&state->ss->policy_rwlock); } static int security_preserve_bools(struct selinux_state *state, struct policydb *newpolicydb); +static void selinux_policy_free(struct selinux_policy *policy) +{ + if (!policy) + return; + + policydb_destroy(&policy->policydb); + sidtab_destroy(&policy->sidtab); + kfree(policy->map.mapping); + kfree(policy); +} + +static void selinux_policy_commit(struct selinux_state *state, + struct selinux_policy *newpolicy) +{ + struct selinux_policy *oldpolicy; + u32 seqno; + + /* + * NOTE: We do not need to take the policy read-lock + * around the code below because other policy-modifying + * operations are already excluded by selinuxfs via + * fsi->mutex. + */ + + /* If switching between different policy types, log MLS status */ + oldpolicy = state->ss->policy; + if (oldpolicy) { + if (oldpolicy->policydb.mls_enabled && !newpolicy->policydb.mls_enabled) + pr_info("SELinux: Disabling MLS support...\n"); + else if (!oldpolicy->policydb.mls_enabled && newpolicy->policydb.mls_enabled) + pr_info("SELinux: Enabling MLS support...\n"); + } + + /* Install the new policy. */ + write_lock_irq(&state->ss->policy_rwlock); + state->ss->policy = newpolicy; + seqno = ++state->ss->latest_granting; + write_unlock_irq(&state->ss->policy_rwlock); + + /* Load the policycaps from the new policy */ + security_load_policycaps(state); + + if (!selinux_initialized(state)) { + /* + * After first policy load, the security server is + * marked as initialized and ready to handle requests and + * any objects created prior to policy load are then labeled. + */ + selinux_mark_initialized(state); + selinux_complete_init(); + } + + /* Free the old policy */ + selinux_policy_free(oldpolicy); + + /* Flush external caches and notify userspace of policy load */ + avc_ss_reset(state->avc, seqno); + selnl_notify_policyload(seqno); + selinux_status_update_policyload(state, seqno); + selinux_netlbl_cache_invalidate(); + selinux_xfrm_notify_policyload(); +} + /** * security_load_policy - Load a security policy configuration. * @data: binary policy data @@ -2122,112 +2197,64 @@ static int security_preserve_bools(struct selinux_state *state, */ int security_load_policy(struct selinux_state *state, void *data, size_t len) { - struct policydb *policydb; - struct sidtab *oldsidtab, *newsidtab; - struct policydb *oldpolicydb, *newpolicydb; - struct selinux_mapping *oldmapping; - struct selinux_map newmap; + struct selinux_policy *newpolicy; struct sidtab_convert_params convert_params; struct convert_context_args args; - u32 seqno; int rc = 0; struct policy_file file = { data, len }, *fp = &file; - policydb = &state->ss->policydb; - - newsidtab = kmalloc(sizeof(*newsidtab), GFP_KERNEL); - if (!newsidtab) + newpolicy = kzalloc(sizeof(*newpolicy), GFP_KERNEL); + if (!newpolicy) return -ENOMEM; - if (!selinux_initialized(state)) { - rc = policydb_read(policydb, fp); - if (rc) { - kfree(newsidtab); - return rc; - } - - policydb->len = len; - rc = selinux_set_mapping(policydb, secclass_map, - &state->ss->map); - if (rc) { - kfree(newsidtab); - policydb_destroy(policydb); - return rc; - } - - rc = policydb_load_isids(policydb, newsidtab); - if (rc) { - kfree(newsidtab); - policydb_destroy(policydb); - return rc; - } - - state->ss->sidtab = newsidtab; - security_load_policycaps(state); - selinux_mark_initialized(state); - seqno = ++state->ss->latest_granting; - selinux_complete_init(); - avc_ss_reset(state->avc, seqno); - selnl_notify_policyload(seqno); - selinux_status_update_policyload(state, seqno); - selinux_netlbl_cache_invalidate(); - selinux_xfrm_notify_policyload(); - return 0; - } + rc = policydb_read(&newpolicy->policydb, fp); + if (rc) + goto err; - oldpolicydb = kcalloc(2, sizeof(*oldpolicydb), GFP_KERNEL); - if (!oldpolicydb) { - kfree(newsidtab); - return -ENOMEM; - } - newpolicydb = oldpolicydb + 1; + newpolicy->policydb.len = len; + rc = selinux_set_mapping(&newpolicy->policydb, secclass_map, + &newpolicy->map); + if (rc) + goto err; - rc = policydb_read(newpolicydb, fp); + rc = policydb_load_isids(&newpolicy->policydb, &newpolicy->sidtab); if (rc) { - kfree(newsidtab); - goto out; + pr_err("SELinux: unable to load the initial SIDs\n"); + goto err; } - newpolicydb->len = len; - /* If switching between different policy types, log MLS status */ - if (policydb->mls_enabled && !newpolicydb->mls_enabled) - pr_info("SELinux: Disabling MLS support...\n"); - else if (!policydb->mls_enabled && newpolicydb->mls_enabled) - pr_info("SELinux: Enabling MLS support...\n"); - rc = policydb_load_isids(newpolicydb, newsidtab); - if (rc) { - pr_err("SELinux: unable to load the initial SIDs\n"); - policydb_destroy(newpolicydb); - kfree(newsidtab); - goto out; + if (!selinux_initialized(state)) { + /* First policy load, so no need to preserve state from old policy */ + selinux_policy_commit(state, newpolicy); + return 0; } - rc = selinux_set_mapping(newpolicydb, secclass_map, &newmap); - if (rc) - goto err; - - rc = security_preserve_bools(state, newpolicydb); + /* Preserve active boolean values from the old policy */ + rc = security_preserve_bools(state, &newpolicy->policydb); if (rc) { pr_err("SELinux: unable to preserve booleans\n"); goto err; } - oldsidtab = state->ss->sidtab; - /* * Convert the internal representations of contexts * in the new SID table. + * + * NOTE: We do not need to take the policy read-lock + * around the code below because other policy-modifying + * operations are already excluded by selinuxfs via + * fsi->mutex. */ args.state = state; - args.oldp = policydb; - args.newp = newpolicydb; + args.oldp = &state->ss->policy->policydb; + args.newp = &newpolicy->policydb; convert_params.func = convert_context; convert_params.args = &args; - convert_params.target = newsidtab; + convert_params.target = &newpolicy->sidtab; - rc = sidtab_convert(oldsidtab, &convert_params); + rc = sidtab_convert(&state->ss->policy->sidtab, &convert_params); if (rc) { pr_err("SELinux: unable to convert the internal" " representation of contexts in the new SID" @@ -2235,53 +2262,19 @@ int security_load_policy(struct selinux_state *state, void *data, size_t len) goto err; } - /* Save the old policydb and SID table to free later. */ - memcpy(oldpolicydb, policydb, sizeof(*policydb)); - - /* Install the new policydb and SID table. */ - write_lock_irq(&state->ss->policy_rwlock); - memcpy(policydb, newpolicydb, sizeof(*policydb)); - state->ss->sidtab = newsidtab; - security_load_policycaps(state); - oldmapping = state->ss->map.mapping; - state->ss->map.mapping = newmap.mapping; - state->ss->map.size = newmap.size; - seqno = ++state->ss->latest_granting; - write_unlock_irq(&state->ss->policy_rwlock); - - /* Free the old policydb and SID table. */ - policydb_destroy(oldpolicydb); - sidtab_destroy(oldsidtab); - kfree(oldsidtab); - kfree(oldmapping); - - avc_ss_reset(state->avc, seqno); - selnl_notify_policyload(seqno); - selinux_status_update_policyload(state, seqno); - selinux_netlbl_cache_invalidate(); - selinux_xfrm_notify_policyload(); - - rc = 0; - goto out; - + selinux_policy_commit(state, newpolicy); + return 0; err: - kfree(newmap.mapping); - sidtab_destroy(newsidtab); - kfree(newsidtab); - policydb_destroy(newpolicydb); - -out: - kfree(oldpolicydb); + selinux_policy_free(newpolicy); return rc; } size_t security_policydb_len(struct selinux_state *state) { - struct policydb *p = &state->ss->policydb; size_t len; read_lock(&state->ss->policy_rwlock); - len = p->len; + len = state->ss->policy->policydb.len; read_unlock(&state->ss->policy_rwlock); return len; @@ -2303,8 +2296,8 @@ int security_port_sid(struct selinux_state *state, read_lock(&state->ss->policy_rwlock); - policydb = &state->ss->policydb; - sidtab = state->ss->sidtab; + policydb = &state->ss->policy->policydb; + sidtab = &state->ss->policy->sidtab; c = policydb->ocontexts[OCON_PORT]; while (c) { @@ -2348,8 +2341,8 @@ int security_ib_pkey_sid(struct selinux_state *state, read_lock(&state->ss->policy_rwlock); - policydb = &state->ss->policydb; - sidtab = state->ss->sidtab; + policydb = &state->ss->policy->policydb; + sidtab = &state->ss->policy->sidtab; c = policydb->ocontexts[OCON_IBPKEY]; while (c) { @@ -2394,8 +2387,8 @@ int security_ib_endport_sid(struct selinux_state *state, read_lock(&state->ss->policy_rwlock); - policydb = &state->ss->policydb; - sidtab = state->ss->sidtab; + policydb = &state->ss->policy->policydb; + sidtab = &state->ss->policy->sidtab; c = policydb->ocontexts[OCON_IBENDPORT]; while (c) { @@ -2439,8 +2432,8 @@ int security_netif_sid(struct selinux_state *state, read_lock(&state->ss->policy_rwlock); - policydb = &state->ss->policydb; - sidtab = state->ss->sidtab; + policydb = &state->ss->policy->policydb; + sidtab = &state->ss->policy->sidtab; c = policydb->ocontexts[OCON_NETIF]; while (c) { @@ -2502,8 +2495,8 @@ int security_node_sid(struct selinux_state *state, read_lock(&state->ss->policy_rwlock); - policydb = &state->ss->policydb; - sidtab = state->ss->sidtab; + policydb = &state->ss->policy->policydb; + sidtab = &state->ss->policy->sidtab; switch (domain) { case AF_INET: { @@ -2602,8 +2595,8 @@ int security_get_user_sids(struct selinux_state *state, read_lock(&state->ss->policy_rwlock); - policydb = &state->ss->policydb; - sidtab = state->ss->sidtab; + policydb = &state->ss->policy->policydb; + sidtab = &state->ss->policy->sidtab; context_init(&usercon); @@ -2704,8 +2697,8 @@ static inline int __security_genfs_sid(struct selinux_state *state, u16 orig_sclass, u32 *sid) { - struct policydb *policydb = &state->ss->policydb; - struct sidtab *sidtab = state->ss->sidtab; + struct policydb *policydb = &state->ss->policy->policydb; + struct sidtab *sidtab = &state->ss->policy->sidtab; int len; u16 sclass; struct genfs *genfs; @@ -2715,7 +2708,7 @@ static inline int __security_genfs_sid(struct selinux_state *state, while (path[0] == '/' && path[1] == '/') path++; - sclass = unmap_class(&state->ss->map, orig_sclass); + sclass = unmap_class(&state->ss->policy->map, orig_sclass); *sid = SECINITSID_UNLABELED; for (genfs = policydb->genfs; genfs; genfs = genfs->next) { @@ -2790,8 +2783,8 @@ int security_fs_use(struct selinux_state *state, struct super_block *sb) read_lock(&state->ss->policy_rwlock); - policydb = &state->ss->policydb; - sidtab = state->ss->sidtab; + policydb = &state->ss->policy->policydb; + sidtab = &state->ss->policy->sidtab; c = policydb->ocontexts[OCON_FSUSE]; while (c) { @@ -2841,7 +2834,7 @@ int security_get_bools(struct selinux_state *state, read_lock(&state->ss->policy_rwlock); - policydb = &state->ss->policydb; + policydb = &state->ss->policy->policydb; *names = NULL; *values = NULL; @@ -2896,7 +2889,7 @@ int security_set_bools(struct selinux_state *state, u32 len, int *values) write_lock_irq(&state->ss->policy_rwlock); - policydb = &state->ss->policydb; + policydb = &state->ss->policy->policydb; rc = -EFAULT; lenp = policydb->p_bools.nprim; @@ -2944,7 +2937,7 @@ int security_get_bool_value(struct selinux_state *state, read_lock(&state->ss->policy_rwlock); - policydb = &state->ss->policydb; + policydb = &state->ss->policy->policydb; rc = -EFAULT; len = policydb->p_bools.nprim; @@ -2992,8 +2985,8 @@ out: int security_sid_mls_copy(struct selinux_state *state, u32 sid, u32 mls_sid, u32 *new_sid) { - struct policydb *policydb = &state->ss->policydb; - struct sidtab *sidtab = state->ss->sidtab; + struct policydb *policydb; + struct sidtab *sidtab; struct context *context1; struct context *context2; struct context newcon; @@ -3002,7 +2995,7 @@ int security_sid_mls_copy(struct selinux_state *state, int rc; rc = 0; - if (!selinux_initialized(state) || !policydb->mls_enabled) { + if (!selinux_initialized(state)) { *new_sid = sid; goto out; } @@ -3011,6 +3004,14 @@ int security_sid_mls_copy(struct selinux_state *state, read_lock(&state->ss->policy_rwlock); + policydb = &state->ss->policy->policydb; + sidtab = &state->ss->policy->sidtab; + + if (!policydb->mls_enabled) { + *new_sid = sid; + goto out_unlock; + } + rc = -EINVAL; context1 = sidtab_search(sidtab, sid); if (!context1) { @@ -3088,8 +3089,8 @@ int security_net_peersid_resolve(struct selinux_state *state, u32 xfrm_sid, u32 *peer_sid) { - struct policydb *policydb = &state->ss->policydb; - struct sidtab *sidtab = state->ss->sidtab; + struct policydb *policydb; + struct sidtab *sidtab; int rc; struct context *nlbl_ctx; struct context *xfrm_ctx; @@ -3111,15 +3112,20 @@ int security_net_peersid_resolve(struct selinux_state *state, return 0; } + read_lock(&state->ss->policy_rwlock); + + policydb = &state->ss->policy->policydb; + sidtab = &state->ss->policy->sidtab; + /* * We don't need to check 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 state->initialized was true. */ - if (!policydb->mls_enabled) - return 0; - - read_lock(&state->ss->policy_rwlock); + if (!policydb->mls_enabled) { + rc = 0; + goto out; + } rc = -EINVAL; nlbl_ctx = sidtab_search(sidtab, nlbl_sid); @@ -3166,7 +3172,7 @@ static int get_classes_callback(void *k, void *d, void *args) int security_get_classes(struct selinux_state *state, char ***classes, int *nclasses) { - struct policydb *policydb = &state->ss->policydb; + struct policydb *policydb; int rc; if (!selinux_initialized(state)) { @@ -3177,6 +3183,8 @@ int security_get_classes(struct selinux_state *state, read_lock(&state->ss->policy_rwlock); + policydb = &state->ss->policy->policydb; + rc = -ENOMEM; *nclasses = policydb->p_classes.nprim; *classes = kcalloc(*nclasses, sizeof(**classes), GFP_ATOMIC); @@ -3213,12 +3221,14 @@ static int get_permissions_callback(void *k, void *d, void *args) int security_get_permissions(struct selinux_state *state, char *class, char ***perms, int *nperms) { - struct policydb *policydb = &state->ss->policydb; + struct policydb *policydb; int rc, i; struct class_datum *match; read_lock(&state->ss->policy_rwlock); + policydb = &state->ss->policy->policydb; + rc = -EINVAL; match = symtab_search(&policydb->p_classes, class); if (!match) { @@ -3259,12 +3269,22 @@ err: int security_get_reject_unknown(struct selinux_state *state) { - return state->ss->policydb.reject_unknown; + int value; + + read_lock(&state->ss->policy_rwlock); + value = state->ss->policy->policydb.reject_unknown; + read_unlock(&state->ss->policy_rwlock); + return value; } int security_get_allow_unknown(struct selinux_state *state) { - return state->ss->policydb.allow_unknown; + int value; + + read_lock(&state->ss->policy_rwlock); + value = state->ss->policy->policydb.allow_unknown; + read_unlock(&state->ss->policy_rwlock); + return value; } /** @@ -3280,11 +3300,10 @@ int security_get_allow_unknown(struct selinux_state *state) int security_policycap_supported(struct selinux_state *state, unsigned int req_cap) { - struct policydb *policydb = &state->ss->policydb; int rc; read_lock(&state->ss->policy_rwlock); - rc = ebitmap_get_bit(&policydb->policycaps, req_cap); + rc = ebitmap_get_bit(&state->ss->policy->policydb.policycaps, req_cap); read_unlock(&state->ss->policy_rwlock); return rc; @@ -3308,7 +3327,7 @@ void selinux_audit_rule_free(void *vrule) int selinux_audit_rule_init(u32 field, u32 op, char *rulestr, void **vrule) { struct selinux_state *state = &selinux_state; - struct policydb *policydb = &state->ss->policydb; + struct policydb *policydb; struct selinux_audit_rule *tmprule; struct role_datum *roledatum; struct type_datum *typedatum; @@ -3353,6 +3372,8 @@ int selinux_audit_rule_init(u32 field, u32 op, char *rulestr, void **vrule) read_lock(&state->ss->policy_rwlock); + policydb = &state->ss->policy->policydb; + tmprule->au_seqno = state->ss->latest_granting; switch (field) { @@ -3449,7 +3470,7 @@ int selinux_audit_rule_match(u32 sid, u32 field, u32 op, void *vrule) goto out; } - ctxt = sidtab_search(state->ss->sidtab, sid); + ctxt = sidtab_search(&state->ss->policy->sidtab, sid); if (unlikely(!ctxt)) { WARN_ONCE(1, "selinux_audit_rule_match: unrecognized SID %d\n", sid); @@ -3611,8 +3632,8 @@ int security_netlbl_secattr_to_sid(struct selinux_state *state, struct netlbl_lsm_secattr *secattr, u32 *sid) { - struct policydb *policydb = &state->ss->policydb; - struct sidtab *sidtab = state->ss->sidtab; + struct policydb *policydb; + struct sidtab *sidtab; int rc; struct context *ctx; struct context ctx_new; @@ -3624,6 +3645,9 @@ int security_netlbl_secattr_to_sid(struct selinux_state *state, read_lock(&state->ss->policy_rwlock); + policydb = &state->ss->policy->policydb; + sidtab = &state->ss->policy->sidtab; + if (secattr->flags & NETLBL_SECATTR_CACHE) *sid = *(u32 *)secattr->cache->data; else if (secattr->flags & NETLBL_SECATTR_SECID) @@ -3680,7 +3704,7 @@ out: int security_netlbl_sid_to_secattr(struct selinux_state *state, u32 sid, struct netlbl_lsm_secattr *secattr) { - struct policydb *policydb = &state->ss->policydb; + struct policydb *policydb; int rc; struct context *ctx; @@ -3689,8 +3713,10 @@ int security_netlbl_sid_to_secattr(struct selinux_state *state, read_lock(&state->ss->policy_rwlock); + policydb = &state->ss->policy->policydb; + rc = -ENOENT; - ctx = sidtab_search(state->ss->sidtab, sid); + ctx = sidtab_search(&state->ss->policy->sidtab, sid); if (ctx == NULL) goto out; @@ -3719,7 +3745,6 @@ out: int security_read_policy(struct selinux_state *state, void **data, size_t *len) { - struct policydb *policydb = &state->ss->policydb; int rc; struct policy_file fp; @@ -3736,7 +3761,7 @@ int security_read_policy(struct selinux_state *state, fp.len = *len; read_lock(&state->ss->policy_rwlock); - rc = policydb_write(policydb, &fp); + rc = policydb_write(&state->ss->policy->policydb, &fp); read_unlock(&state->ss->policy_rwlock); if (rc) diff --git a/security/selinux/ss/services.h b/security/selinux/ss/services.h index a06f3d835216..c36933c1c363 100644 --- a/security/selinux/ss/services.h +++ b/security/selinux/ss/services.h @@ -22,12 +22,16 @@ struct selinux_map { u16 size; /* array size of mapping */ }; -struct selinux_ss { - struct sidtab *sidtab; +struct selinux_policy { + struct sidtab sidtab; struct policydb policydb; + struct selinux_map map; +}; + +struct selinux_ss { rwlock_t policy_rwlock; u32 latest_granting; - struct selinux_map map; + struct selinux_policy *policy; } __randomize_layout; void services_compute_xperms_drivers(struct extended_perms *xperms, -- cgit v1.2.3 From 02a52c5c8c3b8cbad0f12009cde9f36dbefb6972 Mon Sep 17 00:00:00 2001 From: Stephen Smalley Date: Fri, 7 Aug 2020 09:29:34 -0400 Subject: selinux: move policy commit after updating selinuxfs With the refactoring of the policy load logic in the security server from the previous change, it is now possible to split out the committing of the new policy from security_load_policy() and perform it only after successful updating of selinuxfs. Change security_load_policy() to return the newly populated policy data structures to the caller, export selinux_policy_commit() for external callers, and introduce selinux_policy_cancel() to provide a way to cancel the policy load in the event of an error during updating of the selinuxfs directory tree. Further, rework the interfaces used by selinuxfs to get information from the policy when creating the new directory tree to take and act upon the new policy data structure rather than the current/active policy. Update selinuxfs to use these updated and new interfaces. While we are here, stop re-creating the policy_capabilities directory on each policy load since it does not depend on the policy, and stop trying to create the booleans and classes directories during the initial creation of selinuxfs since no information is available until first policy load. After this change, a failure while updating the booleans and class directories will cause the entire policy load to be canceled, leaving the original policy intact, and policy load notifications to userspace will only happen after a successful completion of updating those directories. This does not (yet) provide full atomicity with respect to the updating of the directory trees themselves. Signed-off-by: Stephen Smalley Signed-off-by: Paul Moore --- security/selinux/include/conditional.h | 2 +- security/selinux/include/security.h | 16 +++++-- security/selinux/selinuxfs.c | 69 ++++++++++++++------------- security/selinux/ss/services.c | 85 ++++++++++++++++------------------ security/selinux/ss/sidtab.c | 10 ++++ security/selinux/ss/sidtab.h | 2 + 6 files changed, 104 insertions(+), 80 deletions(-) (limited to 'security') diff --git a/security/selinux/include/conditional.h b/security/selinux/include/conditional.h index 539ab357707d..b09343346e3f 100644 --- a/security/selinux/include/conditional.h +++ b/security/selinux/include/conditional.h @@ -13,7 +13,7 @@ #include "security.h" -int security_get_bools(struct selinux_state *state, +int security_get_bools(struct selinux_policy *policy, u32 *len, char ***names, int **values); int security_set_bools(struct selinux_state *state, u32 len, int *values); diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h index 02dd91c12235..c68ed2beadff 100644 --- a/security/selinux/include/security.h +++ b/security/selinux/include/security.h @@ -85,6 +85,7 @@ extern int selinux_enabled_boot; struct selinux_avc; struct selinux_ss; +struct selinux_policy; struct selinux_state { #ifdef CONFIG_SECURITY_SELINUX_DISABLE @@ -210,7 +211,12 @@ static inline bool selinux_policycap_genfs_seclabel_symlinks(void) int security_mls_enabled(struct selinux_state *state); int security_load_policy(struct selinux_state *state, - void *data, size_t len); + void *data, size_t len, + struct selinux_policy **newpolicyp); +void selinux_policy_commit(struct selinux_state *state, + struct selinux_policy *newpolicy); +void selinux_policy_cancel(struct selinux_state *state, + struct selinux_policy *policy); int security_read_policy(struct selinux_state *state, void **data, size_t *len); size_t security_policydb_len(struct selinux_state *state); @@ -344,9 +350,9 @@ int security_net_peersid_resolve(struct selinux_state *state, u32 xfrm_sid, u32 *peer_sid); -int security_get_classes(struct selinux_state *state, +int security_get_classes(struct selinux_policy *policy, char ***classes, int *nclasses); -int security_get_permissions(struct selinux_state *state, +int security_get_permissions(struct selinux_policy *policy, char *class, char ***perms, int *nperms); int security_get_reject_unknown(struct selinux_state *state); int security_get_allow_unknown(struct selinux_state *state); @@ -366,6 +372,10 @@ int security_genfs_sid(struct selinux_state *state, const char *fstype, char *name, u16 sclass, u32 *sid); +int selinux_policy_genfs_sid(struct selinux_policy *policy, + const char *fstype, char *name, u16 sclass, + u32 *sid); + #ifdef CONFIG_NETLABEL int security_netlbl_secattr_to_sid(struct selinux_state *state, struct netlbl_lsm_secattr *secattr, diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c index 4781314c2510..131816878e50 100644 --- a/security/selinux/selinuxfs.c +++ b/security/selinux/selinuxfs.c @@ -346,9 +346,10 @@ static const struct file_operations sel_policyvers_ops = { }; /* declaration for sel_write_load */ -static int sel_make_bools(struct selinux_fs_info *fsi); -static int sel_make_classes(struct selinux_fs_info *fsi); -static int sel_make_policycap(struct selinux_fs_info *fsi); +static int sel_make_bools(struct selinux_fs_info *fsi, + struct selinux_policy *newpolicy); +static int sel_make_classes(struct selinux_fs_info *fsi, + struct selinux_policy *newpolicy); /* declaration for sel_make_class_dirs */ static struct dentry *sel_make_dir(struct dentry *dir, const char *name, @@ -508,28 +509,23 @@ static const struct file_operations sel_policy_ops = { .llseek = generic_file_llseek, }; -static int sel_make_policy_nodes(struct selinux_fs_info *fsi) +static int sel_make_policy_nodes(struct selinux_fs_info *fsi, + struct selinux_policy *newpolicy) { int ret; - ret = sel_make_bools(fsi); + ret = sel_make_bools(fsi, newpolicy); if (ret) { pr_err("SELinux: failed to load policy booleans\n"); return ret; } - ret = sel_make_classes(fsi); + ret = sel_make_classes(fsi, newpolicy); if (ret) { pr_err("SELinux: failed to load policy classes\n"); return ret; } - ret = sel_make_policycap(fsi); - if (ret) { - pr_err("SELinux: failed to load policy capabilities\n"); - return ret; - } - return 0; } @@ -538,6 +534,7 @@ static ssize_t sel_write_load(struct file *file, const char __user *buf, { struct selinux_fs_info *fsi = file_inode(file)->i_sb->s_fs_info; + struct selinux_policy *newpolicy; ssize_t length; void *data = NULL; @@ -563,15 +560,19 @@ static ssize_t sel_write_load(struct file *file, const char __user *buf, if (copy_from_user(data, buf, count) != 0) goto out; - length = security_load_policy(fsi->state, data, count); + length = security_load_policy(fsi->state, data, count, &newpolicy); if (length) { pr_warn_ratelimited("SELinux: failed to load policy\n"); goto out; } - length = sel_make_policy_nodes(fsi); - if (length) + length = sel_make_policy_nodes(fsi, newpolicy); + if (length) { + selinux_policy_cancel(fsi->state, newpolicy); goto out1; + } + + selinux_policy_commit(fsi->state, newpolicy); length = count; @@ -1333,7 +1334,8 @@ static void sel_remove_entries(struct dentry *de) #define BOOL_DIR_NAME "booleans" -static int sel_make_bools(struct selinux_fs_info *fsi) +static int sel_make_bools(struct selinux_fs_info *fsi, + struct selinux_policy *newpolicy) { int ret; ssize_t len; @@ -1362,7 +1364,7 @@ static int sel_make_bools(struct selinux_fs_info *fsi) if (!page) goto out; - ret = security_get_bools(fsi->state, &num, &names, &values); + ret = security_get_bools(newpolicy, &num, &names, &values); if (ret) goto out; @@ -1388,7 +1390,7 @@ static int sel_make_bools(struct selinux_fs_info *fsi) } isec = selinux_inode(inode); - ret = security_genfs_sid(fsi->state, "selinuxfs", page, + ret = selinux_policy_genfs_sid(newpolicy, "selinuxfs", page, SECCLASS_FILE, &sid); if (ret) { pr_warn_ratelimited("SELinux: no sid found, defaulting to security isid for %s\n", @@ -1791,14 +1793,14 @@ static const struct file_operations sel_policycap_ops = { .llseek = generic_file_llseek, }; -static int sel_make_perm_files(char *objclass, int classvalue, - struct dentry *dir) +static int sel_make_perm_files(struct selinux_policy *newpolicy, + char *objclass, int classvalue, + struct dentry *dir) { - struct selinux_fs_info *fsi = dir->d_sb->s_fs_info; int i, rc, nperms; char **perms; - rc = security_get_permissions(fsi->state, objclass, &perms, &nperms); + rc = security_get_permissions(newpolicy, objclass, &perms, &nperms); if (rc) return rc; @@ -1831,8 +1833,9 @@ out: return rc; } -static int sel_make_class_dir_entries(char *classname, int index, - struct dentry *dir) +static int sel_make_class_dir_entries(struct selinux_policy *newpolicy, + char *classname, int index, + struct dentry *dir) { struct super_block *sb = dir->d_sb; struct selinux_fs_info *fsi = sb->s_fs_info; @@ -1858,12 +1861,13 @@ static int sel_make_class_dir_entries(char *classname, int index, if (IS_ERR(dentry)) return PTR_ERR(dentry); - rc = sel_make_perm_files(classname, index, dentry); + rc = sel_make_perm_files(newpolicy, classname, index, dentry); return rc; } -static int sel_make_classes(struct selinux_fs_info *fsi) +static int sel_make_classes(struct selinux_fs_info *fsi, + struct selinux_policy *newpolicy) { int rc, nclasses, i; @@ -1872,7 +1876,7 @@ static int sel_make_classes(struct selinux_fs_info *fsi) /* delete any existing entries */ sel_remove_entries(fsi->class_dir); - rc = security_get_classes(fsi->state, &classes, &nclasses); + rc = security_get_classes(newpolicy, &classes, &nclasses); if (rc) return rc; @@ -1890,7 +1894,7 @@ static int sel_make_classes(struct selinux_fs_info *fsi) } /* i+1 since class values are 1-indexed */ - rc = sel_make_class_dir_entries(classes[i], i + 1, + rc = sel_make_class_dir_entries(newpolicy, classes[i], i + 1, class_name_dir); if (rc) goto out; @@ -1909,8 +1913,6 @@ static int sel_make_policycap(struct selinux_fs_info *fsi) struct dentry *dentry = NULL; struct inode *inode = NULL; - sel_remove_entries(fsi->policycap_dir); - for (iter = 0; iter <= POLICYDB_CAPABILITY_MAX; iter++) { if (iter < ARRAY_SIZE(selinux_policycap_names)) dentry = d_alloc_name(fsi->policycap_dir, @@ -2075,9 +2077,12 @@ static int sel_fill_super(struct super_block *sb, struct fs_context *fc) goto err; } - ret = sel_make_policy_nodes(fsi); - if (ret) + ret = sel_make_policycap(fsi); + if (ret) { + pr_err("SELinux: failed to load policy capabilities\n"); goto err; + } + return 0; err: pr_err("SELinux: %s: failed while creating inodes\n", diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c index 137d9396742a..a3f26b03c123 100644 --- a/security/selinux/ss/services.c +++ b/security/selinux/ss/services.c @@ -2133,8 +2133,16 @@ static void selinux_policy_free(struct selinux_policy *policy) kfree(policy); } -static void selinux_policy_commit(struct selinux_state *state, - struct selinux_policy *newpolicy) +void selinux_policy_cancel(struct selinux_state *state, + struct selinux_policy *policy) +{ + + sidtab_cancel_convert(&state->ss->policy->sidtab); + selinux_policy_free(policy); +} + +void selinux_policy_commit(struct selinux_state *state, + struct selinux_policy *newpolicy) { struct selinux_policy *oldpolicy; u32 seqno; @@ -2195,7 +2203,8 @@ static void selinux_policy_commit(struct selinux_state *state, * This function will flush the access vector cache after * loading the new policy. */ -int security_load_policy(struct selinux_state *state, void *data, size_t len) +int security_load_policy(struct selinux_state *state, void *data, size_t len, + struct selinux_policy **newpolicyp) { struct selinux_policy *newpolicy; struct sidtab_convert_params convert_params; @@ -2226,7 +2235,7 @@ int security_load_policy(struct selinux_state *state, void *data, size_t len) if (!selinux_initialized(state)) { /* First policy load, so no need to preserve state from old policy */ - selinux_policy_commit(state, newpolicy); + *newpolicyp = newpolicy; return 0; } @@ -2262,7 +2271,7 @@ int security_load_policy(struct selinux_state *state, void *data, size_t len) goto err; } - selinux_policy_commit(state, newpolicy); + *newpolicyp = newpolicy; return 0; err: selinux_policy_free(newpolicy); @@ -2688,17 +2697,15 @@ out: * Obtain a SID to use for a file in a filesystem that * 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. */ -static inline int __security_genfs_sid(struct selinux_state *state, +static inline int __security_genfs_sid(struct selinux_policy *policy, const char *fstype, char *path, u16 orig_sclass, u32 *sid) { - struct policydb *policydb = &state->ss->policy->policydb; - struct sidtab *sidtab = &state->ss->policy->sidtab; + struct policydb *policydb = &policy->policydb; + struct sidtab *sidtab = &policy->sidtab; int len; u16 sclass; struct genfs *genfs; @@ -2708,7 +2715,7 @@ static inline int __security_genfs_sid(struct selinux_state *state, while (path[0] == '/' && path[1] == '/') path++; - sclass = unmap_class(&state->ss->policy->map, orig_sclass); + sclass = unmap_class(&policy->map, orig_sclass); *sid = SECINITSID_UNLABELED; for (genfs = policydb->genfs; genfs; genfs = genfs->next) { @@ -2763,11 +2770,22 @@ int security_genfs_sid(struct selinux_state *state, int retval; read_lock(&state->ss->policy_rwlock); - retval = __security_genfs_sid(state, fstype, path, orig_sclass, sid); + retval = __security_genfs_sid(state->ss->policy, + fstype, path, orig_sclass, sid); read_unlock(&state->ss->policy_rwlock); return retval; } +int selinux_policy_genfs_sid(struct selinux_policy *policy, + const char *fstype, + char *path, + u16 orig_sclass, + u32 *sid) +{ + /* no lock required, policy is not yet accessible by other threads */ + return __security_genfs_sid(policy, fstype, path, orig_sclass, sid); +} + /** * security_fs_use - Determine how to handle labeling for a filesystem. * @sb: superblock in question @@ -2803,8 +2821,8 @@ int security_fs_use(struct selinux_state *state, struct super_block *sb) } sbsec->sid = c->sid[0]; } else { - rc = __security_genfs_sid(state, fstype, "/", SECCLASS_DIR, - &sbsec->sid); + rc = __security_genfs_sid(state->ss->policy, fstype, "/", + SECCLASS_DIR, &sbsec->sid); if (rc) { sbsec->behavior = SECURITY_FS_USE_NONE; rc = 0; @@ -2818,23 +2836,14 @@ out: return rc; } -int security_get_bools(struct selinux_state *state, +int security_get_bools(struct selinux_policy *policy, u32 *len, char ***names, int **values) { struct policydb *policydb; u32 i; int rc; - if (!selinux_initialized(state)) { - *len = 0; - *names = NULL; - *values = NULL; - return 0; - } - - read_lock(&state->ss->policy_rwlock); - - policydb = &state->ss->policy->policydb; + policydb = &policy->policydb; *names = NULL; *values = NULL; @@ -2865,7 +2874,6 @@ int security_get_bools(struct selinux_state *state, } rc = 0; out: - read_unlock(&state->ss->policy_rwlock); return rc; err: if (*names) { @@ -2958,7 +2966,9 @@ static int security_preserve_bools(struct selinux_state *state, struct cond_bool_datum *booldatum; u32 i, nbools = 0; - rc = security_get_bools(state, &nbools, &bnames, &bvalues); + read_lock(&state->ss->policy_rwlock); + rc = security_get_bools(state->ss->policy, &nbools, &bnames, &bvalues); + read_unlock(&state->ss->policy_rwlock); if (rc) goto out; for (i = 0; i < nbools; i++) { @@ -3169,21 +3179,13 @@ static int get_classes_callback(void *k, void *d, void *args) return 0; } -int security_get_classes(struct selinux_state *state, +int security_get_classes(struct selinux_policy *policy, char ***classes, int *nclasses) { struct policydb *policydb; int rc; - if (!selinux_initialized(state)) { - *nclasses = 0; - *classes = NULL; - return 0; - } - - read_lock(&state->ss->policy_rwlock); - - policydb = &state->ss->policy->policydb; + policydb = &policy->policydb; rc = -ENOMEM; *nclasses = policydb->p_classes.nprim; @@ -3201,7 +3203,6 @@ int security_get_classes(struct selinux_state *state, } out: - read_unlock(&state->ss->policy_rwlock); return rc; } @@ -3218,16 +3219,14 @@ static int get_permissions_callback(void *k, void *d, void *args) return 0; } -int security_get_permissions(struct selinux_state *state, +int security_get_permissions(struct selinux_policy *policy, char *class, char ***perms, int *nperms) { struct policydb *policydb; int rc, i; struct class_datum *match; - read_lock(&state->ss->policy_rwlock); - - policydb = &state->ss->policy->policydb; + policydb = &policy->policydb; rc = -EINVAL; match = symtab_search(&policydb->p_classes, class); @@ -3256,11 +3255,9 @@ int security_get_permissions(struct selinux_state *state, goto err; out: - read_unlock(&state->ss->policy_rwlock); return rc; err: - read_unlock(&state->ss->policy_rwlock); for (i = 0; i < *nperms; i++) kfree((*perms)[i]); kfree(*perms); diff --git a/security/selinux/ss/sidtab.c b/security/selinux/ss/sidtab.c index eb6d27b5aeb4..5ee190bd30f5 100644 --- a/security/selinux/ss/sidtab.c +++ b/security/selinux/ss/sidtab.c @@ -464,6 +464,16 @@ int sidtab_convert(struct sidtab *s, struct sidtab_convert_params *params) return 0; } +void sidtab_cancel_convert(struct sidtab *s) +{ + unsigned long flags; + + /* cancelling policy load - disable live convert of sidtab */ + spin_lock_irqsave(&s->lock, flags); + s->convert = NULL; + spin_unlock_irqrestore(&s->lock, flags); +} + static void sidtab_destroy_entry(struct sidtab_entry *entry) { context_destroy(&entry->context); diff --git a/security/selinux/ss/sidtab.h b/security/selinux/ss/sidtab.h index f2a84560b8b3..80c744d07ad6 100644 --- a/security/selinux/ss/sidtab.h +++ b/security/selinux/ss/sidtab.h @@ -123,6 +123,8 @@ static inline struct context *sidtab_search_force(struct sidtab *s, u32 sid) int sidtab_convert(struct sidtab *s, struct sidtab_convert_params *params); +void sidtab_cancel_convert(struct sidtab *s); + int sidtab_context_to_sid(struct sidtab *s, struct context *context, u32 *sid); void sidtab_destroy(struct sidtab *s); -- cgit v1.2.3 From c7c556f1e81bb9e09656ed6650d0c44c84b7c016 Mon Sep 17 00:00:00 2001 From: Stephen Smalley Date: Tue, 11 Aug 2020 15:01:56 -0400 Subject: selinux: refactor changing booleans Refactor the logic for changing SELinux policy booleans in a similar manner to the refactoring of policy load, thereby reducing the size of the critical section when the policy write-lock is held and making it easier to convert the policy rwlock to RCU in the future. Instead of directly modifying the policydb in place, modify a copy and then swap it into place through a single pointer update. Only fully copy the portions of the policydb that are affected by boolean changes to avoid the full cost of a deep policydb copy. Introduce another level of indirection for the sidtab since changing booleans does not require updating the sidtab, unlike policy load. While we are here, create a common helper for notifying other kernel components and userspace of a policy change and call it from both security_set_bools() and selinux_policy_commit(). Based on an old (2004) patch by Kaigai Kohei [1] to convert the policy rwlock to RCU that was deferred at the time since it did not significantly improve performance and introduced complexity. Peter Enderborg later submitted a patch series to convert to RCU [2] that would have made changing booleans a much more expensive operation by requiring a full policydb_write();policydb_read(); sequence to deep copy the entire policydb and also had concerns regarding atomic allocations. This change is now simplified by the earlier work to encapsulate policy state in the selinux_policy struct and to refactor policy load. After this change, the last major obstacle to converting the policy rwlock to RCU is likely the sidtab live convert support. [1] https://lore.kernel.org/selinux/6e2f9128-e191-ebb3-0e87-74bfccb0767f@tycho.nsa.gov/ [2] https://lore.kernel.org/selinux/20180530141104.28569-1-peter.enderborg@sony.com/ Signed-off-by: Stephen Smalley Signed-off-by: Paul Moore --- security/selinux/ss/avtab.c | 49 +++++++++++- security/selinux/ss/avtab.h | 1 + security/selinux/ss/conditional.c | 156 ++++++++++++++++++++++++++++++++++++ security/selinux/ss/conditional.h | 2 + security/selinux/ss/hashtab.c | 53 +++++++++++++ security/selinux/ss/hashtab.h | 6 ++ security/selinux/ss/services.c | 163 +++++++++++++++++++++++--------------- security/selinux/ss/services.h | 2 +- 8 files changed, 368 insertions(+), 64 deletions(-) (limited to 'security') diff --git a/security/selinux/ss/avtab.c b/security/selinux/ss/avtab.c index 01b300a4a882..0172d87e2b9a 100644 --- a/security/selinux/ss/avtab.c +++ b/security/selinux/ss/avtab.c @@ -301,7 +301,6 @@ void avtab_destroy(struct avtab *h) void avtab_init(struct avtab *h) { - kvfree(h->htable); h->htable = NULL; h->nel = 0; } @@ -340,6 +339,54 @@ int avtab_alloc(struct avtab *h, u32 nrules) return 0; } +int avtab_duplicate(struct avtab *new, struct avtab *orig) +{ + int i; + struct avtab_node *node, *tmp, *tail; + + memset(new, 0, sizeof(*new)); + + new->htable = kvcalloc(orig->nslot, sizeof(void *), GFP_KERNEL); + if (!new->htable) + return -ENOMEM; + new->nslot = orig->nslot; + new->mask = orig->mask; + + for (i = 0; i < orig->nslot; i++) { + tail = NULL; + for (node = orig->htable[i]; node; node = node->next) { + tmp = kmem_cache_zalloc(avtab_node_cachep, GFP_KERNEL); + if (!tmp) + goto error; + tmp->key = node->key; + if (tmp->key.specified & AVTAB_XPERMS) { + tmp->datum.u.xperms = + kmem_cache_zalloc(avtab_xperms_cachep, + GFP_KERNEL); + if (!tmp->datum.u.xperms) { + kmem_cache_free(avtab_node_cachep, tmp); + goto error; + } + tmp->datum.u.xperms = node->datum.u.xperms; + } else + tmp->datum.u.data = node->datum.u.data; + + if (tail) + tail->next = tmp; + else + new->htable[i] = tmp; + + tail = tmp; + new->nel++; + } + } + + return 0; +error: + avtab_destroy(new); + return -ENOMEM; +} + void avtab_hash_eval(struct avtab *h, char *tag) { int i, chain_len, slots_used, max_chain_len; diff --git a/security/selinux/ss/avtab.h b/security/selinux/ss/avtab.h index 5fdcb6696bcc..4c4445ca9118 100644 --- a/security/selinux/ss/avtab.h +++ b/security/selinux/ss/avtab.h @@ -89,6 +89,7 @@ struct avtab { void avtab_init(struct avtab *h); int avtab_alloc(struct avtab *, u32); +int avtab_duplicate(struct avtab *new, struct avtab *orig); struct avtab_datum *avtab_search(struct avtab *h, struct avtab_key *k); void avtab_destroy(struct avtab *h); void avtab_hash_eval(struct avtab *h, char *tag); diff --git a/security/selinux/ss/conditional.c b/security/selinux/ss/conditional.c index 5a47258c1d77..05c7a10e0aab 100644 --- a/security/selinux/ss/conditional.c +++ b/security/selinux/ss/conditional.c @@ -600,3 +600,159 @@ void cond_compute_av(struct avtab *ctab, struct avtab_key *key, services_compute_xperms_drivers(xperms, node); } } + +static int cond_dup_av_list(struct cond_av_list *new, + struct cond_av_list *orig, + struct avtab *avtab) +{ + struct avtab_node *avnode; + u32 i; + + memset(new, 0, sizeof(*new)); + + new->nodes = kcalloc(orig->len, sizeof(*new->nodes), GFP_KERNEL); + if (!new->nodes) + return -ENOMEM; + + for (i = 0; i < orig->len; i++) { + avnode = avtab_search_node(avtab, &orig->nodes[i]->key); + if (WARN_ON(!avnode)) + return -EINVAL; + new->nodes[i] = avnode; + new->len++; + } + + return 0; +} + +static int duplicate_policydb_cond_list(struct policydb *newp, + struct policydb *origp) +{ + int rc, i, j; + + rc = avtab_duplicate(&newp->te_cond_avtab, &origp->te_cond_avtab); + if (rc) + return rc; + + newp->cond_list_len = 0; + newp->cond_list = kcalloc(origp->cond_list_len, + sizeof(*newp->cond_list), + GFP_KERNEL); + if (!newp->cond_list) + goto error; + + for (i = 0; i < origp->cond_list_len; i++) { + struct cond_node *newn = &newp->cond_list[i]; + struct cond_node *orign = &origp->cond_list[i]; + + newp->cond_list_len++; + + newn->cur_state = orign->cur_state; + newn->expr.nodes = kcalloc(orign->expr.len, + sizeof(*newn->expr.nodes), GFP_KERNEL); + if (!newn->expr.nodes) + goto error; + for (j = 0; j < orign->expr.len; j++) + newn->expr.nodes[j] = orign->expr.nodes[j]; + newn->expr.len = orign->expr.len; + + rc = cond_dup_av_list(&newn->true_list, &orign->true_list, + &newp->te_cond_avtab); + if (rc) + goto error; + + rc = cond_dup_av_list(&newn->false_list, &orign->false_list, + &newp->te_cond_avtab); + if (rc) + goto error; + } + + return 0; + +error: + avtab_destroy(&newp->te_cond_avtab); + cond_list_destroy(newp); + return -ENOMEM; +} + +static int cond_bools_destroy(void *key, void *datum, void *args) +{ + /* key was not copied so no need to free here */ + kfree(datum); + return 0; +} + +static int cond_bools_copy(struct hashtab_node *new, struct hashtab_node *orig, void *args) +{ + struct cond_bool_datum *datum; + + datum = kmalloc(sizeof(struct cond_bool_datum), GFP_KERNEL); + if (!datum) + return -ENOMEM; + + memcpy(datum, orig->datum, sizeof(struct cond_bool_datum)); + + new->key = orig->key; /* No need to copy, never modified */ + new->datum = datum; + return 0; +} + +static int cond_bools_index(void *key, void *datum, void *args) +{ + struct cond_bool_datum *booldatum, **cond_bool_array; + + booldatum = datum; + cond_bool_array = args; + cond_bool_array[booldatum->value - 1] = booldatum; + + return 0; +} + +static int duplicate_policydb_bools(struct policydb *newdb, + struct policydb *orig) +{ + struct cond_bool_datum **cond_bool_array; + int rc; + + cond_bool_array = kmalloc_array(orig->p_bools.nprim, + sizeof(*orig->bool_val_to_struct), + GFP_KERNEL); + if (!cond_bool_array) + return -ENOMEM; + + rc = hashtab_duplicate(&newdb->p_bools.table, &orig->p_bools.table, + cond_bools_copy, cond_bools_destroy, NULL); + if (rc) { + kfree(cond_bool_array); + return -ENOMEM; + } + + hashtab_map(&newdb->p_bools.table, cond_bools_index, cond_bool_array); + newdb->bool_val_to_struct = cond_bool_array; + + newdb->p_bools.nprim = orig->p_bools.nprim; + + return 0; +} + +void cond_policydb_destroy_dup(struct policydb *p) +{ + hashtab_map(&p->p_bools.table, cond_bools_destroy, NULL); + hashtab_destroy(&p->p_bools.table); + cond_policydb_destroy(p); +} + +int cond_policydb_dup(struct policydb *new, struct policydb *orig) +{ + cond_policydb_init(new); + + if (duplicate_policydb_bools(new, orig)) + return -ENOMEM; + + if (duplicate_policydb_cond_list(new, orig)) { + cond_policydb_destroy_dup(new); + return -ENOMEM; + } + + return 0; +} diff --git a/security/selinux/ss/conditional.h b/security/selinux/ss/conditional.h index 79e7e03db859..e47ec6ddeaf6 100644 --- a/security/selinux/ss/conditional.h +++ b/security/selinux/ss/conditional.h @@ -79,5 +79,7 @@ void cond_compute_av(struct avtab *ctab, struct avtab_key *key, void cond_compute_xperms(struct avtab *ctab, struct avtab_key *key, struct extended_perms_decision *xpermd); void evaluate_cond_nodes(struct policydb *p); +void cond_policydb_destroy_dup(struct policydb *p); +int cond_policydb_dup(struct policydb *new, struct policydb *orig); #endif /* _CONDITIONAL_H_ */ diff --git a/security/selinux/ss/hashtab.c b/security/selinux/ss/hashtab.c index d9287bb4bfeb..dab8c25c739b 100644 --- a/security/selinux/ss/hashtab.c +++ b/security/selinux/ss/hashtab.c @@ -122,6 +122,59 @@ void hashtab_stat(struct hashtab *h, struct hashtab_info *info) info->max_chain_len = max_chain_len; } +int hashtab_duplicate(struct hashtab *new, struct hashtab *orig, + int (*copy)(struct hashtab_node *new, + struct hashtab_node *orig, void *args), + int (*destroy)(void *k, void *d, void *args), + void *args) +{ + struct hashtab_node *cur, *tmp, *tail; + int i, rc; + + memset(new, 0, sizeof(*new)); + + new->htable = kcalloc(orig->size, sizeof(*new->htable), GFP_KERNEL); + if (!new->htable) + return -ENOMEM; + + new->size = orig->size; + + for (i = 0; i < orig->size; i++) { + tail = NULL; + for (cur = orig->htable[i]; cur; cur = cur->next) { + tmp = kmem_cache_zalloc(hashtab_node_cachep, + GFP_KERNEL); + if (!tmp) + goto error; + rc = copy(tmp, cur, args); + if (rc) { + kmem_cache_free(hashtab_node_cachep, tmp); + goto error; + } + tmp->next = NULL; + if (!tail) + new->htable[i] = tmp; + else + tail->next = tmp; + tail = tmp; + new->nel++; + } + } + + return 0; + + error: + for (i = 0; i < new->size; i++) { + for (cur = new->htable[i]; cur; cur = tmp) { + tmp = cur->next; + destroy(cur->key, cur->datum, args); + kmem_cache_free(hashtab_node_cachep, cur); + } + } + kmem_cache_free(hashtab_node_cachep, new); + return -ENOMEM; +} + void __init hashtab_cache_init(void) { hashtab_node_cachep = kmem_cache_create("hashtab_node", diff --git a/security/selinux/ss/hashtab.h b/security/selinux/ss/hashtab.h index 3c952f0f01f9..043a773bf0b7 100644 --- a/security/selinux/ss/hashtab.h +++ b/security/selinux/ss/hashtab.h @@ -136,6 +136,12 @@ int hashtab_map(struct hashtab *h, int (*apply)(void *k, void *d, void *args), void *args); +int hashtab_duplicate(struct hashtab *new, struct hashtab *orig, + int (*copy)(struct hashtab_node *new, + struct hashtab_node *orig, void *args), + int (*destroy)(void *k, void *d, void *args), + void *args); + /* Fill info with some hash table statistics */ void hashtab_stat(struct hashtab *h, struct hashtab_info *info); diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c index a3f26b03c123..f6f78c65f53f 100644 --- a/security/selinux/ss/services.c +++ b/security/selinux/ss/services.c @@ -723,7 +723,7 @@ static int security_validtrans_handle_fail(struct selinux_state *state, u16 tclass) { struct policydb *p = &state->ss->policy->policydb; - struct sidtab *sidtab = &state->ss->policy->sidtab; + struct sidtab *sidtab = state->ss->policy->sidtab; char *o = NULL, *n = NULL, *t = NULL; u32 olen, nlen, tlen; @@ -768,7 +768,7 @@ static int security_compute_validatetrans(struct selinux_state *state, read_lock(&state->ss->policy_rwlock); policydb = &state->ss->policy->policydb; - sidtab = &state->ss->policy->sidtab; + sidtab = state->ss->policy->sidtab; if (!user) tclass = unmap_class(&state->ss->policy->map, orig_tclass); @@ -869,7 +869,7 @@ int security_bounded_transition(struct selinux_state *state, read_lock(&state->ss->policy_rwlock); policydb = &state->ss->policy->policydb; - sidtab = &state->ss->policy->sidtab; + sidtab = state->ss->policy->sidtab; rc = -EINVAL; old_entry = sidtab_search_entry(sidtab, old_sid); @@ -1026,7 +1026,7 @@ void security_compute_xperms_decision(struct selinux_state *state, goto allow; policydb = &state->ss->policy->policydb; - sidtab = &state->ss->policy->sidtab; + sidtab = state->ss->policy->sidtab; scontext = sidtab_search(sidtab, ssid); if (!scontext) { @@ -1111,7 +1111,7 @@ void security_compute_av(struct selinux_state *state, goto allow; policydb = &state->ss->policy->policydb; - sidtab = &state->ss->policy->sidtab; + sidtab = state->ss->policy->sidtab; scontext = sidtab_search(sidtab, ssid); if (!scontext) { @@ -1165,7 +1165,7 @@ void security_compute_av_user(struct selinux_state *state, goto allow; policydb = &state->ss->policy->policydb; - sidtab = &state->ss->policy->sidtab; + sidtab = state->ss->policy->sidtab; scontext = sidtab_search(sidtab, ssid); if (!scontext) { @@ -1288,7 +1288,7 @@ int security_sidtab_hash_stats(struct selinux_state *state, char *page) } read_lock(&state->ss->policy_rwlock); - rc = sidtab_hash_stats(&state->ss->policy->sidtab, page); + rc = sidtab_hash_stats(state->ss->policy->sidtab, page); read_unlock(&state->ss->policy_rwlock); return rc; @@ -1337,7 +1337,7 @@ static int security_sid_to_context_core(struct selinux_state *state, } read_lock(&state->ss->policy_rwlock); policydb = &state->ss->policy->policydb; - sidtab = &state->ss->policy->sidtab; + sidtab = state->ss->policy->sidtab; if (force) entry = sidtab_search_entry_force(sidtab, sid); @@ -1531,7 +1531,7 @@ static int security_context_to_sid_core(struct selinux_state *state, } read_lock(&state->ss->policy_rwlock); policydb = &state->ss->policy->policydb; - sidtab = &state->ss->policy->sidtab; + sidtab = state->ss->policy->sidtab; rc = string_to_context_struct(policydb, sidtab, scontext2, &context, def_sid); if (rc == -EINVAL && force) { @@ -1619,7 +1619,7 @@ static int compute_sid_handle_invalid_context( struct context *newcontext) { struct policydb *policydb = &state->ss->policy->policydb; - struct sidtab *sidtab = &state->ss->policy->sidtab; + struct sidtab *sidtab = state->ss->policy->sidtab; char *s = NULL, *t = NULL, *n = NULL; u32 slen, tlen, nlen; struct audit_buffer *ab; @@ -1724,7 +1724,7 @@ static int security_compute_sid(struct selinux_state *state, } policydb = &state->ss->policy->policydb; - sidtab = &state->ss->policy->sidtab; + sidtab = state->ss->policy->sidtab; sentry = sidtab_search_entry(sidtab, ssid); if (!sentry) { @@ -2128,7 +2128,8 @@ static void selinux_policy_free(struct selinux_policy *policy) return; policydb_destroy(&policy->policydb); - sidtab_destroy(&policy->sidtab); + sidtab_destroy(policy->sidtab); + kfree(policy->sidtab); kfree(policy->map.mapping); kfree(policy); } @@ -2136,11 +2137,21 @@ static void selinux_policy_free(struct selinux_policy *policy) void selinux_policy_cancel(struct selinux_state *state, struct selinux_policy *policy) { - - sidtab_cancel_convert(&state->ss->policy->sidtab); + sidtab_cancel_convert(state->ss->policy->sidtab); selinux_policy_free(policy); } +static void selinux_notify_policy_change(struct selinux_state *state, + u32 seqno) +{ + /* Flush external caches and notify userspace of policy load */ + avc_ss_reset(state->avc, seqno); + selnl_notify_policyload(seqno); + selinux_status_update_policyload(state, seqno); + selinux_netlbl_cache_invalidate(); + selinux_xfrm_notify_policyload(); +} + void selinux_policy_commit(struct selinux_state *state, struct selinux_policy *newpolicy) { @@ -2185,12 +2196,8 @@ void selinux_policy_commit(struct selinux_state *state, /* Free the old policy */ selinux_policy_free(oldpolicy); - /* Flush external caches and notify userspace of policy load */ - avc_ss_reset(state->avc, seqno); - selnl_notify_policyload(seqno); - selinux_status_update_policyload(state, seqno); - selinux_netlbl_cache_invalidate(); - selinux_xfrm_notify_policyload(); + /* Notify others of the policy change */ + selinux_notify_policy_change(state, seqno); } /** @@ -2216,6 +2223,10 @@ int security_load_policy(struct selinux_state *state, void *data, size_t len, if (!newpolicy) return -ENOMEM; + newpolicy->sidtab = kzalloc(sizeof(*newpolicy->sidtab), GFP_KERNEL); + if (!newpolicy) + goto err; + rc = policydb_read(&newpolicy->policydb, fp); if (rc) goto err; @@ -2226,7 +2237,7 @@ int security_load_policy(struct selinux_state *state, void *data, size_t len, if (rc) goto err; - rc = policydb_load_isids(&newpolicy->policydb, &newpolicy->sidtab); + rc = policydb_load_isids(&newpolicy->policydb, newpolicy->sidtab); if (rc) { pr_err("SELinux: unable to load the initial SIDs\n"); goto err; @@ -2261,9 +2272,9 @@ int security_load_policy(struct selinux_state *state, void *data, size_t len, convert_params.func = convert_context; convert_params.args = &args; - convert_params.target = &newpolicy->sidtab; + convert_params.target = newpolicy->sidtab; - rc = sidtab_convert(&state->ss->policy->sidtab, &convert_params); + rc = sidtab_convert(state->ss->policy->sidtab, &convert_params); if (rc) { pr_err("SELinux: unable to convert the internal" " representation of contexts in the new SID" @@ -2306,7 +2317,7 @@ int security_port_sid(struct selinux_state *state, read_lock(&state->ss->policy_rwlock); policydb = &state->ss->policy->policydb; - sidtab = &state->ss->policy->sidtab; + sidtab = state->ss->policy->sidtab; c = policydb->ocontexts[OCON_PORT]; while (c) { @@ -2351,7 +2362,7 @@ int security_ib_pkey_sid(struct selinux_state *state, read_lock(&state->ss->policy_rwlock); policydb = &state->ss->policy->policydb; - sidtab = &state->ss->policy->sidtab; + sidtab = state->ss->policy->sidtab; c = policydb->ocontexts[OCON_IBPKEY]; while (c) { @@ -2397,7 +2408,7 @@ int security_ib_endport_sid(struct selinux_state *state, read_lock(&state->ss->policy_rwlock); policydb = &state->ss->policy->policydb; - sidtab = &state->ss->policy->sidtab; + sidtab = state->ss->policy->sidtab; c = policydb->ocontexts[OCON_IBENDPORT]; while (c) { @@ -2442,7 +2453,7 @@ int security_netif_sid(struct selinux_state *state, read_lock(&state->ss->policy_rwlock); policydb = &state->ss->policy->policydb; - sidtab = &state->ss->policy->sidtab; + sidtab = state->ss->policy->sidtab; c = policydb->ocontexts[OCON_NETIF]; while (c) { @@ -2505,7 +2516,7 @@ int security_node_sid(struct selinux_state *state, read_lock(&state->ss->policy_rwlock); policydb = &state->ss->policy->policydb; - sidtab = &state->ss->policy->sidtab; + sidtab = state->ss->policy->sidtab; switch (domain) { case AF_INET: { @@ -2605,7 +2616,7 @@ int security_get_user_sids(struct selinux_state *state, read_lock(&state->ss->policy_rwlock); policydb = &state->ss->policy->policydb; - sidtab = &state->ss->policy->sidtab; + sidtab = state->ss->policy->sidtab; context_init(&usercon); @@ -2705,7 +2716,7 @@ static inline int __security_genfs_sid(struct selinux_policy *policy, u32 *sid) { struct policydb *policydb = &policy->policydb; - struct sidtab *sidtab = &policy->sidtab; + struct sidtab *sidtab = policy->sidtab; int len; u16 sclass; struct genfs *genfs; @@ -2802,7 +2813,7 @@ int security_fs_use(struct selinux_state *state, struct super_block *sb) read_lock(&state->ss->policy_rwlock); policydb = &state->ss->policy->policydb; - sidtab = &state->ss->policy->sidtab; + sidtab = state->ss->policy->sidtab; c = policydb->ocontexts[OCON_FSUSE]; while (c) { @@ -2891,49 +2902,77 @@ err: int security_set_bools(struct selinux_state *state, u32 len, int *values) { - struct policydb *policydb; + struct selinux_policy *newpolicy, *oldpolicy; int rc; - u32 i, lenp, seqno = 0; + u32 i, seqno = 0; - write_lock_irq(&state->ss->policy_rwlock); + /* + * NOTE: We do not need to take the policy read-lock + * around the code below because other policy-modifying + * operations are already excluded by selinuxfs via + * fsi->mutex. + */ - policydb = &state->ss->policy->policydb; + /* Consistency check on number of booleans, should never fail */ + if (WARN_ON(len != state->ss->policy->policydb.p_bools.nprim)) + return -EINVAL; - rc = -EFAULT; - lenp = policydb->p_bools.nprim; - if (len != lenp) - goto out; + newpolicy = kmemdup(state->ss->policy, sizeof(*newpolicy), + GFP_KERNEL); + if (!newpolicy) + return -ENOMEM; + + oldpolicy = state->ss->policy; + /* + * Deep copy only the parts of the policydb that might be + * modified as a result of changing booleans. + */ + rc = cond_policydb_dup(&newpolicy->policydb, &oldpolicy->policydb); + if (rc) { + kfree(newpolicy); + return -ENOMEM; + } + + /* Update the boolean states in the copy */ for (i = 0; i < len; i++) { - if (!!values[i] != policydb->bool_val_to_struct[i]->state) { + int new_state = !!values[i]; + int old_state = newpolicy->policydb.bool_val_to_struct[i]->state; + + if (new_state != old_state) { audit_log(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), - !!values[i], - policydb->bool_val_to_struct[i]->state, + sym_name(&newpolicy->policydb, SYM_BOOLS, i), + new_state, + old_state, from_kuid(&init_user_ns, audit_get_loginuid(current)), audit_get_sessionid(current)); + newpolicy->policydb.bool_val_to_struct[i]->state = new_state; } - if (values[i]) - policydb->bool_val_to_struct[i]->state = 1; - else - policydb->bool_val_to_struct[i]->state = 0; } - evaluate_cond_nodes(policydb); + /* Re-evaluate the conditional rules in the copy */ + evaluate_cond_nodes(&newpolicy->policydb); + /* Install the new policy */ + write_lock_irq(&state->ss->policy_rwlock); + state->ss->policy = newpolicy; seqno = ++state->ss->latest_granting; - rc = 0; -out: write_unlock_irq(&state->ss->policy_rwlock); - if (!rc) { - avc_ss_reset(state->avc, seqno); - selnl_notify_policyload(seqno); - selinux_status_update_policyload(state, seqno); - selinux_xfrm_notify_policyload(); - } - return rc; + + /* + * Free the conditional portions of the old policydb + * that were copied for the new policy. + */ + cond_policydb_destroy_dup(&oldpolicy->policydb); + + /* Free the old policy structure but not what it references. */ + kfree(oldpolicy); + + /* Notify others of the policy change */ + selinux_notify_policy_change(state, seqno); + return 0; } int security_get_bool_value(struct selinux_state *state, @@ -3015,7 +3054,7 @@ int security_sid_mls_copy(struct selinux_state *state, read_lock(&state->ss->policy_rwlock); policydb = &state->ss->policy->policydb; - sidtab = &state->ss->policy->sidtab; + sidtab = state->ss->policy->sidtab; if (!policydb->mls_enabled) { *new_sid = sid; @@ -3125,7 +3164,7 @@ int security_net_peersid_resolve(struct selinux_state *state, read_lock(&state->ss->policy_rwlock); policydb = &state->ss->policy->policydb; - sidtab = &state->ss->policy->sidtab; + sidtab = state->ss->policy->sidtab; /* * We don't need to check initialized here since the only way both @@ -3467,7 +3506,7 @@ int selinux_audit_rule_match(u32 sid, u32 field, u32 op, void *vrule) goto out; } - ctxt = sidtab_search(&state->ss->policy->sidtab, sid); + ctxt = sidtab_search(state->ss->policy->sidtab, sid); if (unlikely(!ctxt)) { WARN_ONCE(1, "selinux_audit_rule_match: unrecognized SID %d\n", sid); @@ -3643,7 +3682,7 @@ int security_netlbl_secattr_to_sid(struct selinux_state *state, read_lock(&state->ss->policy_rwlock); policydb = &state->ss->policy->policydb; - sidtab = &state->ss->policy->sidtab; + sidtab = state->ss->policy->sidtab; if (secattr->flags & NETLBL_SECATTR_CACHE) *sid = *(u32 *)secattr->cache->data; @@ -3713,7 +3752,7 @@ int security_netlbl_sid_to_secattr(struct selinux_state *state, policydb = &state->ss->policy->policydb; rc = -ENOENT; - ctx = sidtab_search(&state->ss->policy->sidtab, sid); + ctx = sidtab_search(state->ss->policy->sidtab, sid); if (ctx == NULL) goto out; diff --git a/security/selinux/ss/services.h b/security/selinux/ss/services.h index c36933c1c363..06931e34cb24 100644 --- a/security/selinux/ss/services.h +++ b/security/selinux/ss/services.h @@ -23,7 +23,7 @@ struct selinux_map { }; struct selinux_policy { - struct sidtab sidtab; + struct sidtab *sidtab; struct policydb policydb; struct selinux_map map; }; -- cgit v1.2.3 From 69ea651c40f7f08962d440a0486fd0212be74b73 Mon Sep 17 00:00:00 2001 From: Colin Ian King Date: Wed, 19 Aug 2020 11:42:56 +0100 Subject: selinux: fix allocation failure check on newpolicy->sidtab The allocation check of newpolicy->sidtab is null checking if newpolicy is null and not newpolicy->sidtab. Fix this. Addresses-Coverity: ("Logically dead code") Fixes: c7c556f1e81b ("selinux: refactor changing booleans") Signed-off-by: Colin Ian King Acked-by: Stephen Smalley Signed-off-by: Paul Moore --- security/selinux/ss/services.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'security') diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c index f6f78c65f53f..d310910fb639 100644 --- a/security/selinux/ss/services.c +++ b/security/selinux/ss/services.c @@ -2224,7 +2224,7 @@ int security_load_policy(struct selinux_state *state, void *data, size_t len, return -ENOMEM; newpolicy->sidtab = kzalloc(sizeof(*newpolicy->sidtab), GFP_KERNEL); - if (!newpolicy) + if (!newpolicy->sidtab) goto err; rc = policydb_read(&newpolicy->policydb, fp); -- cgit v1.2.3 From 37ea433c66070fcef09c6d118492c36299eb72ba Mon Sep 17 00:00:00 2001 From: Stephen Smalley Date: Wed, 19 Aug 2020 09:45:41 -0400 Subject: selinux: avoid dereferencing the policy prior to initialization Certain SELinux security server functions (e.g. security_port_sid, called during bind) were not explicitly testing to see if SELinux has been initialized (i.e. initial policy loaded) and handling the no-policy-loaded case. In the past this happened to work because the policydb was statically allocated and could always be accessed, but with the recent encapsulation of policy state and conversion to dynamic allocation, we can no longer access the policy state prior to initialization. Add a test of !selinux_initialized(state) to all of the exported functions that were missing them and handle appropriately. Fixes: 461698026ffa ("selinux: encapsulate policy state, refactor policy load") Reported-by: Naresh Kamboju Tested-by: Andy Shevchenko Signed-off-by: Stephen Smalley Signed-off-by: Paul Moore --- security/selinux/ss/services.c | 60 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+) (limited to 'security') diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c index d310910fb639..a48fc1b337ba 100644 --- a/security/selinux/ss/services.c +++ b/security/selinux/ss/services.c @@ -2293,6 +2293,9 @@ size_t security_policydb_len(struct selinux_state *state) { size_t len; + if (!selinux_initialized(state)) + return 0; + read_lock(&state->ss->policy_rwlock); len = state->ss->policy->policydb.len; read_unlock(&state->ss->policy_rwlock); @@ -2314,6 +2317,11 @@ int security_port_sid(struct selinux_state *state, struct ocontext *c; int rc = 0; + if (!selinux_initialized(state)) { + *out_sid = SECINITSID_PORT; + return 0; + } + read_lock(&state->ss->policy_rwlock); policydb = &state->ss->policy->policydb; @@ -2359,6 +2367,11 @@ int security_ib_pkey_sid(struct selinux_state *state, struct ocontext *c; int rc = 0; + if (!selinux_initialized(state)) { + *out_sid = SECINITSID_UNLABELED; + return 0; + } + read_lock(&state->ss->policy_rwlock); policydb = &state->ss->policy->policydb; @@ -2405,6 +2418,11 @@ int security_ib_endport_sid(struct selinux_state *state, struct ocontext *c; int rc = 0; + if (!selinux_initialized(state)) { + *out_sid = SECINITSID_UNLABELED; + return 0; + } + read_lock(&state->ss->policy_rwlock); policydb = &state->ss->policy->policydb; @@ -2450,6 +2468,11 @@ int security_netif_sid(struct selinux_state *state, int rc = 0; struct ocontext *c; + if (!selinux_initialized(state)) { + *if_sid = SECINITSID_NETIF; + return 0; + } + read_lock(&state->ss->policy_rwlock); policydb = &state->ss->policy->policydb; @@ -2513,6 +2536,11 @@ int security_node_sid(struct selinux_state *state, int rc; struct ocontext *c; + if (!selinux_initialized(state)) { + *out_sid = SECINITSID_NODE; + return 0; + } + read_lock(&state->ss->policy_rwlock); policydb = &state->ss->policy->policydb; @@ -2780,6 +2808,11 @@ int security_genfs_sid(struct selinux_state *state, { int retval; + if (!selinux_initialized(state)) { + *sid = SECINITSID_UNLABELED; + return 0; + } + read_lock(&state->ss->policy_rwlock); retval = __security_genfs_sid(state->ss->policy, fstype, path, orig_sclass, sid); @@ -2810,6 +2843,12 @@ int security_fs_use(struct selinux_state *state, struct super_block *sb) struct superblock_security_struct *sbsec = sb->s_security; const char *fstype = sb->s_type->name; + if (!selinux_initialized(state)) { + sbsec->behavior = SECURITY_FS_USE_NONE; + sbsec->sid = SECINITSID_UNLABELED; + return 0; + } + read_lock(&state->ss->policy_rwlock); policydb = &state->ss->policy->policydb; @@ -2906,6 +2945,9 @@ int security_set_bools(struct selinux_state *state, u32 len, int *values) int rc; u32 i, seqno = 0; + if (!selinux_initialized(state)) + return -EINVAL; + /* * NOTE: We do not need to take the policy read-lock * around the code below because other policy-modifying @@ -2982,6 +3024,9 @@ int security_get_bool_value(struct selinux_state *state, int rc; u32 len; + if (!selinux_initialized(state)) + return 0; + read_lock(&state->ss->policy_rwlock); policydb = &state->ss->policy->policydb; @@ -3161,6 +3206,9 @@ int security_net_peersid_resolve(struct selinux_state *state, return 0; } + if (!selinux_initialized(state)) + return 0; + read_lock(&state->ss->policy_rwlock); policydb = &state->ss->policy->policydb; @@ -3307,6 +3355,9 @@ int security_get_reject_unknown(struct selinux_state *state) { int value; + if (!selinux_initialized(state)) + return 0; + read_lock(&state->ss->policy_rwlock); value = state->ss->policy->policydb.reject_unknown; read_unlock(&state->ss->policy_rwlock); @@ -3317,6 +3368,9 @@ int security_get_allow_unknown(struct selinux_state *state) { int value; + if (!selinux_initialized(state)) + return 0; + read_lock(&state->ss->policy_rwlock); value = state->ss->policy->policydb.allow_unknown; read_unlock(&state->ss->policy_rwlock); @@ -3338,6 +3392,9 @@ int security_policycap_supported(struct selinux_state *state, { int rc; + if (!selinux_initialized(state)) + return 0; + read_lock(&state->ss->policy_rwlock); rc = ebitmap_get_bit(&state->ss->policy->policydb.policycaps, req_cap); read_unlock(&state->ss->policy_rwlock); @@ -3499,6 +3556,9 @@ int selinux_audit_rule_match(u32 sid, u32 field, u32 op, void *vrule) return -ENOENT; } + if (!selinux_initialized(state)) + return 0; + read_lock(&state->ss->policy_rwlock); if (rule->au_seqno < state->ss->latest_granting) { -- cgit v1.2.3 From 879229311bc8a887f00d827525ea59f23861c1ad Mon Sep 17 00:00:00 2001 From: kernel test robot Date: Thu, 20 Aug 2020 12:20:51 +0200 Subject: selinux: fix memdup.cocci warnings Use kmemdup rather than duplicating its implementation Generated by: scripts/coccinelle/api/memdup.cocci Fixes: c7c556f1e81b ("selinux: refactor changing booleans") CC: Stephen Smalley Signed-off-by: kernel test robot Signed-off-by: Julia Lawall Acked-by: Stephen Smalley Signed-off-by: Paul Moore --- security/selinux/ss/conditional.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) (limited to 'security') diff --git a/security/selinux/ss/conditional.c b/security/selinux/ss/conditional.c index 05c7a10e0aab..0b32f3ab025e 100644 --- a/security/selinux/ss/conditional.c +++ b/security/selinux/ss/conditional.c @@ -686,12 +686,11 @@ static int cond_bools_copy(struct hashtab_node *new, struct hashtab_node *orig, { struct cond_bool_datum *datum; - datum = kmalloc(sizeof(struct cond_bool_datum), GFP_KERNEL); + datum = kmemdup(orig->datum, sizeof(struct cond_bool_datum), + GFP_KERNEL); if (!datum) return -ENOMEM; - memcpy(datum, orig->datum, sizeof(struct cond_bool_datum)); - new->key = orig->key; /* No need to copy, never modified */ new->datum = datum; return 0; -- cgit v1.2.3 From 9530a3e00459cd6eabf050133205e0e8fecbdfc7 Mon Sep 17 00:00:00 2001 From: Stephen Smalley Date: Thu, 20 Aug 2020 13:00:40 -0400 Subject: selinux: permit removing security.selinux xattr before policy load Currently SELinux denies attempts to remove the security.selinux xattr always, even when permissive or no policy is loaded. This was originally motivated by the view that all files should be labeled, even if that label is unlabeled_t, and we shouldn't permit files that were once labeled to have their labels removed entirely. This however prevents removing SELinux xattrs in the case where one "disables" SELinux by not loading a policy (e.g. a system where runtime disable is removed and selinux=0 was not specified). Allow removing the xattr before SELinux is initialized. We could conceivably permit it even after initialization if permissive, or introduce a separate permission check here. Signed-off-by: Stephen Smalley Signed-off-by: Paul Moore --- security/selinux/hooks.c | 3 +++ 1 file changed, 3 insertions(+) (limited to 'security') diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index ca901025802a..89d3753b7bd5 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -3271,6 +3271,9 @@ static int selinux_inode_removexattr(struct dentry *dentry, const char *name) return dentry_has_perm(current_cred(), dentry, FILE__SETATTR); } + if (!selinux_initialized(&selinux_state)) + return 0; + /* No one is allowed to remove a SELinux security label. You can change the label, but all data must be labeled. */ return -EACCES; -- cgit v1.2.3 From aeecf4a3fb11954cb10b8bc57e1661a6e4e9f3a9 Mon Sep 17 00:00:00 2001 From: Daniel Burgener Date: Wed, 19 Aug 2020 15:59:32 -0400 Subject: selinux: Create function for selinuxfs directory cleanup Separating the cleanup from the creation will simplify two things in future patches in this series. First, the creation can be made generic, to create directories not tied to the selinux_fs_info structure. Second, we will ultimately want to reorder creation and deletion so that the deletions aren't performed until the new directory structures have already been moved into place. Signed-off-by: Daniel Burgener Acked-by: Stephen Smalley Signed-off-by: Paul Moore --- security/selinux/selinuxfs.c | 39 +++++++++++++++++++++++++-------------- 1 file changed, 25 insertions(+), 14 deletions(-) (limited to 'security') diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c index 131816878e50..19670e9bcd72 100644 --- a/security/selinux/selinuxfs.c +++ b/security/selinux/selinuxfs.c @@ -355,6 +355,9 @@ static int sel_make_classes(struct selinux_fs_info *fsi, static struct dentry *sel_make_dir(struct dentry *dir, const char *name, unsigned long *ino); +/* declaration for sel_remove_old_policy_nodes */ +static void sel_remove_entries(struct dentry *de); + static ssize_t sel_read_mls(struct file *filp, char __user *buf, size_t count, loff_t *ppos) { @@ -509,11 +512,33 @@ static const struct file_operations sel_policy_ops = { .llseek = generic_file_llseek, }; +static void sel_remove_old_policy_nodes(struct selinux_fs_info *fsi) +{ + u32 i; + + /* bool_dir cleanup */ + for (i = 0; i < fsi->bool_num; i++) + kfree(fsi->bool_pending_names[i]); + kfree(fsi->bool_pending_names); + kfree(fsi->bool_pending_values); + fsi->bool_num = 0; + fsi->bool_pending_names = NULL; + fsi->bool_pending_values = NULL; + + sel_remove_entries(fsi->bool_dir); + + /* class_dir cleanup */ + sel_remove_entries(fsi->class_dir); + +} + static int sel_make_policy_nodes(struct selinux_fs_info *fsi, struct selinux_policy *newpolicy) { int ret; + sel_remove_old_policy_nodes(fsi); + ret = sel_make_bools(fsi, newpolicy); if (ret) { pr_err("SELinux: failed to load policy booleans\n"); @@ -1348,17 +1373,6 @@ static int sel_make_bools(struct selinux_fs_info *fsi, int *values = NULL; u32 sid; - /* remove any existing files */ - for (i = 0; i < fsi->bool_num; i++) - kfree(fsi->bool_pending_names[i]); - kfree(fsi->bool_pending_names); - kfree(fsi->bool_pending_values); - fsi->bool_num = 0; - fsi->bool_pending_names = NULL; - fsi->bool_pending_values = NULL; - - sel_remove_entries(dir); - ret = -ENOMEM; page = (char *)get_zeroed_page(GFP_KERNEL); if (!page) @@ -1873,9 +1887,6 @@ static int sel_make_classes(struct selinux_fs_info *fsi, int rc, nclasses, i; char **classes; - /* delete any existing entries */ - sel_remove_entries(fsi->class_dir); - rc = security_get_classes(newpolicy, &classes, &nclasses); if (rc) return rc; -- cgit v1.2.3 From 66ec384ad3044d63c9726493a412a2ad5317eae5 Mon Sep 17 00:00:00 2001 From: Daniel Burgener Date: Wed, 19 Aug 2020 15:59:33 -0400 Subject: selinux: Refactor selinuxfs directory populating functions Make sel_make_bools and sel_make_classes take the specific elements of selinux_fs_info that they need rather than the entire struct. This will allow a future patch to pass temporary elements that are not in the selinux_fs_info struct to these functions so that the original elements can be preserved until we are ready to perform the switch over. Signed-off-by: Daniel Burgener Acked-by: Stephen Smalley Signed-off-by: Paul Moore --- security/selinux/selinuxfs.c | 45 ++++++++++++++++++++++++-------------------- 1 file changed, 25 insertions(+), 20 deletions(-) (limited to 'security') diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c index 19670e9bcd72..cac585ce576b 100644 --- a/security/selinux/selinuxfs.c +++ b/security/selinux/selinuxfs.c @@ -346,10 +346,12 @@ static const struct file_operations sel_policyvers_ops = { }; /* declaration for sel_write_load */ -static int sel_make_bools(struct selinux_fs_info *fsi, - struct selinux_policy *newpolicy); -static int sel_make_classes(struct selinux_fs_info *fsi, - struct selinux_policy *newpolicy); +static int sel_make_bools(struct selinux_policy *newpolicy, struct dentry *bool_dir, + unsigned int *bool_num, char ***bool_pending_names, + unsigned int **bool_pending_values); +static int sel_make_classes(struct selinux_policy *newpolicy, + struct dentry *class_dir, + unsigned long *last_class_ino); /* declaration for sel_make_class_dirs */ static struct dentry *sel_make_dir(struct dentry *dir, const char *name, @@ -539,13 +541,15 @@ static int sel_make_policy_nodes(struct selinux_fs_info *fsi, sel_remove_old_policy_nodes(fsi); - ret = sel_make_bools(fsi, newpolicy); + ret = sel_make_bools(newpolicy, fsi->bool_dir, &fsi->bool_num, + &fsi->bool_pending_names, &fsi->bool_pending_values); if (ret) { pr_err("SELinux: failed to load policy booleans\n"); return ret; } - ret = sel_make_classes(fsi, newpolicy); + ret = sel_make_classes(newpolicy, fsi->class_dir, + &fsi->last_class_ino); if (ret) { pr_err("SELinux: failed to load policy classes\n"); return ret; @@ -1359,13 +1363,13 @@ static void sel_remove_entries(struct dentry *de) #define BOOL_DIR_NAME "booleans" -static int sel_make_bools(struct selinux_fs_info *fsi, - struct selinux_policy *newpolicy) +static int sel_make_bools(struct selinux_policy *newpolicy, struct dentry *bool_dir, + unsigned int *bool_num, char ***bool_pending_names, + unsigned int **bool_pending_values) { int ret; ssize_t len; struct dentry *dentry = NULL; - struct dentry *dir = fsi->bool_dir; struct inode *inode = NULL; struct inode_security_struct *isec; char **names = NULL, *page; @@ -1384,12 +1388,12 @@ static int sel_make_bools(struct selinux_fs_info *fsi, for (i = 0; i < num; i++) { ret = -ENOMEM; - dentry = d_alloc_name(dir, names[i]); + dentry = d_alloc_name(bool_dir, names[i]); if (!dentry) goto out; ret = -ENOMEM; - inode = sel_make_inode(dir->d_sb, S_IFREG | S_IRUGO | S_IWUSR); + inode = sel_make_inode(bool_dir->d_sb, S_IFREG | S_IRUGO | S_IWUSR); if (!inode) { dput(dentry); goto out; @@ -1418,9 +1422,9 @@ static int sel_make_bools(struct selinux_fs_info *fsi, inode->i_ino = i|SEL_BOOL_INO_OFFSET; d_add(dentry, inode); } - fsi->bool_num = num; - fsi->bool_pending_names = names; - fsi->bool_pending_values = values; + *bool_num = num; + *bool_pending_names = names; + *bool_pending_values = values; free_page((unsigned long)page); return 0; @@ -1433,7 +1437,7 @@ out: kfree(names); } kfree(values); - sel_remove_entries(dir); + sel_remove_entries(bool_dir); return ret; } @@ -1880,8 +1884,9 @@ static int sel_make_class_dir_entries(struct selinux_policy *newpolicy, return rc; } -static int sel_make_classes(struct selinux_fs_info *fsi, - struct selinux_policy *newpolicy) +static int sel_make_classes(struct selinux_policy *newpolicy, + struct dentry *class_dir, + unsigned long *last_class_ino) { int rc, nclasses, i; @@ -1892,13 +1897,13 @@ static int sel_make_classes(struct selinux_fs_info *fsi, return rc; /* +2 since classes are 1-indexed */ - fsi->last_class_ino = sel_class_to_ino(nclasses + 2); + *last_class_ino = sel_class_to_ino(nclasses + 2); for (i = 0; i < nclasses; i++) { struct dentry *class_name_dir; - class_name_dir = sel_make_dir(fsi->class_dir, classes[i], - &fsi->last_class_ino); + class_name_dir = sel_make_dir(class_dir, classes[i], + last_class_ino); if (IS_ERR(class_name_dir)) { rc = PTR_ERR(class_name_dir); goto out; -- cgit v1.2.3 From 613ba18798ac3cf257ecff65d490e8f1aa323588 Mon Sep 17 00:00:00 2001 From: Daniel Burgener Date: Wed, 19 Aug 2020 15:59:34 -0400 Subject: selinux: Standardize string literal usage for selinuxfs directory names Switch class and policy_capabilities directory names to be referred to with global constants, consistent with booleans directory name. This will allow for easy consistency of naming in future development. Signed-off-by: Daniel Burgener Acked-by: Stephen Smalley Signed-off-by: Paul Moore --- security/selinux/selinuxfs.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) (limited to 'security') diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c index cac585ce576b..2a0e8b5f19d5 100644 --- a/security/selinux/selinuxfs.c +++ b/security/selinux/selinuxfs.c @@ -117,6 +117,10 @@ static void selinux_fs_info_free(struct super_block *sb) #define SEL_POLICYCAP_INO_OFFSET 0x08000000 #define SEL_INO_MASK 0x00ffffff +#define BOOL_DIR_NAME "booleans" +#define CLASS_DIR_NAME "class" +#define POLICYCAP_DIR_NAME "policy_capabilities" + #define TMPBUFLEN 12 static ssize_t sel_read_enforce(struct file *filp, char __user *buf, size_t count, loff_t *ppos) @@ -1361,8 +1365,6 @@ static void sel_remove_entries(struct dentry *de) shrink_dcache_parent(de); } -#define BOOL_DIR_NAME "booleans" - static int sel_make_bools(struct selinux_policy *newpolicy, struct dentry *bool_dir, unsigned int *bool_num, char ***bool_pending_names, unsigned int **bool_pending_values) @@ -2078,14 +2080,14 @@ static int sel_fill_super(struct super_block *sb, struct fs_context *fc) if (ret) goto err; - fsi->class_dir = sel_make_dir(sb->s_root, "class", &fsi->last_ino); + fsi->class_dir = sel_make_dir(sb->s_root, CLASS_DIR_NAME, &fsi->last_ino); if (IS_ERR(fsi->class_dir)) { ret = PTR_ERR(fsi->class_dir); fsi->class_dir = NULL; goto err; } - fsi->policycap_dir = sel_make_dir(sb->s_root, "policy_capabilities", + fsi->policycap_dir = sel_make_dir(sb->s_root, POLICYCAP_DIR_NAME, &fsi->last_ino); if (IS_ERR(fsi->policycap_dir)) { ret = PTR_ERR(fsi->policycap_dir); -- cgit v1.2.3 From 0eea6091539b15572cd278b8d62893c058bdb292 Mon Sep 17 00:00:00 2001 From: Daniel Burgener Date: Wed, 19 Aug 2020 15:59:35 -0400 Subject: selinux: Create new booleans and class dirs out of tree In order to avoid concurrency issues around selinuxfs resource availability during policy load, we first create new directories out of tree for reloaded resources, then swap them in, and finally delete the old versions. This fix focuses on concurrency in each of the two subtrees swapped, and not concurrency between the trees. This means that it is still possible that subsequent reads to eg the booleans directory and the class directory during a policy load could see the old state for one and the new for the other. The problem of ensuring that policy loads are fully atomic from the perspective of userspace is larger than what is dealt with here. This commit focuses on ensuring that the directories contents always match either the new or the old policy state from the perspective of userspace. In the previous implementation, on policy load /sys/fs/selinux is updated by deleting the previous contents of /sys/fs/selinux/{class,booleans} and then recreating them. This means that there is a period of time when the contents of these directories do not exist which can cause race conditions as userspace relies on them for information about the policy. In addition, it means that error recovery in the event of failure is challenging. In order to demonstrate the race condition that this series fixes, you can use the following commands: while true; do cat /sys/fs/selinux/class/service/perms/status >/dev/null; done & while true; do load_policy; done; In the existing code, this will display errors fairly often as the class lookup fails. (In normal operation from systemd, this would result in a permission check which would be allowed or denied based on policy settings around unknown object classes.) After applying this patch series you should expect to no longer see such error messages. Signed-off-by: Daniel Burgener Acked-by: Stephen Smalley Signed-off-by: Paul Moore --- security/selinux/selinuxfs.c | 113 ++++++++++++++++++++++++++++++++++--------- 1 file changed, 90 insertions(+), 23 deletions(-) (limited to 'security') diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c index 2a0e8b5f19d5..d1872adf0c47 100644 --- a/security/selinux/selinuxfs.c +++ b/security/selinux/selinuxfs.c @@ -20,6 +20,7 @@ #include #include #include +#include #include #include #include @@ -361,7 +362,11 @@ static int sel_make_classes(struct selinux_policy *newpolicy, static struct dentry *sel_make_dir(struct dentry *dir, const char *name, unsigned long *ino); -/* declaration for sel_remove_old_policy_nodes */ +/* declaration for sel_make_policy_nodes */ +static struct dentry *sel_make_disconnected_dir(struct super_block *sb, + unsigned long *ino); + +/* declaration for sel_make_policy_nodes */ static void sel_remove_entries(struct dentry *de); static ssize_t sel_read_mls(struct file *filp, char __user *buf, @@ -518,48 +523,94 @@ static const struct file_operations sel_policy_ops = { .llseek = generic_file_llseek, }; -static void sel_remove_old_policy_nodes(struct selinux_fs_info *fsi) +static void sel_remove_old_bool_data(unsigned int bool_num, char **bool_names, + unsigned int *bool_values) { u32 i; /* bool_dir cleanup */ - for (i = 0; i < fsi->bool_num; i++) - kfree(fsi->bool_pending_names[i]); - kfree(fsi->bool_pending_names); - kfree(fsi->bool_pending_values); - fsi->bool_num = 0; - fsi->bool_pending_names = NULL; - fsi->bool_pending_values = NULL; - - sel_remove_entries(fsi->bool_dir); - - /* class_dir cleanup */ - sel_remove_entries(fsi->class_dir); - + for (i = 0; i < bool_num; i++) + kfree(bool_names[i]); + kfree(bool_names); + kfree(bool_values); } static int sel_make_policy_nodes(struct selinux_fs_info *fsi, struct selinux_policy *newpolicy) { - int ret; + int ret = 0; + struct dentry *tmp_parent, *tmp_bool_dir, *tmp_class_dir, *old_dentry; + unsigned int tmp_bool_num, old_bool_num; + char **tmp_bool_names, **old_bool_names; + unsigned int *tmp_bool_values, *old_bool_values; + unsigned long tmp_ino = fsi->last_ino; /* Don't increment last_ino in this function */ - sel_remove_old_policy_nodes(fsi); + tmp_parent = sel_make_disconnected_dir(fsi->sb, &tmp_ino); + if (IS_ERR(tmp_parent)) + return PTR_ERR(tmp_parent); - ret = sel_make_bools(newpolicy, fsi->bool_dir, &fsi->bool_num, - &fsi->bool_pending_names, &fsi->bool_pending_values); + tmp_ino = fsi->bool_dir->d_inode->i_ino - 1; /* sel_make_dir will increment and set */ + tmp_bool_dir = sel_make_dir(tmp_parent, BOOL_DIR_NAME, &tmp_ino); + if (IS_ERR(tmp_bool_dir)) { + ret = PTR_ERR(tmp_bool_dir); + goto out; + } + + tmp_ino = fsi->class_dir->d_inode->i_ino - 1; /* sel_make_dir will increment and set */ + tmp_class_dir = sel_make_dir(tmp_parent, CLASS_DIR_NAME, &tmp_ino); + if (IS_ERR(tmp_class_dir)) { + ret = PTR_ERR(tmp_class_dir); + goto out; + } + + ret = sel_make_bools(newpolicy, tmp_bool_dir, &tmp_bool_num, + &tmp_bool_names, &tmp_bool_values); if (ret) { pr_err("SELinux: failed to load policy booleans\n"); - return ret; + goto out; } - ret = sel_make_classes(newpolicy, fsi->class_dir, + ret = sel_make_classes(newpolicy, tmp_class_dir, &fsi->last_class_ino); if (ret) { pr_err("SELinux: failed to load policy classes\n"); - return ret; + goto out; } - return 0; + /* booleans */ + old_dentry = fsi->bool_dir; + lock_rename(tmp_bool_dir, old_dentry); + d_exchange(tmp_bool_dir, fsi->bool_dir); + + old_bool_num = fsi->bool_num; + old_bool_names = fsi->bool_pending_names; + old_bool_values = fsi->bool_pending_values; + + fsi->bool_num = tmp_bool_num; + fsi->bool_pending_names = tmp_bool_names; + fsi->bool_pending_values = tmp_bool_values; + + sel_remove_old_bool_data(old_bool_num, old_bool_names, old_bool_values); + + fsi->bool_dir = tmp_bool_dir; + unlock_rename(tmp_bool_dir, old_dentry); + + /* classes */ + old_dentry = fsi->class_dir; + lock_rename(tmp_class_dir, old_dentry); + d_exchange(tmp_class_dir, fsi->class_dir); + fsi->class_dir = tmp_class_dir; + unlock_rename(tmp_class_dir, old_dentry); + +out: + /* Since the other temporary dirs are children of tmp_parent + * this will handle all the cleanup in the case of a failure before + * the swapover + */ + sel_remove_entries(tmp_parent); + dput(tmp_parent); /* d_genocide() only handles the children */ + + return ret; } static ssize_t sel_write_load(struct file *file, const char __user *buf, @@ -1982,6 +2033,22 @@ static struct dentry *sel_make_dir(struct dentry *dir, const char *name, return dentry; } +static struct dentry *sel_make_disconnected_dir(struct super_block *sb, + unsigned long *ino) +{ + struct inode *inode = sel_make_inode(sb, S_IFDIR | S_IRUGO | S_IXUGO); + + if (!inode) + return ERR_PTR(-ENOMEM); + + inode->i_op = &simple_dir_inode_operations; + inode->i_fop = &simple_dir_operations; + inode->i_ino = ++(*ino); + /* directory inodes start off with i_nlink == 2 (for "." entry) */ + inc_nlink(inode); + return d_obtain_alias(inode); +} + #define NULL_FILE_NAME "null" static int sel_fill_super(struct super_block *sb, struct fs_context *fc) -- cgit v1.2.3 From dd8166212d9a2eca3181567c953d5687aea4d7dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thi=C3=A9baud=20Weksteen?= Date: Fri, 21 Aug 2020 16:08:21 +0200 Subject: selinux: add tracepoint on audited events MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The audit data currently captures which process and which target is responsible for a denial. There is no data on where exactly in the process that call occurred. Debugging can be made easier by being able to reconstruct the unified kernel and userland stack traces [1]. Add a tracepoint on the SELinux denials which can then be used by userland (i.e. perf). Although this patch could manually be added by each OS developer to trouble shoot a denial, adding it to the kernel streamlines the developers workflow. It is possible to use perf for monitoring the event: # perf record -e avc:selinux_audited -g -a ^C # perf report -g [...] 6.40% 6.40% audited=800000 tclass=4 | __libc_start_main | |--4.60%--__GI___ioctl | entry_SYSCALL_64 | do_syscall_64 | __x64_sys_ioctl | ksys_ioctl | binder_ioctl | binder_set_nice | can_nice | capable | security_capable | cred_has_capability.isra.0 | slow_avc_audit | common_lsm_audit | avc_audit_post_callback | avc_audit_post_callback | It is also possible to use the ftrace interface: # echo 1 > /sys/kernel/debug/tracing/events/avc/selinux_audited/enable # cat /sys/kernel/debug/tracing/trace tracer: nop entries-in-buffer/entries-written: 1/1 #P:8 [...] dmesg-3624 [001] 13072.325358: selinux_denied: audited=800000 tclass=4 The tclass value can be mapped to a class by searching security/selinux/flask.h. The audited value is a bit field of the permissions described in security/selinux/av_permissions.h for the corresponding class. [1] https://source.android.com/devices/tech/debug/native_stack_dump Signed-off-by: Thiébaud Weksteen Suggested-by: Joel Fernandes Reviewed-by: Peter Enderborg Acked-by: Stephen Smalley Signed-off-by: Paul Moore --- security/selinux/avc.c | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'security') diff --git a/security/selinux/avc.c b/security/selinux/avc.c index d18cb32a242a..b0a0af778b70 100644 --- a/security/selinux/avc.c +++ b/security/selinux/avc.c @@ -31,6 +31,9 @@ #include "avc_ss.h" #include "classmap.h" +#define CREATE_TRACE_POINTS +#include + #define AVC_CACHE_SLOTS 512 #define AVC_DEF_CACHE_THRESHOLD 512 #define AVC_CACHE_RECLAIM 16 @@ -706,6 +709,8 @@ static void avc_audit_post_callback(struct audit_buffer *ab, void *a) u32 scontext_len; int rc; + trace_selinux_audited(sad); + rc = security_sid_to_context(sad->state, sad->ssid, &scontext, &scontext_len); if (rc) -- cgit v1.2.3 From 30969bc8e0751a5e5d44896c0632bc688abfa86f Mon Sep 17 00:00:00 2001 From: Peter Enderborg Date: Fri, 21 Aug 2020 16:08:22 +0200 Subject: selinux: add basic filtering for audit trace events MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This patch adds further attributes to the event. These attributes are helpful to understand the context of the message and can be used to filter the events. There are three common items. Source context, target context and tclass. There are also items from the outcome of operation performed. An event is similar to: <...>-1309 [002] .... 6346.691689: selinux_audited: requested=0x4000000 denied=0x4000000 audited=0x4000000 result=-13 scontext=system_u:system_r:cupsd_t:s0-s0:c0.c1023 tcontext=system_u:object_r:bin_t:s0 tclass=file With systems where many denials are occurring, it is useful to apply a filter. The filtering is a set of logic that is inserted with the filter file. Example: echo "tclass==\"file\" " > events/avc/selinux_audited/filter This adds that we only get tclass=file. The trace can also have extra properties. Adding the user stack can be done with echo 1 > options/userstacktrace Now the output will be runcon-1365 [003] .... 6960.955530: selinux_audited: requested=0x4000000 denied=0x4000000 audited=0x4000000 result=-13 scontext=system_u:system_r:cupsd_t:s0-s0:c0.c1023 tcontext=system_u:object_r:bin_t:s0 tclass=file runcon-1365 [003] .... 6960.955560: => <00007f325b4ce45b> => <00005607093efa57> Signed-off-by: Peter Enderborg Reviewed-by: Thiébaud Weksteen Acked-by: Stephen Smalley Signed-off-by: Paul Moore --- security/selinux/avc.c | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) (limited to 'security') diff --git a/security/selinux/avc.c b/security/selinux/avc.c index b0a0af778b70..3c05827608b6 100644 --- a/security/selinux/avc.c +++ b/security/selinux/avc.c @@ -705,35 +705,37 @@ static void avc_audit_post_callback(struct audit_buffer *ab, void *a) { struct common_audit_data *ad = a; struct selinux_audit_data *sad = ad->selinux_audit_data; - char *scontext; + char *scontext = NULL; + char *tcontext = NULL; + const char *tclass = NULL; u32 scontext_len; + u32 tcontext_len; int rc; - trace_selinux_audited(sad); - rc = security_sid_to_context(sad->state, sad->ssid, &scontext, &scontext_len); if (rc) audit_log_format(ab, " ssid=%d", sad->ssid); - else { + else audit_log_format(ab, " scontext=%s", scontext); - kfree(scontext); - } - rc = security_sid_to_context(sad->state, sad->tsid, &scontext, - &scontext_len); + rc = security_sid_to_context(sad->state, sad->tsid, &tcontext, + &tcontext_len); if (rc) audit_log_format(ab, " tsid=%d", sad->tsid); - else { - audit_log_format(ab, " tcontext=%s", scontext); - kfree(scontext); - } + else + audit_log_format(ab, " tcontext=%s", tcontext); - audit_log_format(ab, " tclass=%s", secclass_map[sad->tclass-1].name); + tclass = secclass_map[sad->tclass-1].name; + audit_log_format(ab, " tclass=%s", tclass); if (sad->denied) audit_log_format(ab, " permissive=%u", sad->result ? 0 : 1); + trace_selinux_audited(sad, scontext, tcontext, tclass); + kfree(tcontext); + kfree(scontext); + /* in case of invalid context report also the actual context string */ rc = security_sid_to_context_inval(sad->state, sad->ssid, &scontext, &scontext_len); -- cgit v1.2.3 From c76a2f9ecdcb44cdcdb2de82e90d84283736aeb2 Mon Sep 17 00:00:00 2001 From: Randy Dunlap Date: Fri, 7 Aug 2020 09:51:34 -0700 Subject: selinux: delete repeated words in comments Drop a repeated word in comments. {open, is, then} Signed-off-by: Randy Dunlap Cc: Paul Moore Cc: Stephen Smalley Cc: Eric Paris Cc: selinux@vger.kernel.org Cc: James Morris Cc: "Serge E. Hallyn" Cc: linux-security-module@vger.kernel.org [PM: fix subject line] Signed-off-by: Paul Moore --- security/selinux/hooks.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'security') diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 89d3753b7bd5..e3fe5647a366 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -1978,7 +1978,7 @@ static inline u32 file_to_av(struct file *file) } /* - * Convert a file to an access vector and include the correct open + * Convert a file to an access vector and include the correct * open permission. */ static inline u32 open_file_to_av(struct file *file) @@ -4447,7 +4447,7 @@ static int selinux_skb_peerlbl_sid(struct sk_buff *skb, u16 family, u32 *sid) * * If @skb_sid is valid then the user:role:type information from @sk_sid is * combined with the MLS information from @skb_sid in order to create - * @conn_sid. If @skb_sid is not valid then then @conn_sid is simply a copy + * @conn_sid. If @skb_sid is not valid then @conn_sid is simply a copy * of @sk_sid. Returns zero on success, negative values on failure. * */ @@ -5317,7 +5317,7 @@ static int selinux_sctp_bind_connect(struct sock *sk, int optname, /* As selinux_sctp_bind_connect() is called by the * SCTP protocol layer, the socket is already locked, - * therefore selinux_netlbl_socket_connect_locked() is + * therefore selinux_netlbl_socket_connect_locked() * is called here. The situations handled are: * sctp_connectx(3), sctp_sendmsg(3), sendmsg(2), * whenever a new IP address is added or when a new -- cgit v1.2.3 From 1b8b31a2e6120b7b2bc99137c0ba1ae3e45dbd7d Mon Sep 17 00:00:00 2001 From: Stephen Smalley Date: Wed, 19 Aug 2020 15:45:16 -0400 Subject: selinux: convert policy read-write lock to RCU Convert the policy read-write lock to RCU. This is significantly simplified by the earlier work to encapsulate the policy data structures and refactor the policy load and boolean setting logic. Move the latest_granting sequence number into the selinux_policy structure so that it can be updated atomically with the policy. Since removing the policy rwlock and moving latest_granting reduces the selinux_ss structure to nothing more than a wrapper around the selinux_policy pointer, get rid of the extra layer of indirection. At present this change merely passes a hardcoded 1 to rcu_dereference_check() in the cases where we know we do not need to take rcu_read_lock(), with the preceding comment explaining why. Alternatively we could pass fsi->mutex down from selinuxfs and apply a lockdep check on it instead. Based in part on earlier attempts to convert the policy rwlock to RCU by Kaigai Kohei [1] and by Peter Enderborg [2]. [1] https://lore.kernel.org/selinux/6e2f9128-e191-ebb3-0e87-74bfccb0767f@tycho.nsa.gov/ [2] https://lore.kernel.org/selinux/20180530141104.28569-1-peter.enderborg@sony.com/ Signed-off-by: Stephen Smalley Reviewed-by: Ondrej Mosnacek Signed-off-by: Paul Moore --- security/selinux/hooks.c | 1 - security/selinux/include/security.h | 5 +- security/selinux/ss/services.c | 487 ++++++++++++++++++++---------------- security/selinux/ss/services.h | 5 - 4 files changed, 280 insertions(+), 218 deletions(-) (limited to 'security') diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index e3fe5647a366..6f30ba1a38dc 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -7235,7 +7235,6 @@ static __init int selinux_init(void) memset(&selinux_state, 0, sizeof(selinux_state)); enforcing_set(&selinux_state, selinux_enforcing_boot); selinux_state.checkreqprot = selinux_checkreqprot_boot; - selinux_ss_init(&selinux_state.ss); selinux_avc_init(&selinux_state.avc); mutex_init(&selinux_state.status_lock); diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h index c68ed2beadff..505e51264d51 100644 --- a/security/selinux/include/security.h +++ b/security/selinux/include/security.h @@ -13,6 +13,7 @@ #include #include #include +#include #include #include #include "flask.h" @@ -84,7 +85,6 @@ extern int selinux_enabled_boot; #define POLICYDB_BOUNDS_MAXDEPTH 4 struct selinux_avc; -struct selinux_ss; struct selinux_policy; struct selinux_state { @@ -102,10 +102,9 @@ struct selinux_state { struct mutex status_lock; struct selinux_avc *avc; - struct selinux_ss *ss; + struct selinux_policy __rcu *policy; } __randomize_layout; -void selinux_ss_init(struct selinux_ss **ss); void selinux_avc_init(struct selinux_avc **avc); extern struct selinux_state selinux_state; diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c index a48fc1b337ba..838161462756 100644 --- a/security/selinux/ss/services.c +++ b/security/selinux/ss/services.c @@ -66,14 +66,6 @@ #include "audit.h" #include "policycap_names.h" -static struct selinux_ss selinux_ss; - -void selinux_ss_init(struct selinux_ss **ss) -{ - rwlock_init(&selinux_ss.policy_rwlock); - *ss = &selinux_ss; -} - /* Forward declaration. */ static int context_struct_to_string(struct policydb *policydb, struct context *context, @@ -239,13 +231,15 @@ static void map_decision(struct selinux_map *map, int security_mls_enabled(struct selinux_state *state) { int mls_enabled; + struct selinux_policy *policy; if (!selinux_initialized(state)) return 0; - read_lock(&state->ss->policy_rwlock); - mls_enabled = state->ss->policy->policydb.mls_enabled; - read_unlock(&state->ss->policy_rwlock); + rcu_read_lock(); + policy = rcu_dereference(state->policy); + mls_enabled = policy->policydb.mls_enabled; + rcu_read_unlock(); return mls_enabled; } @@ -717,13 +711,14 @@ static void context_struct_compute_av(struct policydb *policydb, } static int security_validtrans_handle_fail(struct selinux_state *state, - struct sidtab_entry *oentry, - struct sidtab_entry *nentry, - struct sidtab_entry *tentry, - u16 tclass) + struct selinux_policy *policy, + struct sidtab_entry *oentry, + struct sidtab_entry *nentry, + struct sidtab_entry *tentry, + u16 tclass) { - struct policydb *p = &state->ss->policy->policydb; - struct sidtab *sidtab = state->ss->policy->sidtab; + struct policydb *p = &policy->policydb; + struct sidtab *sidtab = policy->sidtab; char *o = NULL, *n = NULL, *t = NULL; u32 olen, nlen, tlen; @@ -751,6 +746,7 @@ static int security_compute_validatetrans(struct selinux_state *state, u32 oldsid, u32 newsid, u32 tasksid, u16 orig_tclass, bool user) { + struct selinux_policy *policy; struct policydb *policydb; struct sidtab *sidtab; struct sidtab_entry *oentry; @@ -765,13 +761,14 @@ static int security_compute_validatetrans(struct selinux_state *state, if (!selinux_initialized(state)) return 0; - read_lock(&state->ss->policy_rwlock); + rcu_read_lock(); - policydb = &state->ss->policy->policydb; - sidtab = state->ss->policy->sidtab; + policy = rcu_dereference(state->policy); + policydb = &policy->policydb; + sidtab = policy->sidtab; if (!user) - tclass = unmap_class(&state->ss->policy->map, orig_tclass); + tclass = unmap_class(&policy->map, orig_tclass); else tclass = orig_tclass; @@ -814,17 +811,18 @@ static int security_compute_validatetrans(struct selinux_state *state, rc = -EPERM; else rc = security_validtrans_handle_fail(state, - oentry, - nentry, - tentry, - tclass); + policy, + oentry, + nentry, + tentry, + tclass); goto out; } constraint = constraint->next; } out: - read_unlock(&state->ss->policy_rwlock); + rcu_read_unlock(); return rc; } @@ -856,6 +854,7 @@ int security_validate_transition(struct selinux_state *state, int security_bounded_transition(struct selinux_state *state, u32 old_sid, u32 new_sid) { + struct selinux_policy *policy; struct policydb *policydb; struct sidtab *sidtab; struct sidtab_entry *old_entry, *new_entry; @@ -866,10 +865,10 @@ int security_bounded_transition(struct selinux_state *state, if (!selinux_initialized(state)) return 0; - read_lock(&state->ss->policy_rwlock); - - policydb = &state->ss->policy->policydb; - sidtab = state->ss->policy->sidtab; + rcu_read_lock(); + policy = rcu_dereference(state->policy); + policydb = &policy->policydb; + sidtab = policy->sidtab; rc = -EINVAL; old_entry = sidtab_search_entry(sidtab, old_sid); @@ -930,17 +929,20 @@ int security_bounded_transition(struct selinux_state *state, kfree(old_name); } out: - read_unlock(&state->ss->policy_rwlock); + rcu_read_unlock(); return rc; } -static void avd_init(struct selinux_state *state, struct av_decision *avd) +static void avd_init(struct selinux_policy *policy, struct av_decision *avd) { avd->allowed = 0; avd->auditallow = 0; avd->auditdeny = 0xffffffff; - avd->seqno = state->ss->latest_granting; + if (policy) + avd->seqno = policy->latest_granting; + else + avd->seqno = 0; avd->flags = 0; } @@ -1005,6 +1007,7 @@ void security_compute_xperms_decision(struct selinux_state *state, u8 driver, struct extended_perms_decision *xpermd) { + struct selinux_policy *policy; struct policydb *policydb; struct sidtab *sidtab; u16 tclass; @@ -1021,12 +1024,13 @@ void security_compute_xperms_decision(struct selinux_state *state, memset(xpermd->auditallow->p, 0, sizeof(xpermd->auditallow->p)); memset(xpermd->dontaudit->p, 0, sizeof(xpermd->dontaudit->p)); - read_lock(&state->ss->policy_rwlock); + rcu_read_lock(); if (!selinux_initialized(state)) goto allow; - policydb = &state->ss->policy->policydb; - sidtab = state->ss->policy->sidtab; + policy = rcu_dereference(state->policy); + policydb = &policy->policydb; + sidtab = policy->sidtab; scontext = sidtab_search(sidtab, ssid); if (!scontext) { @@ -1042,7 +1046,7 @@ void security_compute_xperms_decision(struct selinux_state *state, goto out; } - tclass = unmap_class(&state->ss->policy->map, orig_tclass); + tclass = unmap_class(&policy->map, orig_tclass); if (unlikely(orig_tclass && !tclass)) { if (policydb->allow_unknown) goto allow; @@ -1074,7 +1078,7 @@ void security_compute_xperms_decision(struct selinux_state *state, } } out: - read_unlock(&state->ss->policy_rwlock); + rcu_read_unlock(); return; allow: memset(xpermd->allowed->p, 0xff, sizeof(xpermd->allowed->p)); @@ -1099,19 +1103,21 @@ void security_compute_av(struct selinux_state *state, struct av_decision *avd, struct extended_perms *xperms) { + struct selinux_policy *policy; struct policydb *policydb; struct sidtab *sidtab; u16 tclass; struct context *scontext = NULL, *tcontext = NULL; - read_lock(&state->ss->policy_rwlock); - avd_init(state, avd); + rcu_read_lock(); + policy = rcu_dereference(state->policy); + avd_init(policy, avd); xperms->len = 0; if (!selinux_initialized(state)) goto allow; - policydb = &state->ss->policy->policydb; - sidtab = state->ss->policy->sidtab; + policydb = &policy->policydb; + sidtab = policy->sidtab; scontext = sidtab_search(sidtab, ssid); if (!scontext) { @@ -1131,7 +1137,7 @@ void security_compute_av(struct selinux_state *state, goto out; } - tclass = unmap_class(&state->ss->policy->map, orig_tclass); + tclass = unmap_class(&policy->map, orig_tclass); if (unlikely(orig_tclass && !tclass)) { if (policydb->allow_unknown) goto allow; @@ -1139,10 +1145,10 @@ void security_compute_av(struct selinux_state *state, } context_struct_compute_av(policydb, scontext, tcontext, tclass, avd, xperms); - map_decision(&state->ss->policy->map, orig_tclass, avd, + map_decision(&policy->map, orig_tclass, avd, policydb->allow_unknown); out: - read_unlock(&state->ss->policy_rwlock); + rcu_read_unlock(); return; allow: avd->allowed = 0xffffffff; @@ -1155,17 +1161,19 @@ void security_compute_av_user(struct selinux_state *state, u16 tclass, struct av_decision *avd) { + struct selinux_policy *policy; struct policydb *policydb; struct sidtab *sidtab; struct context *scontext = NULL, *tcontext = NULL; - read_lock(&state->ss->policy_rwlock); - avd_init(state, avd); + rcu_read_lock(); + policy = rcu_dereference(state->policy); + avd_init(policy, avd); if (!selinux_initialized(state)) goto allow; - policydb = &state->ss->policy->policydb; - sidtab = state->ss->policy->sidtab; + policydb = &policy->policydb; + sidtab = policy->sidtab; scontext = sidtab_search(sidtab, ssid); if (!scontext) { @@ -1194,7 +1202,7 @@ void security_compute_av_user(struct selinux_state *state, context_struct_compute_av(policydb, scontext, tcontext, tclass, avd, NULL); out: - read_unlock(&state->ss->policy_rwlock); + rcu_read_unlock(); return; allow: avd->allowed = 0xffffffff; @@ -1279,6 +1287,7 @@ static int sidtab_entry_to_string(struct policydb *p, int security_sidtab_hash_stats(struct selinux_state *state, char *page) { + struct selinux_policy *policy; int rc; if (!selinux_initialized(state)) { @@ -1287,9 +1296,10 @@ int security_sidtab_hash_stats(struct selinux_state *state, char *page) return -EINVAL; } - read_lock(&state->ss->policy_rwlock); - rc = sidtab_hash_stats(state->ss->policy->sidtab, page); - read_unlock(&state->ss->policy_rwlock); + rcu_read_lock(); + policy = rcu_dereference(state->policy); + rc = sidtab_hash_stats(policy->sidtab, page); + rcu_read_unlock(); return rc; } @@ -1306,6 +1316,7 @@ static int security_sid_to_context_core(struct selinux_state *state, u32 *scontext_len, int force, int only_invalid) { + struct selinux_policy *policy; struct policydb *policydb; struct sidtab *sidtab; struct sidtab_entry *entry; @@ -1335,9 +1346,10 @@ static int security_sid_to_context_core(struct selinux_state *state, "load_policy on unknown SID %d\n", __func__, sid); return -EINVAL; } - read_lock(&state->ss->policy_rwlock); - policydb = &state->ss->policy->policydb; - sidtab = state->ss->policy->sidtab; + rcu_read_lock(); + policy = rcu_dereference(state->policy); + policydb = &policy->policydb; + sidtab = policy->sidtab; if (force) entry = sidtab_search_entry_force(sidtab, sid); @@ -1356,7 +1368,7 @@ static int security_sid_to_context_core(struct selinux_state *state, scontext_len); out_unlock: - read_unlock(&state->ss->policy_rwlock); + rcu_read_unlock(); return rc; } @@ -1491,6 +1503,7 @@ static int security_context_to_sid_core(struct selinux_state *state, u32 *sid, u32 def_sid, gfp_t gfp_flags, int force) { + struct selinux_policy *policy; struct policydb *policydb; struct sidtab *sidtab; char *scontext2, *str = NULL; @@ -1529,9 +1542,10 @@ static int security_context_to_sid_core(struct selinux_state *state, if (!str) goto out; } - read_lock(&state->ss->policy_rwlock); - policydb = &state->ss->policy->policydb; - sidtab = state->ss->policy->sidtab; + rcu_read_lock(); + policy = rcu_dereference(state->policy); + policydb = &policy->policydb; + sidtab = policy->sidtab; rc = string_to_context_struct(policydb, sidtab, scontext2, &context, def_sid); if (rc == -EINVAL && force) { @@ -1543,7 +1557,7 @@ static int security_context_to_sid_core(struct selinux_state *state, rc = sidtab_context_to_sid(sidtab, &context, sid); context_destroy(&context); out_unlock: - read_unlock(&state->ss->policy_rwlock); + rcu_read_unlock(); out: kfree(scontext2); kfree(str); @@ -1613,13 +1627,14 @@ int security_context_to_sid_force(struct selinux_state *state, static int compute_sid_handle_invalid_context( struct selinux_state *state, + struct selinux_policy *policy, struct sidtab_entry *sentry, struct sidtab_entry *tentry, u16 tclass, struct context *newcontext) { - struct policydb *policydb = &state->ss->policy->policydb; - struct sidtab *sidtab = state->ss->policy->sidtab; + struct policydb *policydb = &policy->policydb; + struct sidtab *sidtab = policy->sidtab; char *s = NULL, *t = NULL, *n = NULL; u32 slen, tlen, nlen; struct audit_buffer *ab; @@ -1686,6 +1701,7 @@ static int security_compute_sid(struct selinux_state *state, u32 *out_sid, bool kern) { + struct selinux_policy *policy; struct policydb *policydb; struct sidtab *sidtab; struct class_datum *cladatum = NULL; @@ -1712,19 +1728,21 @@ static int security_compute_sid(struct selinux_state *state, context_init(&newcontext); - read_lock(&state->ss->policy_rwlock); + rcu_read_lock(); + + policy = rcu_dereference(state->policy); if (kern) { - tclass = unmap_class(&state->ss->policy->map, orig_tclass); + tclass = unmap_class(&policy->map, orig_tclass); sock = security_is_socket_class(orig_tclass); } else { tclass = orig_tclass; - sock = security_is_socket_class(map_class(&state->ss->policy->map, + sock = security_is_socket_class(map_class(&policy->map, tclass)); } - policydb = &state->ss->policy->policydb; - sidtab = state->ss->policy->sidtab; + policydb = &policy->policydb; + sidtab = policy->sidtab; sentry = sidtab_search_entry(sidtab, ssid); if (!sentry) { @@ -1844,15 +1862,16 @@ static int security_compute_sid(struct selinux_state *state, /* Check the validity of the context. */ if (!policydb_context_isvalid(policydb, &newcontext)) { - rc = compute_sid_handle_invalid_context(state, sentry, tentry, - tclass, &newcontext); + rc = compute_sid_handle_invalid_context(state, policy, sentry, + tentry, tclass, + &newcontext); if (rc) goto out_unlock; } /* Obtain the sid for the context. */ rc = sidtab_context_to_sid(sidtab, &newcontext, out_sid); out_unlock: - read_unlock(&state->ss->policy_rwlock); + rcu_read_unlock(); context_destroy(&newcontext); out: return rc; @@ -1939,9 +1958,9 @@ int security_change_sid(struct selinux_state *state, static inline int convert_context_handle_invalid_context( struct selinux_state *state, + struct policydb *policydb, struct context *context) { - struct policydb *policydb = &state->ss->policy->policydb; char *s; u32 len; @@ -2073,7 +2092,9 @@ static int convert_context(struct context *oldc, struct context *newc, void *p) /* Check the validity of the new context. */ if (!policydb_context_isvalid(args->newp, newc)) { - rc = convert_context_handle_invalid_context(args->state, oldc); + rc = convert_context_handle_invalid_context(args->state, + args->oldp, + oldc); if (rc) goto bad; } @@ -2092,15 +2113,14 @@ bad: return 0; } -static void security_load_policycaps(struct selinux_state *state) +static void security_load_policycaps(struct selinux_state *state, + struct selinux_policy *policy) { struct policydb *p; unsigned int i; struct ebitmap_node *node; - read_lock(&state->ss->policy_rwlock); - - p = &state->ss->policy->policydb; + p = &policy->policydb; for (i = 0; i < ARRAY_SIZE(state->policycap); i++) state->policycap[i] = ebitmap_get_bit(&p->policycaps, i); @@ -2115,12 +2135,10 @@ static void security_load_policycaps(struct selinux_state *state) pr_info("SELinux: unknown policy capability %u\n", i); } - - read_unlock(&state->ss->policy_rwlock); } -static int security_preserve_bools(struct selinux_state *state, - struct policydb *newpolicydb); +static int security_preserve_bools(struct selinux_policy *oldpolicy, + struct selinux_policy *newpolicy); static void selinux_policy_free(struct selinux_policy *policy) { @@ -2134,10 +2152,26 @@ static void selinux_policy_free(struct selinux_policy *policy) kfree(policy); } +static void selinux_policy_cond_free(struct selinux_policy *policy) +{ + cond_policydb_destroy_dup(&policy->policydb); + kfree(policy); +} + void selinux_policy_cancel(struct selinux_state *state, struct selinux_policy *policy) { - sidtab_cancel_convert(state->ss->policy->sidtab); + struct selinux_policy *oldpolicy; + + /* + * NOTE: We do not need to take the rcu read lock + * around the code below because other policy-modifying + * operations are already excluded by selinuxfs via + * fsi->mutex. + */ + oldpolicy = rcu_dereference_check(state->policy, 1); + + sidtab_cancel_convert(oldpolicy->sidtab); selinux_policy_free(policy); } @@ -2159,14 +2193,14 @@ void selinux_policy_commit(struct selinux_state *state, u32 seqno; /* - * NOTE: We do not need to take the policy read-lock + * NOTE: We do not need to take the rcu read lock * around the code below because other policy-modifying * operations are already excluded by selinuxfs via * fsi->mutex. */ + oldpolicy = rcu_dereference_check(state->policy, 1); /* If switching between different policy types, log MLS status */ - oldpolicy = state->ss->policy; if (oldpolicy) { if (oldpolicy->policydb.mls_enabled && !newpolicy->policydb.mls_enabled) pr_info("SELinux: Disabling MLS support...\n"); @@ -2174,14 +2208,18 @@ void selinux_policy_commit(struct selinux_state *state, pr_info("SELinux: Enabling MLS support...\n"); } + /* Set latest granting seqno for new policy. */ + if (oldpolicy) + newpolicy->latest_granting = oldpolicy->latest_granting + 1; + else + newpolicy->latest_granting = 1; + seqno = newpolicy->latest_granting; + /* Install the new policy. */ - write_lock_irq(&state->ss->policy_rwlock); - state->ss->policy = newpolicy; - seqno = ++state->ss->latest_granting; - write_unlock_irq(&state->ss->policy_rwlock); + rcu_assign_pointer(state->policy, newpolicy); /* Load the policycaps from the new policy */ - security_load_policycaps(state); + security_load_policycaps(state, newpolicy); if (!selinux_initialized(state)) { /* @@ -2194,6 +2232,7 @@ void selinux_policy_commit(struct selinux_state *state, } /* Free the old policy */ + synchronize_rcu(); selinux_policy_free(oldpolicy); /* Notify others of the policy change */ @@ -2213,7 +2252,7 @@ void selinux_policy_commit(struct selinux_state *state, int security_load_policy(struct selinux_state *state, void *data, size_t len, struct selinux_policy **newpolicyp) { - struct selinux_policy *newpolicy; + struct selinux_policy *newpolicy, *oldpolicy; struct sidtab_convert_params convert_params; struct convert_context_args args; int rc = 0; @@ -2250,8 +2289,16 @@ int security_load_policy(struct selinux_state *state, void *data, size_t len, return 0; } + /* + * NOTE: We do not need to take the rcu read lock + * around the code below because other policy-modifying + * operations are already excluded by selinuxfs via + * fsi->mutex. + */ + oldpolicy = rcu_dereference_check(state->policy, 1); + /* Preserve active boolean values from the old policy */ - rc = security_preserve_bools(state, &newpolicy->policydb); + rc = security_preserve_bools(oldpolicy, newpolicy); if (rc) { pr_err("SELinux: unable to preserve booleans\n"); goto err; @@ -2260,21 +2307,16 @@ int security_load_policy(struct selinux_state *state, void *data, size_t len, /* * Convert the internal representations of contexts * in the new SID table. - * - * NOTE: We do not need to take the policy read-lock - * around the code below because other policy-modifying - * operations are already excluded by selinuxfs via - * fsi->mutex. */ args.state = state; - args.oldp = &state->ss->policy->policydb; + args.oldp = &oldpolicy->policydb; args.newp = &newpolicy->policydb; convert_params.func = convert_context; convert_params.args = &args; convert_params.target = newpolicy->sidtab; - rc = sidtab_convert(state->ss->policy->sidtab, &convert_params); + rc = sidtab_convert(oldpolicy->sidtab, &convert_params); if (rc) { pr_err("SELinux: unable to convert the internal" " representation of contexts in the new SID" @@ -2291,14 +2333,16 @@ err: size_t security_policydb_len(struct selinux_state *state) { + struct selinux_policy *policy; size_t len; if (!selinux_initialized(state)) return 0; - read_lock(&state->ss->policy_rwlock); - len = state->ss->policy->policydb.len; - read_unlock(&state->ss->policy_rwlock); + rcu_read_lock(); + policy = rcu_dereference(state->policy); + len = policy->policydb.len; + rcu_read_unlock(); return len; } @@ -2312,6 +2356,7 @@ size_t security_policydb_len(struct selinux_state *state) int security_port_sid(struct selinux_state *state, u8 protocol, u16 port, u32 *out_sid) { + struct selinux_policy *policy; struct policydb *policydb; struct sidtab *sidtab; struct ocontext *c; @@ -2322,10 +2367,10 @@ int security_port_sid(struct selinux_state *state, return 0; } - read_lock(&state->ss->policy_rwlock); - - policydb = &state->ss->policy->policydb; - sidtab = state->ss->policy->sidtab; + rcu_read_lock(); + policy = rcu_dereference(state->policy); + policydb = &policy->policydb; + sidtab = policy->sidtab; c = policydb->ocontexts[OCON_PORT]; while (c) { @@ -2349,7 +2394,7 @@ int security_port_sid(struct selinux_state *state, } out: - read_unlock(&state->ss->policy_rwlock); + rcu_read_unlock(); return rc; } @@ -2362,6 +2407,7 @@ out: int security_ib_pkey_sid(struct selinux_state *state, u64 subnet_prefix, u16 pkey_num, u32 *out_sid) { + struct selinux_policy *policy; struct policydb *policydb; struct sidtab *sidtab; struct ocontext *c; @@ -2372,10 +2418,10 @@ int security_ib_pkey_sid(struct selinux_state *state, return 0; } - read_lock(&state->ss->policy_rwlock); - - policydb = &state->ss->policy->policydb; - sidtab = state->ss->policy->sidtab; + rcu_read_lock(); + policy = rcu_dereference(state->policy); + policydb = &policy->policydb; + sidtab = policy->sidtab; c = policydb->ocontexts[OCON_IBPKEY]; while (c) { @@ -2400,7 +2446,7 @@ int security_ib_pkey_sid(struct selinux_state *state, *out_sid = SECINITSID_UNLABELED; out: - read_unlock(&state->ss->policy_rwlock); + rcu_read_unlock(); return rc; } @@ -2413,6 +2459,7 @@ out: int security_ib_endport_sid(struct selinux_state *state, const char *dev_name, u8 port_num, u32 *out_sid) { + struct selinux_policy *policy; struct policydb *policydb; struct sidtab *sidtab; struct ocontext *c; @@ -2423,10 +2470,10 @@ int security_ib_endport_sid(struct selinux_state *state, return 0; } - read_lock(&state->ss->policy_rwlock); - - policydb = &state->ss->policy->policydb; - sidtab = state->ss->policy->sidtab; + rcu_read_lock(); + policy = rcu_dereference(state->policy); + policydb = &policy->policydb; + sidtab = policy->sidtab; c = policydb->ocontexts[OCON_IBENDPORT]; while (c) { @@ -2451,7 +2498,7 @@ int security_ib_endport_sid(struct selinux_state *state, *out_sid = SECINITSID_UNLABELED; out: - read_unlock(&state->ss->policy_rwlock); + rcu_read_unlock(); return rc; } @@ -2463,6 +2510,7 @@ out: int security_netif_sid(struct selinux_state *state, char *name, u32 *if_sid) { + struct selinux_policy *policy; struct policydb *policydb; struct sidtab *sidtab; int rc = 0; @@ -2473,10 +2521,10 @@ int security_netif_sid(struct selinux_state *state, return 0; } - read_lock(&state->ss->policy_rwlock); - - policydb = &state->ss->policy->policydb; - sidtab = state->ss->policy->sidtab; + rcu_read_lock(); + policy = rcu_dereference(state->policy); + policydb = &policy->policydb; + sidtab = policy->sidtab; c = policydb->ocontexts[OCON_NETIF]; while (c) { @@ -2501,7 +2549,7 @@ int security_netif_sid(struct selinux_state *state, *if_sid = SECINITSID_NETIF; out: - read_unlock(&state->ss->policy_rwlock); + rcu_read_unlock(); return rc; } @@ -2531,6 +2579,7 @@ int security_node_sid(struct selinux_state *state, u32 addrlen, u32 *out_sid) { + struct selinux_policy *policy; struct policydb *policydb; struct sidtab *sidtab; int rc; @@ -2541,10 +2590,10 @@ int security_node_sid(struct selinux_state *state, return 0; } - read_lock(&state->ss->policy_rwlock); - - policydb = &state->ss->policy->policydb; - sidtab = state->ss->policy->sidtab; + rcu_read_lock(); + policy = rcu_dereference(state->policy); + policydb = &policy->policydb; + sidtab = policy->sidtab; switch (domain) { case AF_INET: { @@ -2599,7 +2648,7 @@ int security_node_sid(struct selinux_state *state, rc = 0; out: - read_unlock(&state->ss->policy_rwlock); + rcu_read_unlock(); return rc; } @@ -2625,6 +2674,7 @@ int security_get_user_sids(struct selinux_state *state, u32 **sids, u32 *nel) { + struct selinux_policy *policy; struct policydb *policydb; struct sidtab *sidtab; struct context *fromcon, usercon; @@ -2641,10 +2691,10 @@ int security_get_user_sids(struct selinux_state *state, if (!selinux_initialized(state)) goto out; - read_lock(&state->ss->policy_rwlock); - - policydb = &state->ss->policy->policydb; - sidtab = state->ss->policy->sidtab; + rcu_read_lock(); + policy = rcu_dereference(state->policy); + policydb = &policy->policydb; + sidtab = policy->sidtab; context_init(&usercon); @@ -2695,7 +2745,7 @@ int security_get_user_sids(struct selinux_state *state, } rc = 0; out_unlock: - read_unlock(&state->ss->policy_rwlock); + rcu_read_unlock(); if (rc || !mynel) { kfree(mysids); goto out; @@ -2806,6 +2856,7 @@ int security_genfs_sid(struct selinux_state *state, u16 orig_sclass, u32 *sid) { + struct selinux_policy *policy; int retval; if (!selinux_initialized(state)) { @@ -2813,10 +2864,11 @@ int security_genfs_sid(struct selinux_state *state, return 0; } - read_lock(&state->ss->policy_rwlock); - retval = __security_genfs_sid(state->ss->policy, + rcu_read_lock(); + policy = rcu_dereference(state->policy); + retval = __security_genfs_sid(policy, fstype, path, orig_sclass, sid); - read_unlock(&state->ss->policy_rwlock); + rcu_read_unlock(); return retval; } @@ -2836,6 +2888,7 @@ int selinux_policy_genfs_sid(struct selinux_policy *policy, */ int security_fs_use(struct selinux_state *state, struct super_block *sb) { + struct selinux_policy *policy; struct policydb *policydb; struct sidtab *sidtab; int rc = 0; @@ -2849,10 +2902,10 @@ int security_fs_use(struct selinux_state *state, struct super_block *sb) return 0; } - read_lock(&state->ss->policy_rwlock); - - policydb = &state->ss->policy->policydb; - sidtab = state->ss->policy->sidtab; + rcu_read_lock(); + policy = rcu_dereference(state->policy); + policydb = &policy->policydb; + sidtab = policy->sidtab; c = policydb->ocontexts[OCON_FSUSE]; while (c) { @@ -2871,7 +2924,7 @@ int security_fs_use(struct selinux_state *state, struct super_block *sb) } sbsec->sid = c->sid[0]; } else { - rc = __security_genfs_sid(state->ss->policy, fstype, "/", + rc = __security_genfs_sid(policy, fstype, "/", SECCLASS_DIR, &sbsec->sid); if (rc) { sbsec->behavior = SECURITY_FS_USE_NONE; @@ -2882,7 +2935,7 @@ int security_fs_use(struct selinux_state *state, struct super_block *sb) } out: - read_unlock(&state->ss->policy_rwlock); + rcu_read_unlock(); return rc; } @@ -2949,23 +3002,22 @@ int security_set_bools(struct selinux_state *state, u32 len, int *values) return -EINVAL; /* - * NOTE: We do not need to take the policy read-lock + * NOTE: We do not need to take the rcu read lock * around the code below because other policy-modifying * operations are already excluded by selinuxfs via * fsi->mutex. */ + oldpolicy = rcu_dereference_check(state->policy, 1); + /* Consistency check on number of booleans, should never fail */ - if (WARN_ON(len != state->ss->policy->policydb.p_bools.nprim)) + if (WARN_ON(len != oldpolicy->policydb.p_bools.nprim)) return -EINVAL; - newpolicy = kmemdup(state->ss->policy, sizeof(*newpolicy), - GFP_KERNEL); + newpolicy = kmemdup(oldpolicy, sizeof(*newpolicy), GFP_KERNEL); if (!newpolicy) return -ENOMEM; - oldpolicy = state->ss->policy; - /* * Deep copy only the parts of the policydb that might be * modified as a result of changing booleans. @@ -2997,20 +3049,20 @@ int security_set_bools(struct selinux_state *state, u32 len, int *values) /* Re-evaluate the conditional rules in the copy */ evaluate_cond_nodes(&newpolicy->policydb); + /* Set latest granting seqno for new policy */ + newpolicy->latest_granting = oldpolicy->latest_granting + 1; + seqno = newpolicy->latest_granting; + /* Install the new policy */ - write_lock_irq(&state->ss->policy_rwlock); - state->ss->policy = newpolicy; - seqno = ++state->ss->latest_granting; - write_unlock_irq(&state->ss->policy_rwlock); + rcu_assign_pointer(state->policy, newpolicy); /* * Free the conditional portions of the old policydb - * that were copied for the new policy. + * that were copied for the new policy, and the oldpolicy + * structure itself but not what it references. */ - cond_policydb_destroy_dup(&oldpolicy->policydb); - - /* Free the old policy structure but not what it references. */ - kfree(oldpolicy); + synchronize_rcu(); + selinux_policy_cond_free(oldpolicy); /* Notify others of the policy change */ selinux_notify_policy_change(state, seqno); @@ -3020,6 +3072,7 @@ int security_set_bools(struct selinux_state *state, u32 len, int *values) int security_get_bool_value(struct selinux_state *state, u32 index) { + struct selinux_policy *policy; struct policydb *policydb; int rc; u32 len; @@ -3027,9 +3080,9 @@ int security_get_bool_value(struct selinux_state *state, if (!selinux_initialized(state)) return 0; - read_lock(&state->ss->policy_rwlock); - - policydb = &state->ss->policy->policydb; + rcu_read_lock(); + policy = rcu_dereference(state->policy); + policydb = &policy->policydb; rc = -EFAULT; len = policydb->p_bools.nprim; @@ -3038,29 +3091,28 @@ int security_get_bool_value(struct selinux_state *state, rc = policydb->bool_val_to_struct[index]->state; out: - read_unlock(&state->ss->policy_rwlock); + rcu_read_unlock(); return rc; } -static int security_preserve_bools(struct selinux_state *state, - struct policydb *policydb) +static int security_preserve_bools(struct selinux_policy *oldpolicy, + struct selinux_policy *newpolicy) { int rc, *bvalues = NULL; char **bnames = NULL; struct cond_bool_datum *booldatum; u32 i, nbools = 0; - read_lock(&state->ss->policy_rwlock); - rc = security_get_bools(state->ss->policy, &nbools, &bnames, &bvalues); - read_unlock(&state->ss->policy_rwlock); + rc = security_get_bools(oldpolicy, &nbools, &bnames, &bvalues); if (rc) goto out; for (i = 0; i < nbools; i++) { - booldatum = symtab_search(&policydb->p_bools, bnames[i]); + booldatum = symtab_search(&newpolicy->policydb.p_bools, + bnames[i]); if (booldatum) booldatum->state = bvalues[i]; } - evaluate_cond_nodes(policydb); + evaluate_cond_nodes(&newpolicy->policydb); out: if (bnames) { @@ -3079,6 +3131,7 @@ out: int security_sid_mls_copy(struct selinux_state *state, u32 sid, u32 mls_sid, u32 *new_sid) { + struct selinux_policy *policy; struct policydb *policydb; struct sidtab *sidtab; struct context *context1; @@ -3096,10 +3149,10 @@ int security_sid_mls_copy(struct selinux_state *state, context_init(&newcon); - read_lock(&state->ss->policy_rwlock); - - policydb = &state->ss->policy->policydb; - sidtab = state->ss->policy->sidtab; + rcu_read_lock(); + policy = rcu_dereference(state->policy); + policydb = &policy->policydb; + sidtab = policy->sidtab; if (!policydb->mls_enabled) { *new_sid = sid; @@ -3131,7 +3184,8 @@ int security_sid_mls_copy(struct selinux_state *state, /* Check the validity of the new context. */ if (!policydb_context_isvalid(policydb, &newcon)) { - rc = convert_context_handle_invalid_context(state, &newcon); + rc = convert_context_handle_invalid_context(state, policydb, + &newcon); if (rc) { if (!context_struct_to_string(policydb, &newcon, &s, &len)) { @@ -3152,7 +3206,7 @@ int security_sid_mls_copy(struct selinux_state *state, } rc = sidtab_context_to_sid(sidtab, &newcon, new_sid); out_unlock: - read_unlock(&state->ss->policy_rwlock); + rcu_read_unlock(); context_destroy(&newcon); out: return rc; @@ -3183,6 +3237,7 @@ int security_net_peersid_resolve(struct selinux_state *state, u32 xfrm_sid, u32 *peer_sid) { + struct selinux_policy *policy; struct policydb *policydb; struct sidtab *sidtab; int rc; @@ -3209,10 +3264,10 @@ int security_net_peersid_resolve(struct selinux_state *state, if (!selinux_initialized(state)) return 0; - read_lock(&state->ss->policy_rwlock); - - policydb = &state->ss->policy->policydb; - sidtab = state->ss->policy->sidtab; + rcu_read_lock(); + policy = rcu_dereference(state->policy); + policydb = &policy->policydb; + sidtab = policy->sidtab; /* * We don't need to check initialized here since the only way both @@ -3249,7 +3304,7 @@ int security_net_peersid_resolve(struct selinux_state *state, * expressive */ *peer_sid = xfrm_sid; out: - read_unlock(&state->ss->policy_rwlock); + rcu_read_unlock(); return rc; } @@ -3353,27 +3408,31 @@ err: int security_get_reject_unknown(struct selinux_state *state) { + struct selinux_policy *policy; int value; if (!selinux_initialized(state)) return 0; - read_lock(&state->ss->policy_rwlock); - value = state->ss->policy->policydb.reject_unknown; - read_unlock(&state->ss->policy_rwlock); + rcu_read_lock(); + policy = rcu_dereference(state->policy); + value = policy->policydb.reject_unknown; + rcu_read_unlock(); return value; } int security_get_allow_unknown(struct selinux_state *state) { + struct selinux_policy *policy; int value; if (!selinux_initialized(state)) return 0; - read_lock(&state->ss->policy_rwlock); - value = state->ss->policy->policydb.allow_unknown; - read_unlock(&state->ss->policy_rwlock); + rcu_read_lock(); + policy = rcu_dereference(state->policy); + value = policy->policydb.allow_unknown; + rcu_read_unlock(); return value; } @@ -3390,14 +3449,16 @@ int security_get_allow_unknown(struct selinux_state *state) int security_policycap_supported(struct selinux_state *state, unsigned int req_cap) { + struct selinux_policy *policy; int rc; if (!selinux_initialized(state)) return 0; - read_lock(&state->ss->policy_rwlock); - rc = ebitmap_get_bit(&state->ss->policy->policydb.policycaps, req_cap); - read_unlock(&state->ss->policy_rwlock); + rcu_read_lock(); + policy = rcu_dereference(state->policy); + rc = ebitmap_get_bit(&policy->policydb.policycaps, req_cap); + rcu_read_unlock(); return rc; } @@ -3420,6 +3481,7 @@ void selinux_audit_rule_free(void *vrule) int selinux_audit_rule_init(u32 field, u32 op, char *rulestr, void **vrule) { struct selinux_state *state = &selinux_state; + struct selinux_policy *policy; struct policydb *policydb; struct selinux_audit_rule *tmprule; struct role_datum *roledatum; @@ -3463,11 +3525,11 @@ int selinux_audit_rule_init(u32 field, u32 op, char *rulestr, void **vrule) context_init(&tmprule->au_ctxt); - read_lock(&state->ss->policy_rwlock); - - policydb = &state->ss->policy->policydb; + rcu_read_lock(); + policy = rcu_dereference(state->policy); + policydb = &policy->policydb; - tmprule->au_seqno = state->ss->latest_granting; + tmprule->au_seqno = policy->latest_granting; switch (field) { case AUDIT_SUBJ_USER: @@ -3506,7 +3568,7 @@ int selinux_audit_rule_init(u32 field, u32 op, char *rulestr, void **vrule) } rc = 0; out: - read_unlock(&state->ss->policy_rwlock); + rcu_read_unlock(); if (rc) { selinux_audit_rule_free(tmprule); @@ -3546,6 +3608,7 @@ int selinux_audit_rule_known(struct audit_krule *rule) int selinux_audit_rule_match(u32 sid, u32 field, u32 op, void *vrule) { struct selinux_state *state = &selinux_state; + struct selinux_policy *policy; struct context *ctxt; struct mls_level *level; struct selinux_audit_rule *rule = vrule; @@ -3559,14 +3622,16 @@ int selinux_audit_rule_match(u32 sid, u32 field, u32 op, void *vrule) if (!selinux_initialized(state)) return 0; - read_lock(&state->ss->policy_rwlock); + rcu_read_lock(); + + policy = rcu_dereference(state->policy); - if (rule->au_seqno < state->ss->latest_granting) { + if (rule->au_seqno < policy->latest_granting) { match = -ESTALE; goto out; } - ctxt = sidtab_search(state->ss->policy->sidtab, sid); + ctxt = sidtab_search(policy->sidtab, sid); if (unlikely(!ctxt)) { WARN_ONCE(1, "selinux_audit_rule_match: unrecognized SID %d\n", sid); @@ -3650,7 +3715,7 @@ int selinux_audit_rule_match(u32 sid, u32 field, u32 op, void *vrule) } out: - read_unlock(&state->ss->policy_rwlock); + rcu_read_unlock(); return match; } @@ -3728,6 +3793,7 @@ int security_netlbl_secattr_to_sid(struct selinux_state *state, struct netlbl_lsm_secattr *secattr, u32 *sid) { + struct selinux_policy *policy; struct policydb *policydb; struct sidtab *sidtab; int rc; @@ -3739,10 +3805,10 @@ int security_netlbl_secattr_to_sid(struct selinux_state *state, return 0; } - read_lock(&state->ss->policy_rwlock); - - policydb = &state->ss->policy->policydb; - sidtab = state->ss->policy->sidtab; + rcu_read_lock(); + policy = rcu_dereference(state->policy); + policydb = &policy->policydb; + sidtab = policy->sidtab; if (secattr->flags & NETLBL_SECATTR_CACHE) *sid = *(u32 *)secattr->cache->data; @@ -3778,12 +3844,12 @@ int security_netlbl_secattr_to_sid(struct selinux_state *state, } else *sid = SECSID_NULL; - read_unlock(&state->ss->policy_rwlock); + rcu_read_unlock(); return 0; out_free: ebitmap_destroy(&ctx_new.range.level[0].cat); out: - read_unlock(&state->ss->policy_rwlock); + rcu_read_unlock(); return rc; } @@ -3800,6 +3866,7 @@ out: int security_netlbl_sid_to_secattr(struct selinux_state *state, u32 sid, struct netlbl_lsm_secattr *secattr) { + struct selinux_policy *policy; struct policydb *policydb; int rc; struct context *ctx; @@ -3807,12 +3874,12 @@ int security_netlbl_sid_to_secattr(struct selinux_state *state, if (!selinux_initialized(state)) return 0; - read_lock(&state->ss->policy_rwlock); - - policydb = &state->ss->policy->policydb; + rcu_read_lock(); + policy = rcu_dereference(state->policy); + policydb = &policy->policydb; rc = -ENOENT; - ctx = sidtab_search(state->ss->policy->sidtab, sid); + ctx = sidtab_search(policy->sidtab, sid); if (ctx == NULL) goto out; @@ -3827,7 +3894,7 @@ int security_netlbl_sid_to_secattr(struct selinux_state *state, mls_export_netlbl_lvl(policydb, ctx, secattr); rc = mls_export_netlbl_cat(policydb, ctx, secattr); out: - read_unlock(&state->ss->policy_rwlock); + rcu_read_unlock(); return rc; } #endif /* CONFIG_NETLABEL */ @@ -3841,6 +3908,7 @@ out: int security_read_policy(struct selinux_state *state, void **data, size_t *len) { + struct selinux_policy *policy; int rc; struct policy_file fp; @@ -3856,9 +3924,10 @@ int security_read_policy(struct selinux_state *state, fp.data = *data; fp.len = *len; - read_lock(&state->ss->policy_rwlock); - rc = policydb_write(&state->ss->policy->policydb, &fp); - read_unlock(&state->ss->policy_rwlock); + rcu_read_lock(); + policy = rcu_dereference(state->policy); + rc = policydb_write(&policy->policydb, &fp); + rcu_read_unlock(); if (rc) return rc; diff --git a/security/selinux/ss/services.h b/security/selinux/ss/services.h index 06931e34cb24..9555ad074303 100644 --- a/security/selinux/ss/services.h +++ b/security/selinux/ss/services.h @@ -26,12 +26,7 @@ struct selinux_policy { struct sidtab *sidtab; struct policydb policydb; struct selinux_map map; -}; - -struct selinux_ss { - rwlock_t policy_rwlock; u32 latest_granting; - struct selinux_policy *policy; } __randomize_layout; void services_compute_xperms_drivers(struct extended_perms *xperms, -- cgit v1.2.3 From 0256b0aa8019d937a0bdce3584c6b8b47f618202 Mon Sep 17 00:00:00 2001 From: Dan Carpenter Date: Wed, 26 Aug 2020 10:14:19 -0400 Subject: selinux: fix error handling bugs in security_load_policy() There are a few bugs in the error handling for security_load_policy(). 1) If the newpolicy->sidtab allocation fails then it leads to a NULL dereference. Also the error code was not set to -ENOMEM on that path. 2) If policydb_read() failed then we call policydb_destroy() twice which meands we call kvfree(p->sym_val_to_name[i]) twice. 3) If policydb_load_isids() failed then we call sidtab_destroy() twice and that results in a double free in the sidtab_destroy_tree() function because entry.ptr_inner and entry.ptr_leaf are not set to NULL. One thing that makes this code nice to deal with is that none of the functions return partially allocated data. In other words, the policydb_read() either allocates everything successfully or it frees all the data it allocates. It never returns a mix of allocated and not allocated data. I re-wrote this to only free the successfully allocated data which avoids the double frees. I also re-ordered selinux_policy_free() so it's in the reverse order of the allocation function. Fixes: c7c556f1e81b ("selinux: refactor changing booleans") Acked-by: Stephen Smalley Signed-off-by: Dan Carpenter [PM: partially merged by hand due to merge fuzz] Signed-off-by: Paul Moore --- security/selinux/ss/services.c | 34 +++++++++++++++++++++++----------- 1 file changed, 23 insertions(+), 11 deletions(-) (limited to 'security') diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c index 838161462756..e730204f060b 100644 --- a/security/selinux/ss/services.c +++ b/security/selinux/ss/services.c @@ -2145,10 +2145,10 @@ static void selinux_policy_free(struct selinux_policy *policy) if (!policy) return; - policydb_destroy(&policy->policydb); sidtab_destroy(policy->sidtab); - kfree(policy->sidtab); kfree(policy->map.mapping); + policydb_destroy(&policy->policydb); + kfree(policy->sidtab); kfree(policy); } @@ -2263,23 +2263,25 @@ int security_load_policy(struct selinux_state *state, void *data, size_t len, return -ENOMEM; newpolicy->sidtab = kzalloc(sizeof(*newpolicy->sidtab), GFP_KERNEL); - if (!newpolicy->sidtab) - goto err; + if (!newpolicy->sidtab) { + rc = -ENOMEM; + goto err_policy; + } rc = policydb_read(&newpolicy->policydb, fp); if (rc) - goto err; + goto err_sidtab; newpolicy->policydb.len = len; rc = selinux_set_mapping(&newpolicy->policydb, secclass_map, &newpolicy->map); if (rc) - goto err; + goto err_policydb; rc = policydb_load_isids(&newpolicy->policydb, newpolicy->sidtab); if (rc) { pr_err("SELinux: unable to load the initial SIDs\n"); - goto err; + goto err_mapping; } @@ -2301,7 +2303,7 @@ int security_load_policy(struct selinux_state *state, void *data, size_t len, rc = security_preserve_bools(oldpolicy, newpolicy); if (rc) { pr_err("SELinux: unable to preserve booleans\n"); - goto err; + goto err_free_isids; } /* @@ -2321,13 +2323,23 @@ int security_load_policy(struct selinux_state *state, void *data, size_t len, pr_err("SELinux: unable to convert the internal" " representation of contexts in the new SID" " table\n"); - goto err; + goto err_free_isids; } *newpolicyp = newpolicy; return 0; -err: - selinux_policy_free(newpolicy); + +err_free_isids: + sidtab_destroy(newpolicy->sidtab); +err_mapping: + kfree(newpolicy->map.mapping); +err_policydb: + policydb_destroy(&newpolicy->policydb); +err_sidtab: + kfree(newpolicy->sidtab); +err_policy: + kfree(newpolicy); + return rc; } -- cgit v1.2.3 From 9ff9abc4c6be27ff27b6df625501a46711730520 Mon Sep 17 00:00:00 2001 From: Stephen Smalley Date: Wed, 26 Aug 2020 13:28:53 -0400 Subject: selinux: move policy mutex to selinux_state, use in lockdep checks Move the mutex used to synchronize policy changes (reloads and setting of booleans) from selinux_fs_info to selinux_state and use it in lockdep checks for rcu_dereference_protected() calls in the security server functions. This makes the dependency on the mutex explicit in the code rather than relying on comments. Signed-off-by: Stephen Smalley Reviewed-by: Ondrej Mosnacek Signed-off-by: Paul Moore --- security/selinux/hooks.c | 1 + security/selinux/include/security.h | 1 + security/selinux/selinuxfs.c | 26 ++++++++++++-------------- security/selinux/ss/services.c | 37 ++++++++----------------------------- 4 files changed, 22 insertions(+), 43 deletions(-) (limited to 'security') diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 6f30ba1a38dc..6210e98219a5 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -7237,6 +7237,7 @@ static __init int selinux_init(void) selinux_state.checkreqprot = selinux_checkreqprot_boot; selinux_avc_init(&selinux_state.avc); mutex_init(&selinux_state.status_lock); + mutex_init(&selinux_state.policy_mutex); /* Set the security state for the initial task. */ cred_init_security(); diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h index 505e51264d51..bbbf7141ccdb 100644 --- a/security/selinux/include/security.h +++ b/security/selinux/include/security.h @@ -103,6 +103,7 @@ struct selinux_state { struct selinux_avc *avc; struct selinux_policy __rcu *policy; + struct mutex policy_mutex; } __randomize_layout; void selinux_avc_init(struct selinux_avc **avc); diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c index d1872adf0c47..29567acdda21 100644 --- a/security/selinux/selinuxfs.c +++ b/security/selinux/selinuxfs.c @@ -75,7 +75,6 @@ struct selinux_fs_info { unsigned long last_class_ino; bool policy_opened; struct dentry *policycap_dir; - struct mutex mutex; unsigned long last_ino; struct selinux_state *state; struct super_block *sb; @@ -89,7 +88,6 @@ static int selinux_fs_info_create(struct super_block *sb) if (!fsi) return -ENOMEM; - mutex_init(&fsi->mutex); fsi->last_ino = SEL_INO_NEXT - 1; fsi->state = &selinux_state; fsi->sb = sb; @@ -400,7 +398,7 @@ static int sel_open_policy(struct inode *inode, struct file *filp) BUG_ON(filp->private_data); - mutex_lock(&fsi->mutex); + mutex_lock(&fsi->state->policy_mutex); rc = avc_has_perm(&selinux_state, current_sid(), SECINITSID_SECURITY, @@ -431,11 +429,11 @@ static int sel_open_policy(struct inode *inode, struct file *filp) filp->private_data = plm; - mutex_unlock(&fsi->mutex); + mutex_unlock(&fsi->state->policy_mutex); return 0; err: - mutex_unlock(&fsi->mutex); + mutex_unlock(&fsi->state->policy_mutex); if (plm) vfree(plm->data); @@ -622,7 +620,7 @@ static ssize_t sel_write_load(struct file *file, const char __user *buf, ssize_t length; void *data = NULL; - mutex_lock(&fsi->mutex); + mutex_lock(&fsi->state->policy_mutex); length = avc_has_perm(&selinux_state, current_sid(), SECINITSID_SECURITY, @@ -666,7 +664,7 @@ out1: from_kuid(&init_user_ns, audit_get_loginuid(current)), audit_get_sessionid(current)); out: - mutex_unlock(&fsi->mutex); + mutex_unlock(&fsi->state->policy_mutex); vfree(data); return length; } @@ -1271,7 +1269,7 @@ static ssize_t sel_read_bool(struct file *filep, char __user *buf, unsigned index = file_inode(filep)->i_ino & SEL_INO_MASK; const char *name = filep->f_path.dentry->d_name.name; - mutex_lock(&fsi->mutex); + mutex_lock(&fsi->state->policy_mutex); ret = -EINVAL; if (index >= fsi->bool_num || strcmp(name, @@ -1290,14 +1288,14 @@ static ssize_t sel_read_bool(struct file *filep, char __user *buf, } length = scnprintf(page, PAGE_SIZE, "%d %d", cur_enforcing, fsi->bool_pending_values[index]); - mutex_unlock(&fsi->mutex); + mutex_unlock(&fsi->state->policy_mutex); ret = simple_read_from_buffer(buf, count, ppos, page, length); out_free: free_page((unsigned long)page); return ret; out_unlock: - mutex_unlock(&fsi->mutex); + mutex_unlock(&fsi->state->policy_mutex); goto out_free; } @@ -1322,7 +1320,7 @@ static ssize_t sel_write_bool(struct file *filep, const char __user *buf, if (IS_ERR(page)) return PTR_ERR(page); - mutex_lock(&fsi->mutex); + mutex_lock(&fsi->state->policy_mutex); length = avc_has_perm(&selinux_state, current_sid(), SECINITSID_SECURITY, @@ -1347,7 +1345,7 @@ static ssize_t sel_write_bool(struct file *filep, const char __user *buf, length = count; out: - mutex_unlock(&fsi->mutex); + mutex_unlock(&fsi->state->policy_mutex); kfree(page); return length; } @@ -1378,7 +1376,7 @@ static ssize_t sel_commit_bools_write(struct file *filep, if (IS_ERR(page)) return PTR_ERR(page); - mutex_lock(&fsi->mutex); + mutex_lock(&fsi->state->policy_mutex); length = avc_has_perm(&selinux_state, current_sid(), SECINITSID_SECURITY, @@ -1400,7 +1398,7 @@ static ssize_t sel_commit_bools_write(struct file *filep, length = count; out: - mutex_unlock(&fsi->mutex); + mutex_unlock(&fsi->state->policy_mutex); kfree(page); return length; } diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c index e730204f060b..85cfd46836c7 100644 --- a/security/selinux/ss/services.c +++ b/security/selinux/ss/services.c @@ -2163,13 +2163,8 @@ void selinux_policy_cancel(struct selinux_state *state, { struct selinux_policy *oldpolicy; - /* - * NOTE: We do not need to take the rcu read lock - * around the code below because other policy-modifying - * operations are already excluded by selinuxfs via - * fsi->mutex. - */ - oldpolicy = rcu_dereference_check(state->policy, 1); + oldpolicy = rcu_dereference_protected(state->policy, + lockdep_is_held(&state->policy_mutex)); sidtab_cancel_convert(oldpolicy->sidtab); selinux_policy_free(policy); @@ -2192,13 +2187,8 @@ void selinux_policy_commit(struct selinux_state *state, struct selinux_policy *oldpolicy; u32 seqno; - /* - * NOTE: We do not need to take the rcu read lock - * around the code below because other policy-modifying - * operations are already excluded by selinuxfs via - * fsi->mutex. - */ - oldpolicy = rcu_dereference_check(state->policy, 1); + oldpolicy = rcu_dereference_protected(state->policy, + lockdep_is_held(&state->policy_mutex)); /* If switching between different policy types, log MLS status */ if (oldpolicy) { @@ -2291,13 +2281,8 @@ int security_load_policy(struct selinux_state *state, void *data, size_t len, return 0; } - /* - * NOTE: We do not need to take the rcu read lock - * around the code below because other policy-modifying - * operations are already excluded by selinuxfs via - * fsi->mutex. - */ - oldpolicy = rcu_dereference_check(state->policy, 1); + oldpolicy = rcu_dereference_protected(state->policy, + lockdep_is_held(&state->policy_mutex)); /* Preserve active boolean values from the old policy */ rc = security_preserve_bools(oldpolicy, newpolicy); @@ -3013,14 +2998,8 @@ int security_set_bools(struct selinux_state *state, u32 len, int *values) if (!selinux_initialized(state)) return -EINVAL; - /* - * NOTE: We do not need to take the rcu read lock - * around the code below because other policy-modifying - * operations are already excluded by selinuxfs via - * fsi->mutex. - */ - - oldpolicy = rcu_dereference_check(state->policy, 1); + oldpolicy = rcu_dereference_protected(state->policy, + lockdep_is_held(&state->policy_mutex)); /* Consistency check on number of booleans, should never fail */ if (WARN_ON(len != oldpolicy->policydb.p_bools.nprim)) -- cgit v1.2.3 From 66ccd2560affc6e653ef7372ea36fb825743d186 Mon Sep 17 00:00:00 2001 From: Ondrej Mosnacek Date: Thu, 27 Aug 2020 18:27:53 +0200 Subject: selinux: simplify away security_policydb_len() Remove the security_policydb_len() calls from sel_open_policy() and instead update the inode size from the size returned from security_read_policy(). Since after this change security_policydb_len() is only called from security_load_policy(), remove it entirely and just open-code it there. Also, since security_load_policy() is always called with policy_mutex held, make it dereference the policy pointer directly and drop the unnecessary RCU locking. Signed-off-by: Ondrej Mosnacek Acked-by: Stephen Smalley Signed-off-by: Paul Moore --- security/selinux/include/security.h | 1 - security/selinux/selinuxfs.c | 12 ++++++------ security/selinux/ss/services.c | 27 ++++----------------------- 3 files changed, 10 insertions(+), 30 deletions(-) (limited to 'security') diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h index bbbf7141ccdb..cbdd3c7aff8b 100644 --- a/security/selinux/include/security.h +++ b/security/selinux/include/security.h @@ -219,7 +219,6 @@ void selinux_policy_cancel(struct selinux_state *state, struct selinux_policy *policy); int security_read_policy(struct selinux_state *state, void **data, size_t *len); -size_t security_policydb_len(struct selinux_state *state); int security_policycap_supported(struct selinux_state *state, unsigned int req_cap); diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c index 29567acdda21..45e9efa9bf5b 100644 --- a/security/selinux/selinuxfs.c +++ b/security/selinux/selinuxfs.c @@ -415,16 +415,16 @@ static int sel_open_policy(struct inode *inode, struct file *filp) if (!plm) goto err; - if (i_size_read(inode) != security_policydb_len(state)) { - inode_lock(inode); - i_size_write(inode, security_policydb_len(state)); - inode_unlock(inode); - } - rc = security_read_policy(state, &plm->data, &plm->len); if (rc) goto err; + if ((size_t)i_size_read(inode) != plm->len) { + inode_lock(inode); + i_size_write(inode, plm->len); + inode_unlock(inode); + } + fsi->policy_opened = 1; filp->private_data = plm; diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c index 85cfd46836c7..8dc111fbe23a 100644 --- a/security/selinux/ss/services.c +++ b/security/selinux/ss/services.c @@ -2328,22 +2328,6 @@ err_policy: return rc; } -size_t security_policydb_len(struct selinux_state *state) -{ - struct selinux_policy *policy; - size_t len; - - if (!selinux_initialized(state)) - return 0; - - rcu_read_lock(); - policy = rcu_dereference(state->policy); - len = policy->policydb.len; - rcu_read_unlock(); - - return len; -} - /** * security_port_sid - Obtain the SID for a port. * @protocol: protocol number @@ -3903,11 +3887,12 @@ int security_read_policy(struct selinux_state *state, int rc; struct policy_file fp; - if (!selinux_initialized(state)) + policy = rcu_dereference_protected( + state->policy, lockdep_is_held(&state->policy_mutex)); + if (!policy) return -EINVAL; - *len = security_policydb_len(state); - + *len = policy->policydb.len; *data = vmalloc_user(*len); if (!*data) return -ENOMEM; @@ -3915,11 +3900,7 @@ int security_read_policy(struct selinux_state *state, fp.data = *data; fp.len = *len; - rcu_read_lock(); - policy = rcu_dereference(state->policy); rc = policydb_write(&policy->policydb, &fp); - rcu_read_unlock(); - if (rc) return rc; -- cgit v1.2.3 From e8ba53d0023a76ba0f50e6ee3e6288c5442f9d33 Mon Sep 17 00:00:00 2001 From: Stephen Smalley Date: Thu, 10 Sep 2020 10:28:05 -0400 Subject: selinux: access policycaps with READ_ONCE/WRITE_ONCE Use READ_ONCE/WRITE_ONCE for all accesses to the selinux_state.policycaps booleans to prevent compiler mischief. Signed-off-by: Stephen Smalley Signed-off-by: Paul Moore --- security/selinux/include/security.h | 14 +++++++------- security/selinux/ss/services.c | 3 ++- 2 files changed, 9 insertions(+), 8 deletions(-) (limited to 'security') diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h index cbdd3c7aff8b..0ce2ef684ed0 100644 --- a/security/selinux/include/security.h +++ b/security/selinux/include/security.h @@ -164,49 +164,49 @@ static inline bool selinux_policycap_netpeer(void) { struct selinux_state *state = &selinux_state; - return state->policycap[POLICYDB_CAPABILITY_NETPEER]; + return READ_ONCE(state->policycap[POLICYDB_CAPABILITY_NETPEER]); } static inline bool selinux_policycap_openperm(void) { struct selinux_state *state = &selinux_state; - return state->policycap[POLICYDB_CAPABILITY_OPENPERM]; + return READ_ONCE(state->policycap[POLICYDB_CAPABILITY_OPENPERM]); } static inline bool selinux_policycap_extsockclass(void) { struct selinux_state *state = &selinux_state; - return state->policycap[POLICYDB_CAPABILITY_EXTSOCKCLASS]; + return READ_ONCE(state->policycap[POLICYDB_CAPABILITY_EXTSOCKCLASS]); } static inline bool selinux_policycap_alwaysnetwork(void) { struct selinux_state *state = &selinux_state; - return state->policycap[POLICYDB_CAPABILITY_ALWAYSNETWORK]; + return READ_ONCE(state->policycap[POLICYDB_CAPABILITY_ALWAYSNETWORK]); } static inline bool selinux_policycap_cgroupseclabel(void) { struct selinux_state *state = &selinux_state; - return state->policycap[POLICYDB_CAPABILITY_CGROUPSECLABEL]; + return READ_ONCE(state->policycap[POLICYDB_CAPABILITY_CGROUPSECLABEL]); } static inline bool selinux_policycap_nnp_nosuid_transition(void) { struct selinux_state *state = &selinux_state; - return state->policycap[POLICYDB_CAPABILITY_NNP_NOSUID_TRANSITION]; + return READ_ONCE(state->policycap[POLICYDB_CAPABILITY_NNP_NOSUID_TRANSITION]); } static inline bool selinux_policycap_genfs_seclabel_symlinks(void) { struct selinux_state *state = &selinux_state; - return state->policycap[POLICYDB_CAPABILITY_GENFS_SECLABEL_SYMLINKS]; + return READ_ONCE(state->policycap[POLICYDB_CAPABILITY_GENFS_SECLABEL_SYMLINKS]); } int security_mls_enabled(struct selinux_state *state); diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c index 8dc111fbe23a..9704c8a32303 100644 --- a/security/selinux/ss/services.c +++ b/security/selinux/ss/services.c @@ -2123,7 +2123,8 @@ static void security_load_policycaps(struct selinux_state *state, p = &policy->policydb; for (i = 0; i < ARRAY_SIZE(state->policycap); i++) - state->policycap[i] = ebitmap_get_bit(&p->policycaps, i); + WRITE_ONCE(state->policycap[i], + ebitmap_get_bit(&p->policycaps, i)); for (i = 0; i < ARRAY_SIZE(selinux_policycap_names); i++) pr_info("SELinux: policy capability %s=%d\n", -- cgit v1.2.3 From 8861d0af642c646c8e148ce34c294bdef6f32f6a Mon Sep 17 00:00:00 2001 From: Lakshmi Ramasubramanian Date: Mon, 14 Sep 2020 10:31:57 -0700 Subject: selinux: Add helper functions to get and set checkreqprot checkreqprot data member in selinux_state struct is accessed directly by SELinux functions to get and set. This could cause unexpected read or write access to this data member due to compiler optimizations and/or compiler's reordering of access to this field. Add helper functions to get and set checkreqprot data member in selinux_state struct. These helper functions use READ_ONCE and WRITE_ONCE macros to ensure atomic read or write of memory for this data member. Signed-off-by: Lakshmi Ramasubramanian Suggested-by: Stephen Smalley Suggested-by: Paul Moore Acked-by: Stephen Smalley Signed-off-by: Paul Moore --- security/selinux/hooks.c | 6 +++--- security/selinux/include/security.h | 10 ++++++++++ security/selinux/selinuxfs.c | 5 +++-- 3 files changed, 16 insertions(+), 5 deletions(-) (limited to 'security') diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 6210e98219a5..d6b182c11700 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -3718,7 +3718,7 @@ static int selinux_mmap_file(struct file *file, unsigned long reqprot, return rc; } - if (selinux_state.checkreqprot) + if (checkreqprot_get(&selinux_state)) prot = reqprot; return file_map_prot_check(file, prot, @@ -3732,7 +3732,7 @@ static int selinux_file_mprotect(struct vm_area_struct *vma, const struct cred *cred = current_cred(); u32 sid = cred_sid(cred); - if (selinux_state.checkreqprot) + if (checkreqprot_get(&selinux_state)) prot = reqprot; if (default_noexec && @@ -7234,7 +7234,7 @@ static __init int selinux_init(void) memset(&selinux_state, 0, sizeof(selinux_state)); enforcing_set(&selinux_state, selinux_enforcing_boot); - selinux_state.checkreqprot = selinux_checkreqprot_boot; + checkreqprot_set(&selinux_state, selinux_checkreqprot_boot); selinux_avc_init(&selinux_state.avc); mutex_init(&selinux_state.status_lock); mutex_init(&selinux_state.policy_mutex); diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h index 0ce2ef684ed0..3cc8bab31ea8 100644 --- a/security/selinux/include/security.h +++ b/security/selinux/include/security.h @@ -143,6 +143,16 @@ static inline void enforcing_set(struct selinux_state *state, bool value) } #endif +static inline bool checkreqprot_get(const struct selinux_state *state) +{ + return READ_ONCE(state->checkreqprot); +} + +static inline void checkreqprot_set(struct selinux_state *state, bool value) +{ + WRITE_ONCE(state->checkreqprot, value); +} + #ifdef CONFIG_SECURITY_SELINUX_DISABLE static inline bool selinux_disabled(struct selinux_state *state) { diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c index 45e9efa9bf5b..4bde570d56a2 100644 --- a/security/selinux/selinuxfs.c +++ b/security/selinux/selinuxfs.c @@ -717,7 +717,8 @@ static ssize_t sel_read_checkreqprot(struct file *filp, char __user *buf, char tmpbuf[TMPBUFLEN]; ssize_t length; - length = scnprintf(tmpbuf, TMPBUFLEN, "%u", fsi->state->checkreqprot); + length = scnprintf(tmpbuf, TMPBUFLEN, "%u", + checkreqprot_get(fsi->state)); return simple_read_from_buffer(buf, count, ppos, tmpbuf, length); } @@ -759,7 +760,7 @@ static ssize_t sel_write_checkreqprot(struct file *file, const char __user *buf, comm, current->pid); } - fsi->state->checkreqprot = new_value ? 1 : 0; + checkreqprot_set(fsi->state, (new_value ? 1 : 0)); length = count; out: kfree(page); -- cgit v1.2.3