From 66f8e2f03c02e812002f8e9e465681cc62edda5b Mon Sep 17 00:00:00 2001 From: Jeff Vander Stoep Date: Fri, 22 Nov 2019 10:33:06 +0100 Subject: [PATCH 01/28] selinux: sidtab reverse lookup hash table This replaces the reverse table lookup and reverse cache with a hashtable which improves cache-miss reverse-lookup times from O(n) to O(1)* and maintains the same performance as a reverse cache hit. This reduces the time needed to add a new sidtab entry from ~500us to 5us on a Pixel 3 when there are ~10,000 sidtab entries. The implementation uses the kernel's generic hashtable API, It uses the context's string represtation as the hash source, and the kernels generic string hashing algorithm full_name_hash() to reduce the string to a 32 bit value. This change also maintains the improvement introduced in commit ee1a84fdfeed ("selinux: overhaul sidtab to fix bug and improve performance") which removed the need to keep the current sidtab locked during policy reload. It does however introduce periodic locking of the target sidtab while converting the hashtable. Sidtab entries are never modified or removed, so the context struct stored in the sid_to_context tree can also be used for the context_to_sid hashtable to reduce memory usage. This bug was reported by: - On the selinux bug tracker. BUG: kernel softlockup due to too many SIDs/contexts #37 https://github.com/SELinuxProject/selinux-kernel/issues/37 - Jovana Knezevic on Android's bugtracker. Bug: 140252993 "During multi-user performance testing, we create and remove users many times. selinux_android_restorecon_pkgdir goes from 1ms to over 20ms after about 200 user creations and removals. Accumulated over ~280 packages, that adds a significant time to user creation, making perf benchmarks unreliable." * Hashtable lookup is only O(1) when n < the number of buckets. Signed-off-by: Jeff Vander Stoep Reported-by: Stephen Smalley Reported-by: Jovana Knezevic Reviewed-by: Stephen Smalley Tested-by: Stephen Smalley [PM: subj tweak, removed changelog from patch description] Signed-off-by: Paul Moore --- security/selinux/Kconfig | 12 ++ security/selinux/include/security.h | 1 + security/selinux/selinuxfs.c | 65 +++++++ security/selinux/ss/context.h | 11 +- security/selinux/ss/policydb.c | 5 + security/selinux/ss/services.c | 96 +++++++--- security/selinux/ss/services.h | 4 +- security/selinux/ss/sidtab.c | 263 ++++++++++++++-------------- security/selinux/ss/sidtab.h | 16 +- 9 files changed, 306 insertions(+), 167 deletions(-) diff --git a/security/selinux/Kconfig b/security/selinux/Kconfig index 5711689deb6a..c9e576c430c2 100644 --- a/security/selinux/Kconfig +++ b/security/selinux/Kconfig @@ -85,3 +85,15 @@ config SECURITY_SELINUX_CHECKREQPROT_VALUE via /selinux/checkreqprot if authorized by policy. If you are unsure how to answer this question, answer 0. + +config SECURITY_SELINUX_SIDTAB_HASH_BITS + int "NSA SELinux sidtab hashtable size" + depends on SECURITY_SELINUX + range 8 13 + default 9 + help + This option sets the number of buckets used in the sidtab hashtable + to 2^SECURITY_SELINUX_SIDTAB_HASH_BITS buckets. The number of hash + collisions may be viewed at /sys/fs/selinux/ss/sidtab_hash_stats. If + chain lengths are high (e.g. > 20) then selecting a higher value here + will ensure that lookups times are short and stable. diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h index ae840634e3c7..8c0dbbd076c6 100644 --- a/security/selinux/include/security.h +++ b/security/selinux/include/security.h @@ -395,5 +395,6 @@ extern int selinux_nlmsg_lookup(u16 sclass, u16 nlmsg_type, u32 *perm); extern void avtab_cache_init(void); extern void ebitmap_cache_init(void); extern void hashtab_cache_init(void); +extern int security_sidtab_hash_stats(struct selinux_state *state, char *page); #endif /* _SELINUX_SECURITY_H_ */ diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c index ee94fa469c29..dd7bb1f1dc99 100644 --- a/security/selinux/selinuxfs.c +++ b/security/selinux/selinuxfs.c @@ -1482,6 +1482,32 @@ static ssize_t sel_read_avc_hash_stats(struct file *filp, char __user *buf, return length; } +static ssize_t sel_read_sidtab_hash_stats(struct file *filp, char __user *buf, + size_t count, loff_t *ppos) +{ + struct selinux_fs_info *fsi = file_inode(filp)->i_sb->s_fs_info; + struct selinux_state *state = fsi->state; + char *page; + ssize_t length; + + page = (char *)__get_free_page(GFP_KERNEL); + if (!page) + return -ENOMEM; + + length = security_sidtab_hash_stats(state, page); + if (length >= 0) + length = simple_read_from_buffer(buf, count, ppos, page, + length); + free_page((unsigned long)page); + + return length; +} + +static const struct file_operations sel_sidtab_hash_stats_ops = { + .read = sel_read_sidtab_hash_stats, + .llseek = generic_file_llseek, +}; + static const struct file_operations sel_avc_cache_threshold_ops = { .read = sel_read_avc_cache_threshold, .write = sel_write_avc_cache_threshold, @@ -1599,6 +1625,37 @@ static int sel_make_avc_files(struct dentry *dir) return 0; } +static int sel_make_ss_files(struct dentry *dir) +{ + struct super_block *sb = dir->d_sb; + struct selinux_fs_info *fsi = sb->s_fs_info; + int i; + static struct tree_descr files[] = { + { "sidtab_hash_stats", &sel_sidtab_hash_stats_ops, S_IRUGO }, + }; + + for (i = 0; i < ARRAY_SIZE(files); i++) { + struct inode *inode; + struct dentry *dentry; + + dentry = d_alloc_name(dir, files[i].name); + if (!dentry) + return -ENOMEM; + + inode = sel_make_inode(dir->d_sb, S_IFREG|files[i].mode); + if (!inode) { + dput(dentry); + return -ENOMEM; + } + + inode->i_fop = files[i].ops; + inode->i_ino = ++fsi->last_ino; + d_add(dentry, inode); + } + + return 0; +} + static ssize_t sel_read_initcon(struct file *file, char __user *buf, size_t count, loff_t *ppos) { @@ -1963,6 +2020,14 @@ static int sel_fill_super(struct super_block *sb, struct fs_context *fc) } ret = sel_make_avc_files(dentry); + + dentry = sel_make_dir(sb->s_root, "ss", &fsi->last_ino); + if (IS_ERR(dentry)) { + ret = PTR_ERR(dentry); + goto err; + } + + ret = sel_make_ss_files(dentry); if (ret) goto err; diff --git a/security/selinux/ss/context.h b/security/selinux/ss/context.h index 513e67f48878..3ba044fe02ed 100644 --- a/security/selinux/ss/context.h +++ b/security/selinux/ss/context.h @@ -31,6 +31,7 @@ struct context { u32 len; /* length of string in bytes */ struct mls_range range; char *str; /* string representation if context cannot be mapped. */ + u32 hash; /* a hash of the string representation */ }; static inline void mls_context_init(struct context *c) @@ -168,12 +169,13 @@ static inline int context_cpy(struct context *dst, struct context *src) kfree(dst->str); return rc; } + dst->hash = src->hash; return 0; } static inline void context_destroy(struct context *c) { - c->user = c->role = c->type = 0; + c->user = c->role = c->type = c->hash = 0; kfree(c->str); c->str = NULL; c->len = 0; @@ -182,6 +184,8 @@ static inline void context_destroy(struct context *c) static inline int context_cmp(struct context *c1, struct context *c2) { + if (c1->hash && c2->hash && (c1->hash != c2->hash)) + return 0; if (c1->len && c2->len) return (c1->len == c2->len && !strcmp(c1->str, c2->str)); if (c1->len || c2->len) @@ -192,5 +196,10 @@ static inline int context_cmp(struct context *c1, struct context *c2) mls_context_cmp(c1, c2)); } +static inline unsigned int context_compute_hash(const char *s) +{ + return full_name_hash(NULL, s, strlen(s)); +} + #endif /* _SS_CONTEXT_H_ */ diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c index e20624a68f5d..e369b0092cdf 100644 --- a/security/selinux/ss/policydb.c +++ b/security/selinux/ss/policydb.c @@ -878,6 +878,11 @@ int policydb_load_isids(struct policydb *p, struct sidtab *s) sidtab_destroy(s); goto out; } + rc = context_add_hash(p, &c->context[0]); + if (rc) { + sidtab_destroy(s); + goto out; + } rc = sidtab_set_initial(s, c->sid[0], &c->context[0]); if (rc) { diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c index a5813c7629c1..38fb6fdd65ca 100644 --- a/security/selinux/ss/services.c +++ b/security/selinux/ss/services.c @@ -1257,6 +1257,17 @@ static int context_struct_to_string(struct policydb *p, #include "initial_sid_to_string.h" +int security_sidtab_hash_stats(struct selinux_state *state, char *page) +{ + int rc; + + read_lock(&state->ss->policy_rwlock); + rc = sidtab_hash_stats(state->ss->sidtab, page); + read_unlock(&state->ss->policy_rwlock); + + return rc; +} + const char *security_get_initial_sid_context(u32 sid) { if (unlikely(sid > SECINITSID_NUM)) @@ -1449,6 +1460,42 @@ out: return rc; } +int context_add_hash(struct policydb *policydb, + struct context *context) +{ + int rc; + char *str; + int len; + + if (context->str) { + context->hash = context_compute_hash(context->str); + } else { + rc = context_struct_to_string(policydb, context, + &str, &len); + if (rc) + return rc; + context->hash = context_compute_hash(str); + kfree(str); + } + return 0; +} + +static int context_struct_to_sid(struct selinux_state *state, + struct context *context, u32 *sid) +{ + int rc; + struct sidtab *sidtab = state->ss->sidtab; + struct policydb *policydb = &state->ss->policydb; + + if (!context->hash) { + rc = context_add_hash(policydb, context); + if (rc) + return rc; + } + + return sidtab_context_to_sid(sidtab, context, sid); +} + static int security_context_to_sid_core(struct selinux_state *state, const char *scontext, u32 scontext_len, u32 *sid, u32 def_sid, gfp_t gfp_flags, @@ -1501,7 +1548,7 @@ static int security_context_to_sid_core(struct selinux_state *state, str = NULL; } else if (rc) goto out_unlock; - rc = sidtab_context_to_sid(sidtab, &context, sid); + rc = context_struct_to_sid(state, &context, sid); context_destroy(&context); out_unlock: read_unlock(&state->ss->policy_rwlock); @@ -1805,7 +1852,7 @@ static int security_compute_sid(struct selinux_state *state, goto out_unlock; } /* Obtain the sid for the context. */ - rc = sidtab_context_to_sid(sidtab, &newcontext, out_sid); + rc = context_struct_to_sid(state, &newcontext, out_sid); out_unlock: read_unlock(&state->ss->policy_rwlock); context_destroy(&newcontext); @@ -1957,6 +2004,7 @@ static int convert_context(struct context *oldc, struct context *newc, void *p) context_init(newc); newc->str = s; newc->len = oldc->len; + newc->hash = oldc->hash; return 0; } kfree(s); @@ -2033,6 +2081,10 @@ static int convert_context(struct context *oldc, struct context *newc, void *p) goto bad; } + rc = context_add_hash(args->newp, newc); + if (rc) + goto bad; + return 0; bad: /* Map old representation to string and save it. */ @@ -2042,6 +2094,7 @@ bad: context_destroy(newc); newc->str = s; newc->len = len; + newc->hash = context_compute_hash(s); pr_info("SELinux: Context %s became invalid (unmapped).\n", newc->str); return 0; @@ -2280,8 +2333,7 @@ int security_port_sid(struct selinux_state *state, if (c) { if (!c->sid[0]) { - rc = sidtab_context_to_sid(sidtab, - &c->context[0], + rc = context_struct_to_sid(state, &c->context[0], &c->sid[0]); if (rc) goto out; @@ -2306,14 +2358,12 @@ int security_ib_pkey_sid(struct selinux_state *state, u64 subnet_prefix, u16 pkey_num, u32 *out_sid) { struct policydb *policydb; - struct sidtab *sidtab; struct ocontext *c; int rc = 0; read_lock(&state->ss->policy_rwlock); policydb = &state->ss->policydb; - sidtab = state->ss->sidtab; c = policydb->ocontexts[OCON_IBPKEY]; while (c) { @@ -2327,7 +2377,7 @@ int security_ib_pkey_sid(struct selinux_state *state, if (c) { if (!c->sid[0]) { - rc = sidtab_context_to_sid(sidtab, + rc = context_struct_to_sid(state, &c->context[0], &c->sid[0]); if (rc) @@ -2374,8 +2424,7 @@ int security_ib_endport_sid(struct selinux_state *state, if (c) { if (!c->sid[0]) { - rc = sidtab_context_to_sid(sidtab, - &c->context[0], + rc = context_struct_to_sid(state, &c->context[0], &c->sid[0]); if (rc) goto out; @@ -2416,13 +2465,11 @@ int security_netif_sid(struct selinux_state *state, if (c) { if (!c->sid[0] || !c->sid[1]) { - rc = sidtab_context_to_sid(sidtab, - &c->context[0], - &c->sid[0]); + rc = context_struct_to_sid(state, &c->context[0], + &c->sid[0]); if (rc) goto out; - rc = sidtab_context_to_sid(sidtab, - &c->context[1], + rc = context_struct_to_sid(state, &c->context[1], &c->sid[1]); if (rc) goto out; @@ -2463,14 +2510,12 @@ int security_node_sid(struct selinux_state *state, u32 *out_sid) { struct policydb *policydb; - struct sidtab *sidtab; int rc; struct ocontext *c; read_lock(&state->ss->policy_rwlock); policydb = &state->ss->policydb; - sidtab = state->ss->sidtab; switch (domain) { case AF_INET: { @@ -2512,7 +2557,7 @@ int security_node_sid(struct selinux_state *state, if (c) { if (!c->sid[0]) { - rc = sidtab_context_to_sid(sidtab, + rc = context_struct_to_sid(state, &c->context[0], &c->sid[0]); if (rc) @@ -2596,12 +2641,17 @@ int security_get_user_sids(struct selinux_state *state, usercon.role = i + 1; ebitmap_for_each_positive_bit(&role->types, tnode, j) { usercon.type = j + 1; + /* + * The same context struct is reused here so the hash + * must be reset. + */ + usercon.hash = 0; if (mls_setup_user_range(policydb, fromcon, user, &usercon)) continue; - rc = sidtab_context_to_sid(sidtab, &usercon, &sid); + rc = context_struct_to_sid(state, &usercon, &sid); if (rc) goto out_unlock; if (mynel < maxnel) { @@ -2672,7 +2722,6 @@ static inline int __security_genfs_sid(struct selinux_state *state, u32 *sid) { struct policydb *policydb = &state->ss->policydb; - struct sidtab *sidtab = state->ss->sidtab; int len; u16 sclass; struct genfs *genfs; @@ -2707,7 +2756,7 @@ static inline int __security_genfs_sid(struct selinux_state *state, goto out; if (!c->sid[0]) { - rc = sidtab_context_to_sid(sidtab, &c->context[0], &c->sid[0]); + rc = context_struct_to_sid(state, &c->context[0], &c->sid[0]); if (rc) goto out; } @@ -2770,7 +2819,7 @@ int security_fs_use(struct selinux_state *state, struct super_block *sb) if (c) { sbsec->behavior = c->v.behavior; if (!c->sid[0]) { - rc = sidtab_context_to_sid(sidtab, &c->context[0], + rc = context_struct_to_sid(state, &c->context[0], &c->sid[0]); if (rc) goto out; @@ -3026,8 +3075,7 @@ int security_sid_mls_copy(struct selinux_state *state, goto out_unlock; } } - - rc = sidtab_context_to_sid(sidtab, &newcon, new_sid); + rc = context_struct_to_sid(state, &newcon, new_sid); out_unlock: read_unlock(&state->ss->policy_rwlock); context_destroy(&newcon); @@ -3620,7 +3668,7 @@ int security_netlbl_secattr_to_sid(struct selinux_state *state, if (!mls_context_isvalid(policydb, &ctx_new)) goto out_free; - rc = sidtab_context_to_sid(sidtab, &ctx_new, sid); + rc = context_struct_to_sid(state, &ctx_new, sid); if (rc) goto out_free; diff --git a/security/selinux/ss/services.h b/security/selinux/ss/services.h index 9a36de860368..fc40640a9725 100644 --- a/security/selinux/ss/services.h +++ b/security/selinux/ss/services.h @@ -8,7 +8,7 @@ #define _SS_SERVICES_H_ #include "policydb.h" -#include "sidtab.h" +#include "context.h" /* Mapping for a single class */ struct selinux_mapping { @@ -39,4 +39,6 @@ void services_compute_xperms_drivers(struct extended_perms *xperms, void services_compute_xperms_decision(struct extended_perms_decision *xpermd, struct avtab_node *node); +int context_add_hash(struct policydb *policydb, struct context *context); + #endif /* _SS_SERVICES_H_ */ diff --git a/security/selinux/ss/sidtab.c b/security/selinux/ss/sidtab.c index 7d49994e8d5f..d9d8599e8e63 100644 --- a/security/selinux/ss/sidtab.c +++ b/security/selinux/ss/sidtab.c @@ -17,26 +17,43 @@ #include "security.h" #include "sidtab.h" +#define index_to_sid(index) (index + SECINITSID_NUM + 1) +#define sid_to_index(sid) (sid - (SECINITSID_NUM + 1)) + int sidtab_init(struct sidtab *s) { u32 i; memset(s->roots, 0, sizeof(s->roots)); - /* max count is SIDTAB_MAX so valid index is always < SIDTAB_MAX */ - for (i = 0; i < SIDTAB_RCACHE_SIZE; i++) - s->rcache[i] = SIDTAB_MAX; - for (i = 0; i < SECINITSID_NUM; i++) s->isids[i].set = 0; s->count = 0; s->convert = NULL; + hash_init(s->context_to_sid); spin_lock_init(&s->lock); return 0; } +static u32 context_to_sid(struct sidtab *s, struct context *context) +{ + struct sidtab_entry_leaf *entry; + u32 sid = 0; + + rcu_read_lock(); + hash_for_each_possible_rcu(s->context_to_sid, entry, list, + context->hash) { + if (context_cmp(&entry->context, context)) { + sid = entry->sid; + break; + } + } + rcu_read_unlock(); + return sid; +} + int sidtab_set_initial(struct sidtab *s, u32 sid, struct context *context) { struct sidtab_isid_entry *entry; @@ -47,14 +64,60 @@ int sidtab_set_initial(struct sidtab *s, u32 sid, struct context *context) entry = &s->isids[sid - 1]; - rc = context_cpy(&entry->context, context); + rc = context_cpy(&entry->leaf.context, context); if (rc) return rc; entry->set = 1; + + /* + * Multiple initial sids may map to the same context. Check that this + * context is not already represented in the context_to_sid hashtable + * to avoid duplicate entries and long linked lists upon hash + * collision. + */ + if (!context_to_sid(s, context)) { + entry->leaf.sid = sid; + hash_add(s->context_to_sid, &entry->leaf.list, context->hash); + } + return 0; } +int sidtab_hash_stats(struct sidtab *sidtab, char *page) +{ + int i; + int chain_len = 0; + int slots_used = 0; + int entries = 0; + int max_chain_len = 0; + int cur_bucket = 0; + struct sidtab_entry_leaf *entry; + + rcu_read_lock(); + hash_for_each_rcu(sidtab->context_to_sid, i, entry, list) { + entries++; + if (i == cur_bucket) { + chain_len++; + if (chain_len == 1) + slots_used++; + } else { + cur_bucket = i; + if (chain_len > max_chain_len) + max_chain_len = chain_len; + chain_len = 0; + } + } + rcu_read_unlock(); + + if (chain_len > max_chain_len) + max_chain_len = chain_len; + + return scnprintf(page, PAGE_SIZE, "entries: %d\nbuckets used: %d/%d\n" + "longest chain: %d\n", entries, + slots_used, SIDTAB_HASH_BUCKETS, max_chain_len); +} + static u32 sidtab_level_from_count(u32 count) { u32 capacity = SIDTAB_LEAF_ENTRIES; @@ -88,7 +151,8 @@ static int sidtab_alloc_roots(struct sidtab *s, u32 level) return 0; } -static struct context *sidtab_do_lookup(struct sidtab *s, u32 index, int alloc) +static struct sidtab_entry_leaf *sidtab_do_lookup(struct sidtab *s, u32 index, + int alloc) { union sidtab_entry_inner *entry; u32 level, capacity_shift, leaf_index = index / SIDTAB_LEAF_ENTRIES; @@ -125,7 +189,7 @@ static struct context *sidtab_do_lookup(struct sidtab *s, u32 index, int alloc) if (!entry->ptr_leaf) return NULL; } - return &entry->ptr_leaf->entries[index % SIDTAB_LEAF_ENTRIES].context; + return &entry->ptr_leaf->entries[index % SIDTAB_LEAF_ENTRIES]; } static struct context *sidtab_lookup(struct sidtab *s, u32 index) @@ -136,12 +200,12 @@ static struct context *sidtab_lookup(struct sidtab *s, u32 index) if (index >= count) return NULL; - return sidtab_do_lookup(s, index, 0); + return &sidtab_do_lookup(s, index, 0)->context; } static struct context *sidtab_lookup_initial(struct sidtab *s, u32 sid) { - return s->isids[sid - 1].set ? &s->isids[sid - 1].context : NULL; + return s->isids[sid - 1].set ? &s->isids[sid - 1].leaf.context : NULL; } static struct context *sidtab_search_core(struct sidtab *s, u32 sid, int force) @@ -150,7 +214,7 @@ static struct context *sidtab_search_core(struct sidtab *s, u32 sid, int force) if (sid != 0) { if (sid > SECINITSID_NUM) - context = sidtab_lookup(s, sid - (SECINITSID_NUM + 1)); + context = sidtab_lookup(s, sid_to_index(sid)); else context = sidtab_lookup_initial(s, sid); if (context && (!context->len || force)) @@ -170,117 +234,30 @@ struct context *sidtab_search_force(struct sidtab *s, u32 sid) return sidtab_search_core(s, sid, 1); } -static int sidtab_find_context(union sidtab_entry_inner entry, - u32 *pos, u32 count, u32 level, - struct context *context, u32 *index) -{ - int rc; - u32 i; - - if (level != 0) { - struct sidtab_node_inner *node = entry.ptr_inner; - - i = 0; - while (i < SIDTAB_INNER_ENTRIES && *pos < count) { - rc = sidtab_find_context(node->entries[i], - pos, count, level - 1, - context, index); - if (rc == 0) - return 0; - i++; - } - } else { - struct sidtab_node_leaf *node = entry.ptr_leaf; - - i = 0; - while (i < SIDTAB_LEAF_ENTRIES && *pos < count) { - if (context_cmp(&node->entries[i].context, context)) { - *index = *pos; - return 0; - } - (*pos)++; - i++; - } - } - return -ENOENT; -} - -static void sidtab_rcache_update(struct sidtab *s, u32 index, u32 pos) -{ - while (pos > 0) { - WRITE_ONCE(s->rcache[pos], READ_ONCE(s->rcache[pos - 1])); - --pos; - } - WRITE_ONCE(s->rcache[0], index); -} - -static void sidtab_rcache_push(struct sidtab *s, u32 index) -{ - sidtab_rcache_update(s, index, SIDTAB_RCACHE_SIZE - 1); -} - -static int sidtab_rcache_search(struct sidtab *s, struct context *context, - u32 *index) -{ - u32 i; - - for (i = 0; i < SIDTAB_RCACHE_SIZE; i++) { - u32 v = READ_ONCE(s->rcache[i]); - - if (v >= SIDTAB_MAX) - continue; - - if (context_cmp(sidtab_do_lookup(s, v, 0), context)) { - sidtab_rcache_update(s, v, i); - *index = v; - return 0; - } - } - return -ENOENT; -} - -static int sidtab_reverse_lookup(struct sidtab *s, struct context *context, - u32 *index) +int sidtab_context_to_sid(struct sidtab *s, struct context *context, + u32 *sid) { unsigned long flags; - u32 count, count_locked, level, pos; + u32 count; struct sidtab_convert_params *convert; - struct context *dst, *dst_convert; + struct sidtab_entry_leaf *dst, *dst_convert; int rc; - rc = sidtab_rcache_search(s, context, index); - if (rc == 0) + *sid = context_to_sid(s, context); + if (*sid) return 0; - /* read entries only after reading count */ - count = smp_load_acquire(&s->count); - level = sidtab_level_from_count(count); - - pos = 0; - rc = sidtab_find_context(s->roots[level], &pos, count, level, - context, index); - if (rc == 0) { - sidtab_rcache_push(s, *index); - return 0; - } - /* lock-free search failed: lock, re-search, and insert if not found */ spin_lock_irqsave(&s->lock, flags); - convert = s->convert; - count_locked = s->count; - level = sidtab_level_from_count(count_locked); + rc = 0; + *sid = context_to_sid(s, context); + if (*sid) + goto out_unlock; - /* if count has changed before we acquired the lock, then catch up */ - while (count < count_locked) { - if (context_cmp(sidtab_do_lookup(s, count, 0), context)) { - sidtab_rcache_push(s, count); - *index = count; - rc = 0; - goto out_unlock; - } - ++count; - } + /* read entries only after reading count */ + count = smp_load_acquire(&s->count); + convert = s->convert; /* bail out if we already reached max entries */ rc = -EOVERFLOW; @@ -293,7 +270,9 @@ static int sidtab_reverse_lookup(struct sidtab *s, struct context *context, if (!dst) goto out_unlock; - rc = context_cpy(dst, context); + dst->sid = index_to_sid(count); + + rc = context_cpy(&dst->context, context); if (rc) goto out_unlock; @@ -305,29 +284,32 @@ static int sidtab_reverse_lookup(struct sidtab *s, struct context *context, rc = -ENOMEM; dst_convert = sidtab_do_lookup(convert->target, count, 1); if (!dst_convert) { - context_destroy(dst); + context_destroy(&dst->context); goto out_unlock; } - rc = convert->func(context, dst_convert, convert->args); + rc = convert->func(context, &dst_convert->context, + convert->args); if (rc) { - context_destroy(dst); + context_destroy(&dst->context); goto out_unlock; } - - /* at this point we know the insert won't fail */ + dst_convert->sid = index_to_sid(count); convert->target->count = count + 1; + + hash_add_rcu(convert->target->context_to_sid, + &dst_convert->list, dst_convert->context.hash); } if (context->len) pr_info("SELinux: Context %s is not valid (left unmapped).\n", context->str); - sidtab_rcache_push(s, count); - *index = count; + *sid = index_to_sid(count); - /* write entries before writing new count */ + /* write entries before updating count */ smp_store_release(&s->count, count + 1); + hash_add_rcu(s->context_to_sid, &dst->list, dst->context.hash); rc = 0; out_unlock: @@ -335,25 +317,19 @@ out_unlock: return rc; } -int sidtab_context_to_sid(struct sidtab *s, struct context *context, u32 *sid) +static void sidtab_convert_hashtable(struct sidtab *s, u32 count) { - int rc; + struct sidtab_entry_leaf *entry; u32 i; - for (i = 0; i < SECINITSID_NUM; i++) { - struct sidtab_isid_entry *entry = &s->isids[i]; + for (i = 0; i < count; i++) { + entry = sidtab_do_lookup(s, i, 0); + entry->sid = index_to_sid(i); + + hash_add_rcu(s->context_to_sid, &entry->list, + entry->context.hash); - if (entry->set && context_cmp(context, &entry->context)) { - *sid = i + 1; - return 0; - } } - - rc = sidtab_reverse_lookup(s, context, sid); - if (rc) - return rc; - *sid += SECINITSID_NUM + 1; - return 0; } static int sidtab_convert_tree(union sidtab_entry_inner *edst, @@ -400,6 +376,7 @@ static int sidtab_convert_tree(union sidtab_entry_inner *edst, } cond_resched(); } + return 0; } @@ -435,7 +412,7 @@ int sidtab_convert(struct sidtab *s, struct sidtab_convert_params *params) /* enable live convert of new entries */ s->convert = params; - /* we can safely do the rest of the conversion outside the lock */ + /* we can safely convert the tree outside the lock */ spin_unlock_irqrestore(&s->lock, flags); pr_info("SELinux: Converting %u SID table entries...\n", count); @@ -449,8 +426,17 @@ int sidtab_convert(struct sidtab *s, struct sidtab_convert_params *params) spin_lock_irqsave(&s->lock, flags); s->convert = NULL; spin_unlock_irqrestore(&s->lock, flags); + return rc; } - return rc; + /* + * The hashtable can also be modified in sidtab_context_to_sid() + * so we must re-acquire the lock here. + */ + spin_lock_irqsave(&s->lock, flags); + sidtab_convert_hashtable(params->target, count); + spin_unlock_irqrestore(&s->lock, flags); + + return 0; } static void sidtab_destroy_tree(union sidtab_entry_inner entry, u32 level) @@ -484,11 +470,16 @@ void sidtab_destroy(struct sidtab *s) for (i = 0; i < SECINITSID_NUM; i++) if (s->isids[i].set) - context_destroy(&s->isids[i].context); + context_destroy(&s->isids[i].leaf.context); level = SIDTAB_MAX_LEVEL; while (level && !s->roots[level].ptr_inner) --level; sidtab_destroy_tree(s->roots[level], level); + /* + * The context_to_sid hashtable's objects are all shared + * with the isids array and context tree, and so don't need + * to be cleaned up here. + */ } diff --git a/security/selinux/ss/sidtab.h b/security/selinux/ss/sidtab.h index 1f4763141aa1..e2809401c417 100644 --- a/security/selinux/ss/sidtab.h +++ b/security/selinux/ss/sidtab.h @@ -13,11 +13,14 @@ #include #include +#include #include "context.h" struct sidtab_entry_leaf { + u32 sid; struct context context; + struct hlist_node list; }; struct sidtab_node_inner; @@ -57,7 +60,7 @@ struct sidtab_node_inner { struct sidtab_isid_entry { int set; - struct context context; + struct sidtab_entry_leaf leaf; }; struct sidtab_convert_params { @@ -66,7 +69,8 @@ struct sidtab_convert_params { struct sidtab *target; }; -#define SIDTAB_RCACHE_SIZE 3 +#define SIDTAB_HASH_BITS CONFIG_SECURITY_SELINUX_SIDTAB_HASH_BITS +#define SIDTAB_HASH_BUCKETS (1 << SIDTAB_HASH_BITS) struct sidtab { /* @@ -83,11 +87,11 @@ struct sidtab { struct sidtab_convert_params *convert; spinlock_t lock; - /* reverse lookup cache - access atomically via {READ|WRITE}_ONCE() */ - u32 rcache[SIDTAB_RCACHE_SIZE]; - /* index == SID - 1 (no entry for SECSID_NULL) */ struct sidtab_isid_entry isids[SECINITSID_NUM]; + + /* Hash table for fast reverse context-to-sid lookups. */ + DECLARE_HASHTABLE(context_to_sid, SIDTAB_HASH_BITS); }; int sidtab_init(struct sidtab *s); @@ -101,6 +105,8 @@ int sidtab_context_to_sid(struct sidtab *s, struct context *context, u32 *sid); void sidtab_destroy(struct sidtab *s); +int sidtab_hash_stats(struct sidtab *sidtab, char *page); + #endif /* _SS_SIDTAB_H_ */ From d97bd23c2d7d866e99eb3a927c742715c85a90ef Mon Sep 17 00:00:00 2001 From: Ondrej Mosnacek Date: Tue, 26 Nov 2019 14:57:00 +0100 Subject: [PATCH 02/28] selinux: cache the SID -> context string translation Translating a context struct to string can be quite slow, especially if the context has a lot of category bits set. This can cause quite noticeable performance impact in situations where the translation needs to be done repeatedly. A common example is a UNIX datagram socket with the SO_PASSSEC option enabled, which is used e.g. by systemd-journald when receiving log messages via datagram socket. This scenario can be reproduced with: cat /dev/urandom | base64 | logger & timeout 30s perf record -p $(pidof systemd-journald) -a -g kill %1 perf report -g none --pretty raw | grep security_secid_to_secctx Before the caching introduced by this patch, computing the context string (security_secid_to_secctx() function) takes up ~65% of systemd-journald's CPU time (assuming a context with 1024 categories set and Fedora x86_64 release kernel configs). After this patch (assuming near-perfect cache hit ratio) this overhead is reduced to just ~2%. This patch addresses the issue by caching a certain number (compile-time configurable) of recently used context strings to speed up repeated translations of the same context, while using only a small amount of memory. The cache is integrated into the existing sidtab table by adding a field to each entry, which when not NULL contains an RCU-protected pointer to a cache entry containing the cached string. The cache entries are kept in a linked list sorted according to how recently they were used. On a cache miss when the cache is full, the least recently used entry is removed to make space for the new entry. The patch migrates security_sid_to_context_core() to use the cache (also a few other functions where it was possible without too much fuss, but these mostly use the translation for logging in case of error, which is rare). Link: https://bugzilla.redhat.com/show_bug.cgi?id=1733259 Cc: Michal Sekletar Signed-off-by: Ondrej Mosnacek Reviewed-by: Stephen Smalley Tested-by: Stephen Smalley Reviewed-by: Paul E. McKenney [PM: lots of merge fixups due to collisions with other sidtab patches] Signed-off-by: Paul Moore --- security/selinux/Kconfig | 11 +++ security/selinux/ss/services.c | 138 ++++++++++++++++---------- security/selinux/ss/sidtab.c | 175 +++++++++++++++++++++++++++------ security/selinux/ss/sidtab.h | 58 +++++++++-- 4 files changed, 288 insertions(+), 94 deletions(-) diff --git a/security/selinux/Kconfig b/security/selinux/Kconfig index c9e576c430c2..996d35d950f7 100644 --- a/security/selinux/Kconfig +++ b/security/selinux/Kconfig @@ -97,3 +97,14 @@ config SECURITY_SELINUX_SIDTAB_HASH_BITS collisions may be viewed at /sys/fs/selinux/ss/sidtab_hash_stats. If chain lengths are high (e.g. > 20) then selecting a higher value here will ensure that lookups times are short and stable. + +config SECURITY_SELINUX_SID2STR_CACHE_SIZE + int "NSA SELinux SID to context string translation cache size" + depends on SECURITY_SELINUX + default 256 + help + This option defines the size of the internal SID -> context string + cache, which improves the performance of context to string + conversion. Setting this option to 0 disables the cache completely. + + If unsure, keep the default value. diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c index 38fb6fdd65ca..743b85ede4ef 100644 --- a/security/selinux/ss/services.c +++ b/security/selinux/ss/services.c @@ -91,6 +91,12 @@ static int context_struct_to_string(struct policydb *policydb, char **scontext, u32 *scontext_len); +static int sidtab_entry_to_string(struct policydb *policydb, + struct sidtab *sidtab, + struct sidtab_entry *entry, + char **scontext, + u32 *scontext_len); + static void context_struct_compute_av(struct policydb *policydb, struct context *scontext, struct context *tcontext, @@ -716,20 +722,21 @@ static void context_struct_compute_av(struct policydb *policydb, } static int security_validtrans_handle_fail(struct selinux_state *state, - struct context *ocontext, - struct context *ncontext, - struct context *tcontext, + struct sidtab_entry *oentry, + struct sidtab_entry *nentry, + struct sidtab_entry *tentry, u16 tclass) { struct policydb *p = &state->ss->policydb; + struct sidtab *sidtab = state->ss->sidtab; char *o = NULL, *n = NULL, *t = NULL; u32 olen, nlen, tlen; - if (context_struct_to_string(p, ocontext, &o, &olen)) + if (sidtab_entry_to_string(p, sidtab, oentry, &o, &olen)) goto out; - if (context_struct_to_string(p, ncontext, &n, &nlen)) + if (sidtab_entry_to_string(p, sidtab, nentry, &n, &nlen)) goto out; - if (context_struct_to_string(p, tcontext, &t, &tlen)) + if (sidtab_entry_to_string(p, sidtab, tentry, &t, &tlen)) goto out; audit_log(audit_context(), GFP_ATOMIC, AUDIT_SELINUX_ERR, "op=security_validate_transition seresult=denied" @@ -751,9 +758,9 @@ static int security_compute_validatetrans(struct selinux_state *state, { struct policydb *policydb; struct sidtab *sidtab; - struct context *ocontext; - struct context *ncontext; - struct context *tcontext; + struct sidtab_entry *oentry; + struct sidtab_entry *nentry; + struct sidtab_entry *tentry; struct class_datum *tclass_datum; struct constraint_node *constraint; u16 tclass; @@ -779,24 +786,24 @@ static int security_compute_validatetrans(struct selinux_state *state, } tclass_datum = policydb->class_val_to_struct[tclass - 1]; - ocontext = sidtab_search(sidtab, oldsid); - if (!ocontext) { + oentry = sidtab_search_entry(sidtab, oldsid); + if (!oentry) { pr_err("SELinux: %s: unrecognized SID %d\n", __func__, oldsid); rc = -EINVAL; goto out; } - ncontext = sidtab_search(sidtab, newsid); - if (!ncontext) { + nentry = sidtab_search_entry(sidtab, newsid); + if (!nentry) { pr_err("SELinux: %s: unrecognized SID %d\n", __func__, newsid); rc = -EINVAL; goto out; } - tcontext = sidtab_search(sidtab, tasksid); - if (!tcontext) { + tentry = sidtab_search_entry(sidtab, tasksid); + if (!tentry) { pr_err("SELinux: %s: unrecognized SID %d\n", __func__, tasksid); rc = -EINVAL; @@ -805,15 +812,16 @@ static int security_compute_validatetrans(struct selinux_state *state, constraint = tclass_datum->validatetrans; while (constraint) { - if (!constraint_expr_eval(policydb, ocontext, ncontext, - tcontext, constraint->expr)) { + if (!constraint_expr_eval(policydb, &oentry->context, + &nentry->context, &tentry->context, + constraint->expr)) { if (user) rc = -EPERM; else rc = security_validtrans_handle_fail(state, - ocontext, - ncontext, - tcontext, + oentry, + nentry, + tentry, tclass); goto out; } @@ -855,7 +863,7 @@ int security_bounded_transition(struct selinux_state *state, { struct policydb *policydb; struct sidtab *sidtab; - struct context *old_context, *new_context; + struct sidtab_entry *old_entry, *new_entry; struct type_datum *type; int index; int rc; @@ -869,16 +877,16 @@ int security_bounded_transition(struct selinux_state *state, sidtab = state->ss->sidtab; rc = -EINVAL; - old_context = sidtab_search(sidtab, old_sid); - if (!old_context) { + old_entry = sidtab_search_entry(sidtab, old_sid); + if (!old_entry) { pr_err("SELinux: %s: unrecognized SID %u\n", __func__, old_sid); goto out; } rc = -EINVAL; - new_context = sidtab_search(sidtab, new_sid); - if (!new_context) { + new_entry = sidtab_search_entry(sidtab, new_sid); + if (!new_entry) { pr_err("SELinux: %s: unrecognized SID %u\n", __func__, new_sid); goto out; @@ -886,10 +894,10 @@ int security_bounded_transition(struct selinux_state *state, rc = 0; /* type/domain unchanged */ - if (old_context->type == new_context->type) + if (old_entry->context.type == new_entry->context.type) goto out; - index = new_context->type; + index = new_entry->context.type; while (true) { type = policydb->type_val_to_struct[index - 1]; BUG_ON(!type); @@ -901,7 +909,7 @@ int security_bounded_transition(struct selinux_state *state, /* @newsid is bounded by @oldsid */ rc = 0; - if (type->bounds == old_context->type) + if (type->bounds == old_entry->context.type) break; index = type->bounds; @@ -912,10 +920,10 @@ int security_bounded_transition(struct selinux_state *state, char *new_name = NULL; u32 length; - if (!context_struct_to_string(policydb, old_context, - &old_name, &length) && - !context_struct_to_string(policydb, new_context, - &new_name, &length)) { + if (!sidtab_entry_to_string(policydb, sidtab, old_entry, + &old_name, &length) && + !sidtab_entry_to_string(policydb, sidtab, new_entry, + &new_name, &length)) { audit_log(audit_context(), GFP_ATOMIC, AUDIT_SELINUX_ERR, "op=security_bounded_transition " @@ -1255,6 +1263,23 @@ static int context_struct_to_string(struct policydb *p, return 0; } +static int sidtab_entry_to_string(struct policydb *p, + struct sidtab *sidtab, + struct sidtab_entry *entry, + char **scontext, u32 *scontext_len) +{ + int rc = sidtab_sid2str_get(sidtab, entry, scontext, scontext_len); + + if (rc != -ENOENT) + return rc; + + rc = context_struct_to_string(p, &entry->context, scontext, + scontext_len); + if (!rc && scontext) + sidtab_sid2str_put(sidtab, entry, *scontext, *scontext_len); + return rc; +} + #include "initial_sid_to_string.h" int security_sidtab_hash_stats(struct selinux_state *state, char *page) @@ -1282,7 +1307,7 @@ static int security_sid_to_context_core(struct selinux_state *state, { struct policydb *policydb; struct sidtab *sidtab; - struct context *context; + struct sidtab_entry *entry; int rc = 0; if (scontext) @@ -1313,21 +1338,23 @@ static int security_sid_to_context_core(struct selinux_state *state, read_lock(&state->ss->policy_rwlock); policydb = &state->ss->policydb; sidtab = state->ss->sidtab; + if (force) - context = sidtab_search_force(sidtab, sid); + entry = sidtab_search_entry_force(sidtab, sid); else - context = sidtab_search(sidtab, sid); - if (!context) { + entry = sidtab_search_entry(sidtab, sid); + if (!entry) { pr_err("SELinux: %s: unrecognized SID %d\n", __func__, sid); rc = -EINVAL; goto out_unlock; } - if (only_invalid && !context->len) - rc = 0; - else - rc = context_struct_to_string(policydb, context, scontext, - scontext_len); + if (only_invalid && !entry->context.len) + goto out_unlock; + + rc = sidtab_entry_to_string(policydb, sidtab, entry, scontext, + scontext_len); + out_unlock: read_unlock(&state->ss->policy_rwlock); out: @@ -1621,19 +1648,20 @@ int security_context_to_sid_force(struct selinux_state *state, static int compute_sid_handle_invalid_context( struct selinux_state *state, - struct context *scontext, - struct context *tcontext, + struct sidtab_entry *sentry, + struct sidtab_entry *tentry, u16 tclass, struct context *newcontext) { struct policydb *policydb = &state->ss->policydb; + struct sidtab *sidtab = state->ss->sidtab; char *s = NULL, *t = NULL, *n = NULL; u32 slen, tlen, nlen; struct audit_buffer *ab; - if (context_struct_to_string(policydb, scontext, &s, &slen)) + if (sidtab_entry_to_string(policydb, sidtab, sentry, &s, &slen)) goto out; - if (context_struct_to_string(policydb, tcontext, &t, &tlen)) + if (sidtab_entry_to_string(policydb, sidtab, tentry, &t, &tlen)) goto out; if (context_struct_to_string(policydb, newcontext, &n, &nlen)) goto out; @@ -1692,7 +1720,8 @@ static int security_compute_sid(struct selinux_state *state, struct policydb *policydb; struct sidtab *sidtab; struct class_datum *cladatum = NULL; - struct context *scontext = NULL, *tcontext = NULL, newcontext; + struct context *scontext, *tcontext, newcontext; + struct sidtab_entry *sentry, *tentry; struct role_trans *roletr = NULL; struct avtab_key avkey; struct avtab_datum *avdatum; @@ -1729,21 +1758,24 @@ static int security_compute_sid(struct selinux_state *state, policydb = &state->ss->policydb; sidtab = state->ss->sidtab; - scontext = sidtab_search(sidtab, ssid); - if (!scontext) { + sentry = sidtab_search_entry(sidtab, ssid); + if (!sentry) { pr_err("SELinux: %s: unrecognized SID %d\n", __func__, ssid); rc = -EINVAL; goto out_unlock; } - tcontext = sidtab_search(sidtab, tsid); - if (!tcontext) { + tentry = sidtab_search_entry(sidtab, tsid); + if (!tentry) { pr_err("SELinux: %s: unrecognized SID %d\n", __func__, tsid); rc = -EINVAL; goto out_unlock; } + scontext = &sentry->context; + tcontext = &tentry->context; + if (tclass && tclass <= policydb->p_classes.nprim) cladatum = policydb->class_val_to_struct[tclass - 1]; @@ -1844,10 +1876,8 @@ 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, scontext, - tcontext, - tclass, - &newcontext); + rc = compute_sid_handle_invalid_context(state, sentry, tentry, + tclass, &newcontext); if (rc) goto out_unlock; } diff --git a/security/selinux/ss/sidtab.c b/security/selinux/ss/sidtab.c index d9d8599e8e63..a308ce1e6a13 100644 --- a/security/selinux/ss/sidtab.c +++ b/security/selinux/ss/sidtab.c @@ -9,6 +9,8 @@ */ #include #include +#include +#include #include #include #include @@ -17,6 +19,14 @@ #include "security.h" #include "sidtab.h" +struct sidtab_str_cache { + struct rcu_head rcu_member; + struct list_head lru_member; + struct sidtab_entry *parent; + u32 len; + char str[]; +}; + #define index_to_sid(index) (index + SECINITSID_NUM + 1) #define sid_to_index(sid) (sid - (SECINITSID_NUM + 1)) @@ -34,12 +44,19 @@ int sidtab_init(struct sidtab *s) hash_init(s->context_to_sid); spin_lock_init(&s->lock); + +#if CONFIG_SECURITY_SELINUX_SID2STR_CACHE_SIZE > 0 + s->cache_free_slots = CONFIG_SECURITY_SELINUX_SID2STR_CACHE_SIZE; + INIT_LIST_HEAD(&s->cache_lru_list); + spin_lock_init(&s->cache_lock); +#endif + return 0; } static u32 context_to_sid(struct sidtab *s, struct context *context) { - struct sidtab_entry_leaf *entry; + struct sidtab_entry *entry; u32 sid = 0; rcu_read_lock(); @@ -56,19 +73,22 @@ static u32 context_to_sid(struct sidtab *s, struct context *context) int sidtab_set_initial(struct sidtab *s, u32 sid, struct context *context) { - struct sidtab_isid_entry *entry; + struct sidtab_isid_entry *isid; int rc; if (sid == 0 || sid > SECINITSID_NUM) return -EINVAL; - entry = &s->isids[sid - 1]; + isid = &s->isids[sid - 1]; - rc = context_cpy(&entry->leaf.context, context); + rc = context_cpy(&isid->entry.context, context); if (rc) return rc; - entry->set = 1; +#if CONFIG_SECURITY_SELINUX_SID2STR_CACHE_SIZE > 0 + isid->entry.cache = NULL; +#endif + isid->set = 1; /* * Multiple initial sids may map to the same context. Check that this @@ -77,8 +97,8 @@ int sidtab_set_initial(struct sidtab *s, u32 sid, struct context *context) * collision. */ if (!context_to_sid(s, context)) { - entry->leaf.sid = sid; - hash_add(s->context_to_sid, &entry->leaf.list, context->hash); + isid->entry.sid = sid; + hash_add(s->context_to_sid, &isid->entry.list, context->hash); } return 0; @@ -92,7 +112,7 @@ int sidtab_hash_stats(struct sidtab *sidtab, char *page) int entries = 0; int max_chain_len = 0; int cur_bucket = 0; - struct sidtab_entry_leaf *entry; + struct sidtab_entry *entry; rcu_read_lock(); hash_for_each_rcu(sidtab->context_to_sid, i, entry, list) { @@ -151,8 +171,8 @@ static int sidtab_alloc_roots(struct sidtab *s, u32 level) return 0; } -static struct sidtab_entry_leaf *sidtab_do_lookup(struct sidtab *s, u32 index, - int alloc) +static struct sidtab_entry *sidtab_do_lookup(struct sidtab *s, u32 index, + int alloc) { union sidtab_entry_inner *entry; u32 level, capacity_shift, leaf_index = index / SIDTAB_LEAF_ENTRIES; @@ -192,7 +212,7 @@ static struct sidtab_entry_leaf *sidtab_do_lookup(struct sidtab *s, u32 index, return &entry->ptr_leaf->entries[index % SIDTAB_LEAF_ENTRIES]; } -static struct context *sidtab_lookup(struct sidtab *s, u32 index) +static struct sidtab_entry *sidtab_lookup(struct sidtab *s, u32 index) { /* read entries only after reading count */ u32 count = smp_load_acquire(&s->count); @@ -200,36 +220,37 @@ static struct context *sidtab_lookup(struct sidtab *s, u32 index) if (index >= count) return NULL; - return &sidtab_do_lookup(s, index, 0)->context; + return sidtab_do_lookup(s, index, 0); } -static struct context *sidtab_lookup_initial(struct sidtab *s, u32 sid) +static struct sidtab_entry *sidtab_lookup_initial(struct sidtab *s, u32 sid) { - return s->isids[sid - 1].set ? &s->isids[sid - 1].leaf.context : NULL; + return s->isids[sid - 1].set ? &s->isids[sid - 1].entry : NULL; } -static struct context *sidtab_search_core(struct sidtab *s, u32 sid, int force) +static struct sidtab_entry *sidtab_search_core(struct sidtab *s, u32 sid, + int force) { - struct context *context; - if (sid != 0) { + struct sidtab_entry *entry; + if (sid > SECINITSID_NUM) - context = sidtab_lookup(s, sid_to_index(sid)); + entry = sidtab_lookup(s, sid_to_index(sid)); else - context = sidtab_lookup_initial(s, sid); - if (context && (!context->len || force)) - return context; + entry = sidtab_lookup_initial(s, sid); + if (entry && (!entry->context.len || force)) + return entry; } return sidtab_lookup_initial(s, SECINITSID_UNLABELED); } -struct context *sidtab_search(struct sidtab *s, u32 sid) +struct sidtab_entry *sidtab_search_entry(struct sidtab *s, u32 sid) { return sidtab_search_core(s, sid, 0); } -struct context *sidtab_search_force(struct sidtab *s, u32 sid) +struct sidtab_entry *sidtab_search_entry_force(struct sidtab *s, u32 sid) { return sidtab_search_core(s, sid, 1); } @@ -240,7 +261,7 @@ int sidtab_context_to_sid(struct sidtab *s, struct context *context, unsigned long flags; u32 count; struct sidtab_convert_params *convert; - struct sidtab_entry_leaf *dst, *dst_convert; + struct sidtab_entry *dst, *dst_convert; int rc; *sid = context_to_sid(s, context); @@ -289,7 +310,7 @@ int sidtab_context_to_sid(struct sidtab *s, struct context *context, } rc = convert->func(context, &dst_convert->context, - convert->args); + convert->args); if (rc) { context_destroy(&dst->context); goto out_unlock; @@ -298,7 +319,7 @@ int sidtab_context_to_sid(struct sidtab *s, struct context *context, convert->target->count = count + 1; hash_add_rcu(convert->target->context_to_sid, - &dst_convert->list, dst_convert->context.hash); + &dst_convert->list, dst_convert->context.hash); } if (context->len) @@ -319,7 +340,7 @@ out_unlock: static void sidtab_convert_hashtable(struct sidtab *s, u32 count) { - struct sidtab_entry_leaf *entry; + struct sidtab_entry *entry; u32 i; for (i = 0; i < count; i++) { @@ -327,7 +348,7 @@ static void sidtab_convert_hashtable(struct sidtab *s, u32 count) entry->sid = index_to_sid(i); hash_add_rcu(s->context_to_sid, &entry->list, - entry->context.hash); + entry->context.hash); } } @@ -376,7 +397,6 @@ static int sidtab_convert_tree(union sidtab_entry_inner *edst, } cond_resched(); } - return 0; } @@ -439,6 +459,14 @@ int sidtab_convert(struct sidtab *s, struct sidtab_convert_params *params) return 0; } +static void sidtab_destroy_entry(struct sidtab_entry *entry) +{ + context_destroy(&entry->context); +#if CONFIG_SECURITY_SELINUX_SID2STR_CACHE_SIZE > 0 + kfree(rcu_dereference_raw(entry->cache)); +#endif +} + static void sidtab_destroy_tree(union sidtab_entry_inner entry, u32 level) { u32 i; @@ -459,7 +487,7 @@ static void sidtab_destroy_tree(union sidtab_entry_inner entry, u32 level) return; for (i = 0; i < SIDTAB_LEAF_ENTRIES; i++) - context_destroy(&node->entries[i].context); + sidtab_destroy_entry(&node->entries[i]); kfree(node); } } @@ -470,7 +498,7 @@ void sidtab_destroy(struct sidtab *s) for (i = 0; i < SECINITSID_NUM; i++) if (s->isids[i].set) - context_destroy(&s->isids[i].leaf.context); + sidtab_destroy_entry(&s->isids[i].entry); level = SIDTAB_MAX_LEVEL; while (level && !s->roots[level].ptr_inner) @@ -483,3 +511,88 @@ void sidtab_destroy(struct sidtab *s) * to be cleaned up here. */ } + +#if CONFIG_SECURITY_SELINUX_SID2STR_CACHE_SIZE > 0 + +void sidtab_sid2str_put(struct sidtab *s, struct sidtab_entry *entry, + const char *str, u32 str_len) +{ + struct sidtab_str_cache *cache, *victim = NULL; + + /* do not cache invalid contexts */ + if (entry->context.len) + return; + + /* + * Skip the put operation when in non-task context to avoid the need + * to disable interrupts while holding s->cache_lock. + */ + if (!in_task()) + return; + + spin_lock(&s->cache_lock); + + cache = rcu_dereference_protected(entry->cache, + lockdep_is_held(&s->cache_lock)); + if (cache) { + /* entry in cache - just bump to the head of LRU list */ + list_move(&cache->lru_member, &s->cache_lru_list); + goto out_unlock; + } + + cache = kmalloc(sizeof(struct sidtab_str_cache) + str_len, GFP_ATOMIC); + if (!cache) + goto out_unlock; + + if (s->cache_free_slots == 0) { + /* pop a cache entry from the tail and free it */ + victim = container_of(s->cache_lru_list.prev, + struct sidtab_str_cache, lru_member); + list_del(&victim->lru_member); + rcu_assign_pointer(victim->parent->cache, NULL); + } else { + s->cache_free_slots--; + } + cache->parent = entry; + cache->len = str_len; + memcpy(cache->str, str, str_len); + list_add(&cache->lru_member, &s->cache_lru_list); + + rcu_assign_pointer(entry->cache, cache); + +out_unlock: + spin_unlock(&s->cache_lock); + kfree_rcu(victim, rcu_member); +} + +int sidtab_sid2str_get(struct sidtab *s, struct sidtab_entry *entry, + char **out, u32 *out_len) +{ + struct sidtab_str_cache *cache; + int rc = 0; + + if (entry->context.len) + return -ENOENT; /* do not cache invalid contexts */ + + rcu_read_lock(); + + cache = rcu_dereference(entry->cache); + if (!cache) { + rc = -ENOENT; + } else { + *out_len = cache->len; + if (out) { + *out = kmemdup(cache->str, cache->len, GFP_ATOMIC); + if (!*out) + rc = -ENOMEM; + } + } + + rcu_read_unlock(); + + if (!rc && out) + sidtab_sid2str_put(s, entry, *out, *out_len); + return rc; +} + +#endif /* CONFIG_SECURITY_SELINUX_SID2STR_CACHE_SIZE > 0 */ diff --git a/security/selinux/ss/sidtab.h b/security/selinux/ss/sidtab.h index e2809401c417..3311d9f236c0 100644 --- a/security/selinux/ss/sidtab.h +++ b/security/selinux/ss/sidtab.h @@ -17,15 +17,15 @@ #include "context.h" -struct sidtab_entry_leaf { +struct sidtab_entry { u32 sid; struct context context; +#if CONFIG_SECURITY_SELINUX_SID2STR_CACHE_SIZE > 0 + struct sidtab_str_cache __rcu *cache; +#endif struct hlist_node list; }; -struct sidtab_node_inner; -struct sidtab_node_leaf; - union sidtab_entry_inner { struct sidtab_node_inner *ptr_inner; struct sidtab_node_leaf *ptr_leaf; @@ -41,7 +41,7 @@ union sidtab_entry_inner { (SIDTAB_NODE_ALLOC_SHIFT - size_to_shift(sizeof(union sidtab_entry_inner))) #define SIDTAB_INNER_ENTRIES ((size_t)1 << SIDTAB_INNER_SHIFT) #define SIDTAB_LEAF_ENTRIES \ - (SIDTAB_NODE_ALLOC_SIZE / sizeof(struct sidtab_entry_leaf)) + (SIDTAB_NODE_ALLOC_SIZE / sizeof(struct sidtab_entry)) #define SIDTAB_MAX_BITS 32 #define SIDTAB_MAX U32_MAX @@ -51,7 +51,7 @@ union sidtab_entry_inner { SIDTAB_INNER_SHIFT) struct sidtab_node_leaf { - struct sidtab_entry_leaf entries[SIDTAB_LEAF_ENTRIES]; + struct sidtab_entry entries[SIDTAB_LEAF_ENTRIES]; }; struct sidtab_node_inner { @@ -60,7 +60,7 @@ struct sidtab_node_inner { struct sidtab_isid_entry { int set; - struct sidtab_entry_leaf leaf; + struct sidtab_entry entry; }; struct sidtab_convert_params { @@ -87,6 +87,13 @@ struct sidtab { struct sidtab_convert_params *convert; spinlock_t lock; +#if CONFIG_SECURITY_SELINUX_SID2STR_CACHE_SIZE > 0 + /* SID -> context string cache */ + u32 cache_free_slots; + struct list_head cache_lru_list; + spinlock_t cache_lock; +#endif + /* index == SID - 1 (no entry for SECSID_NULL) */ struct sidtab_isid_entry isids[SECINITSID_NUM]; @@ -96,8 +103,22 @@ struct sidtab { int sidtab_init(struct sidtab *s); int sidtab_set_initial(struct sidtab *s, u32 sid, struct context *context); -struct context *sidtab_search(struct sidtab *s, u32 sid); -struct context *sidtab_search_force(struct sidtab *s, u32 sid); +struct sidtab_entry *sidtab_search_entry(struct sidtab *s, u32 sid); +struct sidtab_entry *sidtab_search_entry_force(struct sidtab *s, u32 sid); + +static inline struct context *sidtab_search(struct sidtab *s, u32 sid) +{ + struct sidtab_entry *entry = sidtab_search_entry(s, sid); + + return entry ? &entry->context : NULL; +} + +static inline struct context *sidtab_search_force(struct sidtab *s, u32 sid) +{ + struct sidtab_entry *entry = sidtab_search_entry_force(s, sid); + + return entry ? &entry->context : NULL; +} int sidtab_convert(struct sidtab *s, struct sidtab_convert_params *params); @@ -107,6 +128,25 @@ void sidtab_destroy(struct sidtab *s); int sidtab_hash_stats(struct sidtab *sidtab, char *page); +#if CONFIG_SECURITY_SELINUX_SID2STR_CACHE_SIZE > 0 +void sidtab_sid2str_put(struct sidtab *s, struct sidtab_entry *entry, + const char *str, u32 str_len); +int sidtab_sid2str_get(struct sidtab *s, struct sidtab_entry *entry, + char **out, u32 *out_len); +#else +static inline void sidtab_sid2str_put(struct sidtab *s, + struct sidtab_entry *entry, + const char *str, u32 str_len) +{ +} +static inline int sidtab_sid2str_get(struct sidtab *s, + struct sidtab_entry *entry, + char **out, u32 *out_len) +{ + return -ENOENT; +} +#endif /* CONFIG_SECURITY_SELINUX_SID2STR_CACHE_SIZE > 0 */ + #endif /* _SS_SIDTAB_H_ */ From 59438b46471ae6cdfb761afc8c9beaf1e428a331 Mon Sep 17 00:00:00 2001 From: Stephen Smalley Date: Wed, 27 Nov 2019 12:04:36 -0500 Subject: [PATCH 03/28] security,lockdown,selinux: implement SELinux lockdown Implement a SELinux hook for lockdown. If the lockdown module is also enabled, then a denial by the lockdown module will take precedence over SELinux, so SELinux can only further restrict lockdown decisions. The SELinux hook only distinguishes at the granularity of integrity versus confidentiality similar to the lockdown module, but includes the full lockdown reason as part of the audit record as a hint in diagnosing what triggered the denial. To support this auditing, move the lockdown_reasons[] string array from being private to the lockdown module to the security framework so that it can be used by the lsm audit code and so that it is always available even when the lockdown module is disabled. Note that the SELinux implementation allows the integrity and confidentiality reasons to be controlled independently from one another. Thus, in an SELinux policy, one could allow operations that specify an integrity reason while blocking operations that specify a confidentiality reason. The SELinux hook implementation is stricter than the lockdown module in validating the provided reason value. Sample AVC audit output from denials: avc: denied { integrity } for pid=3402 comm="fwupd" lockdown_reason="/dev/mem,kmem,port" scontext=system_u:system_r:fwupd_t:s0 tcontext=system_u:system_r:fwupd_t:s0 tclass=lockdown permissive=0 avc: denied { confidentiality } for pid=4628 comm="cp" lockdown_reason="/proc/kcore access" scontext=unconfined_u:unconfined_r:test_lockdown_integrity_t:s0-s0:c0.c1023 tcontext=unconfined_u:unconfined_r:test_lockdown_integrity_t:s0-s0:c0.c1023 tclass=lockdown permissive=0 Signed-off-by: Stephen Smalley Reviewed-by: James Morris [PM: some merge fuzz do the the perf hooks] Signed-off-by: Paul Moore --- include/linux/lsm_audit.h | 2 ++ include/linux/security.h | 2 ++ security/lockdown/lockdown.c | 27 ----------------------- security/lsm_audit.c | 5 +++++ security/security.c | 33 +++++++++++++++++++++++++++++ security/selinux/hooks.c | 30 ++++++++++++++++++++++++++ security/selinux/include/classmap.h | 2 ++ 7 files changed, 74 insertions(+), 27 deletions(-) diff --git a/include/linux/lsm_audit.h b/include/linux/lsm_audit.h index 915330abf6e5..99d629fd9944 100644 --- a/include/linux/lsm_audit.h +++ b/include/linux/lsm_audit.h @@ -74,6 +74,7 @@ struct common_audit_data { #define LSM_AUDIT_DATA_FILE 12 #define LSM_AUDIT_DATA_IBPKEY 13 #define LSM_AUDIT_DATA_IBENDPORT 14 +#define LSM_AUDIT_DATA_LOCKDOWN 15 union { struct path path; struct dentry *dentry; @@ -93,6 +94,7 @@ struct common_audit_data { struct file *file; struct lsm_ibpkey_audit *ibpkey; struct lsm_ibendport_audit *ibendport; + int reason; } u; /* this union contains LSM specific data */ union { diff --git a/include/linux/security.h b/include/linux/security.h index 3e8d4bacd59d..64b19f050343 100644 --- a/include/linux/security.h +++ b/include/linux/security.h @@ -128,6 +128,8 @@ enum lockdown_reason { LOCKDOWN_CONFIDENTIALITY_MAX, }; +extern const char *const lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1]; + /* These functions are in security/commoncap.c */ extern int cap_capable(const struct cred *cred, struct user_namespace *ns, int cap, unsigned int opts); diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c index b2f87015d6e9..5a952617a0eb 100644 --- a/security/lockdown/lockdown.c +++ b/security/lockdown/lockdown.c @@ -16,33 +16,6 @@ static enum lockdown_reason kernel_locked_down; -static const char *const lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1] = { - [LOCKDOWN_NONE] = "none", - [LOCKDOWN_MODULE_SIGNATURE] = "unsigned module loading", - [LOCKDOWN_DEV_MEM] = "/dev/mem,kmem,port", - [LOCKDOWN_EFI_TEST] = "/dev/efi_test access", - [LOCKDOWN_KEXEC] = "kexec of unsigned images", - [LOCKDOWN_HIBERNATION] = "hibernation", - [LOCKDOWN_PCI_ACCESS] = "direct PCI access", - [LOCKDOWN_IOPORT] = "raw io port access", - [LOCKDOWN_MSR] = "raw MSR access", - [LOCKDOWN_ACPI_TABLES] = "modifying ACPI tables", - [LOCKDOWN_PCMCIA_CIS] = "direct PCMCIA CIS storage", - [LOCKDOWN_TIOCSSERIAL] = "reconfiguration of serial port IO", - [LOCKDOWN_MODULE_PARAMETERS] = "unsafe module parameters", - [LOCKDOWN_MMIOTRACE] = "unsafe mmio", - [LOCKDOWN_DEBUGFS] = "debugfs access", - [LOCKDOWN_XMON_WR] = "xmon write access", - [LOCKDOWN_INTEGRITY_MAX] = "integrity", - [LOCKDOWN_KCORE] = "/proc/kcore access", - [LOCKDOWN_KPROBES] = "use of kprobes", - [LOCKDOWN_BPF_READ] = "use of bpf to read kernel RAM", - [LOCKDOWN_PERF] = "unsafe use of perf", - [LOCKDOWN_TRACEFS] = "use of tracefs", - [LOCKDOWN_XMON_RW] = "xmon read and write access", - [LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality", -}; - static const enum lockdown_reason lockdown_levels[] = {LOCKDOWN_NONE, LOCKDOWN_INTEGRITY_MAX, LOCKDOWN_CONFIDENTIALITY_MAX}; diff --git a/security/lsm_audit.c b/security/lsm_audit.c index e40874373f2b..2d2bf49016f4 100644 --- a/security/lsm_audit.c +++ b/security/lsm_audit.c @@ -27,6 +27,7 @@ #include #include #include +#include /** * ipv4_skb_to_auditdata : fill auditdata from skb @@ -425,6 +426,10 @@ static void dump_common_audit_data(struct audit_buffer *ab, a->u.ibendport->dev_name, a->u.ibendport->port); break; + case LSM_AUDIT_DATA_LOCKDOWN: + audit_log_format(ab, " lockdown_reason="); + audit_log_string(ab, lockdown_reasons[a->u.reason]); + break; } /* switch (a->type) */ } diff --git a/security/security.c b/security/security.c index cd2d18d2d279..2b5473d92416 100644 --- a/security/security.c +++ b/security/security.c @@ -35,6 +35,39 @@ #define LSM_COUNT (__end_lsm_info - __start_lsm_info) #define EARLY_LSM_COUNT (__end_early_lsm_info - __start_early_lsm_info) +/* + * These are descriptions of the reasons that can be passed to the + * security_locked_down() LSM hook. Placing this array here allows + * all security modules to use the same descriptions for auditing + * purposes. + */ +const char *const lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1] = { + [LOCKDOWN_NONE] = "none", + [LOCKDOWN_MODULE_SIGNATURE] = "unsigned module loading", + [LOCKDOWN_DEV_MEM] = "/dev/mem,kmem,port", + [LOCKDOWN_EFI_TEST] = "/dev/efi_test access", + [LOCKDOWN_KEXEC] = "kexec of unsigned images", + [LOCKDOWN_HIBERNATION] = "hibernation", + [LOCKDOWN_PCI_ACCESS] = "direct PCI access", + [LOCKDOWN_IOPORT] = "raw io port access", + [LOCKDOWN_MSR] = "raw MSR access", + [LOCKDOWN_ACPI_TABLES] = "modifying ACPI tables", + [LOCKDOWN_PCMCIA_CIS] = "direct PCMCIA CIS storage", + [LOCKDOWN_TIOCSSERIAL] = "reconfiguration of serial port IO", + [LOCKDOWN_MODULE_PARAMETERS] = "unsafe module parameters", + [LOCKDOWN_MMIOTRACE] = "unsafe mmio", + [LOCKDOWN_DEBUGFS] = "debugfs access", + [LOCKDOWN_XMON_WR] = "xmon write access", + [LOCKDOWN_INTEGRITY_MAX] = "integrity", + [LOCKDOWN_KCORE] = "/proc/kcore access", + [LOCKDOWN_KPROBES] = "use of kprobes", + [LOCKDOWN_BPF_READ] = "use of bpf to read kernel RAM", + [LOCKDOWN_PERF] = "unsafe use of perf", + [LOCKDOWN_TRACEFS] = "use of tracefs", + [LOCKDOWN_XMON_RW] = "xmon read and write access", + [LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality", +}; + struct security_hook_heads security_hook_heads __lsm_ro_after_init; static BLOCKING_NOTIFIER_HEAD(blocking_lsm_notifier_chain); diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 116b4d644f68..9e1c4780dc20 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -6795,6 +6795,34 @@ static void selinux_bpf_prog_free(struct bpf_prog_aux *aux) } #endif +static int selinux_lockdown(enum lockdown_reason what) +{ + struct common_audit_data ad; + u32 sid = current_sid(); + int invalid_reason = (what <= LOCKDOWN_NONE) || + (what == LOCKDOWN_INTEGRITY_MAX) || + (what >= LOCKDOWN_CONFIDENTIALITY_MAX); + + if (WARN(invalid_reason, "Invalid lockdown reason")) { + audit_log(audit_context(), + GFP_ATOMIC, AUDIT_SELINUX_ERR, + "lockdown_reason=invalid"); + return -EINVAL; + } + + ad.type = LSM_AUDIT_DATA_LOCKDOWN; + ad.u.reason = what; + + if (what <= LOCKDOWN_INTEGRITY_MAX) + return avc_has_perm(&selinux_state, + sid, sid, SECCLASS_LOCKDOWN, + LOCKDOWN__INTEGRITY, &ad); + else + return avc_has_perm(&selinux_state, + sid, sid, SECCLASS_LOCKDOWN, + LOCKDOWN__CONFIDENTIALITY, &ad); +} + struct lsm_blob_sizes selinux_blob_sizes __lsm_ro_after_init = { .lbs_cred = sizeof(struct task_security_struct), .lbs_file = sizeof(struct file_security_struct), @@ -7107,6 +7135,8 @@ static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = { LSM_HOOK_INIT(perf_event_read, selinux_perf_event_read), LSM_HOOK_INIT(perf_event_write, selinux_perf_event_write), #endif + + LSM_HOOK_INIT(locked_down, selinux_lockdown), }; static __init int selinux_init(void) diff --git a/security/selinux/include/classmap.h b/security/selinux/include/classmap.h index 7db24855e12d..986f3ac14282 100644 --- a/security/selinux/include/classmap.h +++ b/security/selinux/include/classmap.h @@ -246,6 +246,8 @@ struct security_class_mapping secclass_map[] = { { COMMON_SOCK_PERMS, NULL } }, { "perf_event", {"open", "cpu", "kernel", "tracepoint", "read", "write"} }, + { "lockdown", + { "integrity", "confidentiality", NULL } }, { NULL } }; From 1a37079c236d55fb31ebbf4b59945dab8ec8764c Mon Sep 17 00:00:00 2001 From: Stephen Smalley Date: Fri, 22 Nov 2019 12:22:44 -0500 Subject: [PATCH 04/28] selinux: revert "stop passing MAY_NOT_BLOCK to the AVC upon follow_link" This reverts commit e46e01eebbbc ("selinux: stop passing MAY_NOT_BLOCK to the AVC upon follow_link"). The correct fix is to instead fall back to ref-walk if audit is required irrespective of the specific audit data type. This is done in the next commit. Fixes: e46e01eebbbc ("selinux: stop passing MAY_NOT_BLOCK to the AVC upon follow_link") Reported-by: Will Deacon Signed-off-by: Stephen Smalley Signed-off-by: Paul Moore --- security/selinux/avc.c | 24 ++++++++++++++++++++++-- security/selinux/hooks.c | 5 +++-- security/selinux/include/avc.h | 5 +++++ 3 files changed, 30 insertions(+), 4 deletions(-) diff --git a/security/selinux/avc.c b/security/selinux/avc.c index ecd3829996aa..74c43ebe34bb 100644 --- a/security/selinux/avc.c +++ b/security/selinux/avc.c @@ -862,8 +862,9 @@ static int avc_update_node(struct selinux_avc *avc, * permissive mode that only appear when in enforcing mode. * * See the corresponding handling in slow_avc_audit(), and the - * logic in selinux_inode_permission for the MAY_NOT_BLOCK flag, - * which is transliterated into AVC_NONBLOCKING. + * logic in selinux_inode_follow_link and selinux_inode_permission + * for the VFS MAY_NOT_BLOCK flag, which is transliterated into + * AVC_NONBLOCKING for avc_has_perm_noaudit(). */ if (flags & AVC_NONBLOCKING) return 0; @@ -1205,6 +1206,25 @@ int avc_has_perm(struct selinux_state *state, u32 ssid, u32 tsid, u16 tclass, return rc; } +int avc_has_perm_flags(struct selinux_state *state, + u32 ssid, u32 tsid, u16 tclass, u32 requested, + struct common_audit_data *auditdata, + int flags) +{ + struct av_decision avd; + int rc, rc2; + + rc = avc_has_perm_noaudit(state, ssid, tsid, tclass, requested, + (flags & MAY_NOT_BLOCK) ? AVC_NONBLOCKING : 0, + &avd); + + rc2 = avc_audit(state, ssid, tsid, tclass, requested, &avd, rc, + auditdata, flags); + if (rc2) + return rc2; + return rc; +} + u32 avc_policy_seqno(struct selinux_state *state) { return state->avc->avc_cache.latest_notif; diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 9e1c4780dc20..ed64cb4cd4c5 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -3004,8 +3004,9 @@ static int selinux_inode_follow_link(struct dentry *dentry, struct inode *inode, if (IS_ERR(isec)) return PTR_ERR(isec); - return avc_has_perm(&selinux_state, - sid, isec->sid, isec->sclass, FILE__READ, &ad); + return avc_has_perm_flags(&selinux_state, + sid, isec->sid, isec->sclass, FILE__READ, &ad, + rcu ? MAY_NOT_BLOCK : 0); } static noinline int audit_inode_permission(struct inode *inode, diff --git a/security/selinux/include/avc.h b/security/selinux/include/avc.h index 7be0e1e90e8b..74ea50977c20 100644 --- a/security/selinux/include/avc.h +++ b/security/selinux/include/avc.h @@ -153,6 +153,11 @@ int avc_has_perm(struct selinux_state *state, u32 ssid, u32 tsid, u16 tclass, u32 requested, struct common_audit_data *auditdata); +int avc_has_perm_flags(struct selinux_state *state, + u32 ssid, u32 tsid, + u16 tclass, u32 requested, + struct common_audit_data *auditdata, + int flags); int avc_has_extended_perms(struct selinux_state *state, u32 ssid, u32 tsid, u16 tclass, u32 requested, From 0188d5c025ca8fe756ba3193bd7d150139af5a88 Mon Sep 17 00:00:00 2001 From: Stephen Smalley Date: Fri, 22 Nov 2019 12:22:45 -0500 Subject: [PATCH 05/28] selinux: fall back to ref-walk if audit is required commit bda0be7ad994 ("security: make inode_follow_link RCU-walk aware") passed down the rcu flag to the SELinux AVC, but failed to adjust the test in slow_avc_audit() to also return -ECHILD on LSM_AUDIT_DATA_DENTRY. Previously, we only returned -ECHILD if generating an audit record with LSM_AUDIT_DATA_INODE since this was only relevant from inode_permission. Move the handling of MAY_NOT_BLOCK to avc_audit() and its inlined equivalent in selinux_inode_permission() immediately after we determine that audit is required, and always fall back to ref-walk in this case. Fixes: bda0be7ad994 ("security: make inode_follow_link RCU-walk aware") Reported-by: Will Deacon Suggested-by: Al Viro Signed-off-by: Stephen Smalley Signed-off-by: Paul Moore --- security/selinux/avc.c | 24 +++++------------------- security/selinux/hooks.c | 11 +++++++---- security/selinux/include/avc.h | 8 +++++--- 3 files changed, 17 insertions(+), 26 deletions(-) diff --git a/security/selinux/avc.c b/security/selinux/avc.c index 74c43ebe34bb..23dc888ae305 100644 --- a/security/selinux/avc.c +++ b/security/selinux/avc.c @@ -424,7 +424,7 @@ static inline int avc_xperms_audit(struct selinux_state *state, if (likely(!audited)) return 0; return slow_avc_audit(state, ssid, tsid, tclass, requested, - audited, denied, result, ad, 0); + audited, denied, result, ad); } static void avc_node_free(struct rcu_head *rhead) @@ -758,8 +758,7 @@ static void avc_audit_post_callback(struct audit_buffer *ab, void *a) noinline int slow_avc_audit(struct selinux_state *state, u32 ssid, u32 tsid, u16 tclass, u32 requested, u32 audited, u32 denied, int result, - struct common_audit_data *a, - unsigned int flags) + struct common_audit_data *a) { struct common_audit_data stack_data; struct selinux_audit_data sad; @@ -772,17 +771,6 @@ noinline int slow_avc_audit(struct selinux_state *state, a->type = LSM_AUDIT_DATA_NONE; } - /* - * When in a RCU walk do the audit on the RCU retry. This is because - * the collection of the dname in an inode audit message is not RCU - * safe. Note this may drop some audits when the situation changes - * during retry. However this is logically just as if the operation - * happened a little later. - */ - if ((a->type == LSM_AUDIT_DATA_INODE) && - (flags & MAY_NOT_BLOCK)) - return -ECHILD; - sad.tclass = tclass; sad.requested = requested; sad.ssid = ssid; @@ -855,16 +843,14 @@ static int avc_update_node(struct selinux_avc *avc, /* * If we are in a non-blocking code path, e.g. VFS RCU walk, * then we must not add permissions to a cache entry - * because we cannot safely audit the denial. Otherwise, + * because we will not audit the denial. Otherwise, * during the subsequent blocking retry (e.g. VFS ref walk), we * will find the permissions already granted in the cache entry * and won't audit anything at all, leading to silent denials in * permissive mode that only appear when in enforcing mode. * - * See the corresponding handling in slow_avc_audit(), and the - * logic in selinux_inode_follow_link and selinux_inode_permission - * for the VFS MAY_NOT_BLOCK flag, which is transliterated into - * AVC_NONBLOCKING for avc_has_perm_noaudit(). + * See the corresponding handling of MAY_NOT_BLOCK in avc_audit() + * and selinux_inode_permission(). */ if (flags & AVC_NONBLOCKING) return 0; diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index ed64cb4cd4c5..328d455ec293 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -3011,8 +3011,7 @@ static int selinux_inode_follow_link(struct dentry *dentry, struct inode *inode, static noinline int audit_inode_permission(struct inode *inode, u32 perms, u32 audited, u32 denied, - int result, - unsigned flags) + int result) { struct common_audit_data ad; struct inode_security_struct *isec = selinux_inode(inode); @@ -3023,7 +3022,7 @@ static noinline int audit_inode_permission(struct inode *inode, rc = slow_avc_audit(&selinux_state, current_sid(), isec->sid, isec->sclass, perms, - audited, denied, result, &ad, flags); + audited, denied, result, &ad); if (rc) return rc; return 0; @@ -3070,7 +3069,11 @@ static int selinux_inode_permission(struct inode *inode, int mask) if (likely(!audited)) return rc; - rc2 = audit_inode_permission(inode, perms, audited, denied, rc, flags); + /* fall back to ref-walk if we have to generate audit */ + if (flags & MAY_NOT_BLOCK) + return -ECHILD; + + rc2 = audit_inode_permission(inode, perms, audited, denied, rc); if (rc2) return rc2; return rc; diff --git a/security/selinux/include/avc.h b/security/selinux/include/avc.h index 74ea50977c20..cf4cc3ef959b 100644 --- a/security/selinux/include/avc.h +++ b/security/selinux/include/avc.h @@ -100,8 +100,7 @@ static inline u32 avc_audit_required(u32 requested, int slow_avc_audit(struct selinux_state *state, u32 ssid, u32 tsid, u16 tclass, u32 requested, u32 audited, u32 denied, int result, - struct common_audit_data *a, - unsigned flags); + struct common_audit_data *a); /** * avc_audit - Audit the granting or denial of permissions. @@ -135,9 +134,12 @@ static inline int avc_audit(struct selinux_state *state, audited = avc_audit_required(requested, avd, result, 0, &denied); if (likely(!audited)) return 0; + /* fall back to ref-walk if we have to generate audit */ + if (flags & MAY_NOT_BLOCK) + return -ECHILD; return slow_avc_audit(state, ssid, tsid, tclass, requested, audited, denied, result, - a, flags); + a); } #define AVC_STRICT 1 /* Ignore permissive mode. */ From 5298d0b9b98089f5af406f7e05a41a53f9a15c11 Mon Sep 17 00:00:00 2001 From: Stephen Smalley Date: Fri, 22 Nov 2019 16:16:56 -0500 Subject: [PATCH 06/28] selinux: clean up selinux_inode_permission MAY_NOT_BLOCK tests Through a somewhat convoluted series of changes, we have ended up with multiple unnecessary occurrences of (flags & MAY_NOT_BLOCK) tests in selinux_inode_permission(). Clean it up and simplify. No functional change. Signed-off-by: Stephen Smalley Signed-off-by: Paul Moore --- security/selinux/hooks.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 328d455ec293..47626342b6e5 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -3033,7 +3033,7 @@ static int selinux_inode_permission(struct inode *inode, int mask) const struct cred *cred = current_cred(); u32 perms; bool from_access; - unsigned flags = mask & MAY_NOT_BLOCK; + bool no_block = mask & MAY_NOT_BLOCK; struct inode_security_struct *isec; u32 sid; struct av_decision avd; @@ -3055,13 +3055,13 @@ static int selinux_inode_permission(struct inode *inode, int mask) perms = file_mask_to_av(inode->i_mode, mask); sid = cred_sid(cred); - isec = inode_security_rcu(inode, flags & MAY_NOT_BLOCK); + isec = inode_security_rcu(inode, no_block); if (IS_ERR(isec)) return PTR_ERR(isec); rc = avc_has_perm_noaudit(&selinux_state, sid, isec->sid, isec->sclass, perms, - (flags & MAY_NOT_BLOCK) ? AVC_NONBLOCKING : 0, + no_block ? AVC_NONBLOCKING : 0, &avd); audited = avc_audit_required(perms, &avd, rc, from_access ? FILE__AUDIT_ACCESS : 0, @@ -3070,7 +3070,7 @@ static int selinux_inode_permission(struct inode *inode, int mask) return rc; /* fall back to ref-walk if we have to generate audit */ - if (flags & MAY_NOT_BLOCK) + if (no_block) return -ECHILD; rc2 = audit_inode_permission(inode, perms, audited, denied, rc); From b2104ac0bd951a2887a03b09e2106fcff5fad94e Mon Sep 17 00:00:00 2001 From: Stephen Smalley Date: Tue, 10 Dec 2019 11:55:41 -0500 Subject: [PATCH 07/28] security: only build lsm_audit if CONFIG_SECURITY=y The lsm_audit code is only required when CONFIG_SECURITY is enabled. It does not have a build dependency on CONFIG_AUDIT since audit.h provides trivial static inlines for audit_log*() when CONFIG_AUDIT is disabled. Hence, the Makefile should only add lsm_audit to the obj lists based on CONFIG_SECURITY, not CONFIG_AUDIT. Fixes: 59438b46471a ("security,lockdown,selinux: implement SELinux lockdown") Signed-off-by: Stephen Smalley Signed-off-by: Paul Moore --- security/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/security/Makefile b/security/Makefile index be1dd9d2cb2f..746438499029 100644 --- a/security/Makefile +++ b/security/Makefile @@ -22,7 +22,7 @@ obj-$(CONFIG_SECURITY) += security.o obj-$(CONFIG_SECURITYFS) += inode.o obj-$(CONFIG_SECURITY_SELINUX) += selinux/ obj-$(CONFIG_SECURITY_SMACK) += smack/ -obj-$(CONFIG_AUDIT) += lsm_audit.o +obj-$(CONFIG_SECURITY) += lsm_audit.o obj-$(CONFIG_SECURITY_TOMOYO) += tomoyo/ obj-$(CONFIG_SECURITY_APPARMOR) += apparmor/ obj-$(CONFIG_SECURITY_YAMA) += yama/ From d8db60cb23e49a92cf8cada3297395c7fa50fdf8 Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Mon, 9 Dec 2019 20:39:46 -0500 Subject: [PATCH 08/28] selinux: ensure we cleanup the internal AVC counters on error in avc_insert() Fix avc_insert() to call avc_node_kill() if we've already allocated an AVC node and the code fails to insert the node in the cache. Fixes: fa1aa143ac4a ("selinux: extended permissions for ioctls") Reported-by: rsiddoji@codeaurora.org Suggested-by: Stephen Smalley Acked-by: Stephen Smalley Signed-off-by: Paul Moore --- security/selinux/avc.c | 53 ++++++++++++++++++++---------------------- 1 file changed, 25 insertions(+), 28 deletions(-) diff --git a/security/selinux/avc.c b/security/selinux/avc.c index 23dc888ae305..6646300f7ccb 100644 --- a/security/selinux/avc.c +++ b/security/selinux/avc.c @@ -617,40 +617,37 @@ static struct avc_node *avc_insert(struct selinux_avc *avc, struct avc_node *pos, *node = NULL; int hvalue; unsigned long flag; + spinlock_t *lock; + struct hlist_head *head; if (avc_latest_notif_update(avc, avd->seqno, 1)) - goto out; + return NULL; node = avc_alloc_node(avc); - if (node) { - struct hlist_head *head; - spinlock_t *lock; - int rc = 0; + if (!node) + return NULL; - hvalue = avc_hash(ssid, tsid, tclass); - avc_node_populate(node, ssid, tsid, tclass, avd); - rc = avc_xperms_populate(node, xp_node); - if (rc) { - kmem_cache_free(avc_node_cachep, node); - return NULL; - } - head = &avc->avc_cache.slots[hvalue]; - lock = &avc->avc_cache.slots_lock[hvalue]; - - spin_lock_irqsave(lock, flag); - hlist_for_each_entry(pos, head, list) { - if (pos->ae.ssid == ssid && - pos->ae.tsid == tsid && - pos->ae.tclass == tclass) { - avc_node_replace(avc, node, pos); - goto found; - } - } - hlist_add_head_rcu(&node->list, head); -found: - spin_unlock_irqrestore(lock, flag); + avc_node_populate(node, ssid, tsid, tclass, avd); + if (avc_xperms_populate(node, xp_node)) { + avc_node_kill(avc, node); + return NULL; } -out: + + hvalue = avc_hash(ssid, tsid, tclass); + head = &avc->avc_cache.slots[hvalue]; + lock = &avc->avc_cache.slots_lock[hvalue]; + spin_lock_irqsave(lock, flag); + hlist_for_each_entry(pos, head, list) { + if (pos->ae.ssid == ssid && + pos->ae.tsid == tsid && + pos->ae.tclass == tclass) { + avc_node_replace(avc, node, pos); + goto found; + } + } + hlist_add_head_rcu(&node->list, head); +found: + spin_unlock_irqrestore(lock, flag); return node; } From 210a292874517782bed2e2220c7beb1608d3b05d Mon Sep 17 00:00:00 2001 From: Yang Guo Date: Thu, 12 Dec 2019 10:02:24 +0800 Subject: [PATCH 09/28] selinux: remove unnecessary selinux cred request task_security_struct was obtained at the beginning of may_create and selinux_inode_init_security, no need to obtain again. may_create will be called very frequently when create dir and file. Cc: Paul Moore Cc: Stephen Smalley Cc: Eric Paris Signed-off-by: Yang Guo Signed-off-by: Shaokun Zhang Signed-off-by: Paul Moore --- security/selinux/hooks.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 47626342b6e5..40ec866e48da 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -1833,8 +1833,8 @@ static int may_create(struct inode *dir, if (rc) return rc; - rc = selinux_determine_inode_label(selinux_cred(current_cred()), dir, - &dentry->d_name, tclass, &newsid); + rc = selinux_determine_inode_label(tsec, dir, &dentry->d_name, tclass, + &newsid); if (rc) return rc; @@ -2906,8 +2906,7 @@ static int selinux_inode_init_security(struct inode *inode, struct inode *dir, newsid = tsec->create_sid; - rc = selinux_determine_inode_label(selinux_cred(current_cred()), - dir, qstr, + rc = selinux_determine_inode_label(tsec, dir, qstr, inode_mode_to_security_class(inode->i_mode), &newsid); if (rc) From 6c5a682e6497cb1f7a67303ce098462a36bed362 Mon Sep 17 00:00:00 2001 From: Stephen Smalley Date: Tue, 17 Dec 2019 09:15:10 -0500 Subject: [PATCH 10/28] selinux: clean up selinux_enabled/disabled/enforcing_boot Rename selinux_enabled to selinux_enabled_boot to make it clear that it only reflects whether SELinux was enabled at boot. Replace the references to it in the MAC_STATUS audit log in sel_write_enforce() with hardcoded "1" values because this code is only reachable if SELinux is enabled and does not change its value, and update the corresponding MAC_STATUS audit log in sel_write_disable(). Stop clearing selinux_enabled in selinux_disable() since it is not used outside of initialization code that runs before selinux_disable() can be reached. Mark both selinux_enabled_boot and selinux_enforcing_boot as __initdata since they are only used in initialization code. Wrap the disabled field in the struct selinux_state with CONFIG_SECURITY_SELINUX_DISABLE since it is only used for runtime disable. Signed-off-by: Stephen Smalley Signed-off-by: Paul Moore --- security/selinux/hooks.c | 12 +++++------- security/selinux/ibpkey.c | 2 +- security/selinux/include/security.h | 4 +++- security/selinux/netif.c | 2 +- security/selinux/netnode.c | 2 +- security/selinux/netport.c | 2 +- security/selinux/selinuxfs.c | 11 +++++------ 7 files changed, 17 insertions(+), 18 deletions(-) diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 40ec866e48da..659c4a81e897 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -109,7 +109,7 @@ struct selinux_state selinux_state; static atomic_t selinux_secmark_refcount = ATOMIC_INIT(0); #ifdef CONFIG_SECURITY_SELINUX_DEVELOP -static int selinux_enforcing_boot; +static int selinux_enforcing_boot __initdata; static int __init enforcing_setup(char *str) { @@ -123,13 +123,13 @@ __setup("enforcing=", enforcing_setup); #define selinux_enforcing_boot 1 #endif -int selinux_enabled __lsm_ro_after_init = 1; +int selinux_enabled_boot __initdata = 1; #ifdef CONFIG_SECURITY_SELINUX_BOOTPARAM static int __init selinux_enabled_setup(char *str) { unsigned long enabled; if (!kstrtoul(str, 0, &enabled)) - selinux_enabled = enabled ? 1 : 0; + selinux_enabled_boot = enabled ? 1 : 0; return 1; } __setup("selinux=", selinux_enabled_setup); @@ -7202,7 +7202,7 @@ void selinux_complete_init(void) DEFINE_LSM(selinux) = { .name = "selinux", .flags = LSM_FLAG_LEGACY_MAJOR | LSM_FLAG_EXCLUSIVE, - .enabled = &selinux_enabled, + .enabled = &selinux_enabled_boot, .blobs = &selinux_blob_sizes, .init = selinux_init, }; @@ -7271,7 +7271,7 @@ static int __init selinux_nf_ip_init(void) { int err; - if (!selinux_enabled) + if (!selinux_enabled_boot) return 0; pr_debug("SELinux: Registering netfilter hooks\n"); @@ -7318,8 +7318,6 @@ int selinux_disable(struct selinux_state *state) pr_info("SELinux: Disabled at runtime.\n"); - selinux_enabled = 0; - security_delete_hooks(selinux_hooks, ARRAY_SIZE(selinux_hooks)); /* Try to destroy the avc node cache */ diff --git a/security/selinux/ibpkey.c b/security/selinux/ibpkey.c index de92365e4324..f68a7617cfb9 100644 --- a/security/selinux/ibpkey.c +++ b/security/selinux/ibpkey.c @@ -222,7 +222,7 @@ static __init int sel_ib_pkey_init(void) { int iter; - if (!selinux_enabled) + if (!selinux_enabled_boot) return 0; for (iter = 0; iter < SEL_PKEY_HASH_SIZE; iter++) { diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h index 8c0dbbd076c6..af623f03922c 100644 --- a/security/selinux/include/security.h +++ b/security/selinux/include/security.h @@ -69,7 +69,7 @@ struct netlbl_lsm_secattr; -extern int selinux_enabled; +extern int selinux_enabled_boot; /* Policy capabilities */ enum { @@ -99,7 +99,9 @@ struct selinux_avc; struct selinux_ss; struct selinux_state { +#ifdef CONFIG_SECURITY_SELINUX_DISABLE bool disabled; +#endif #ifdef CONFIG_SECURITY_SELINUX_DEVELOP bool enforcing; #endif diff --git a/security/selinux/netif.c b/security/selinux/netif.c index e40fecd73752..15b8c1bcd7d0 100644 --- a/security/selinux/netif.c +++ b/security/selinux/netif.c @@ -266,7 +266,7 @@ static __init int sel_netif_init(void) { int i; - if (!selinux_enabled) + if (!selinux_enabled_boot) return 0; for (i = 0; i < SEL_NETIF_HASH_SIZE; i++) diff --git a/security/selinux/netnode.c b/security/selinux/netnode.c index 9ab84efa46c7..dff587d1e164 100644 --- a/security/selinux/netnode.c +++ b/security/selinux/netnode.c @@ -291,7 +291,7 @@ static __init int sel_netnode_init(void) { int iter; - if (!selinux_enabled) + if (!selinux_enabled_boot) return 0; for (iter = 0; iter < SEL_NETNODE_HASH_SIZE; iter++) { diff --git a/security/selinux/netport.c b/security/selinux/netport.c index 3f8b2c0458c8..de727f7489b7 100644 --- a/security/selinux/netport.c +++ b/security/selinux/netport.c @@ -225,7 +225,7 @@ static __init int sel_netport_init(void) { int iter; - if (!selinux_enabled) + if (!selinux_enabled_boot) return 0; for (iter = 0; iter < SEL_NETPORT_HASH_SIZE; iter++) { diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c index dd7bb1f1dc99..278417e67b4c 100644 --- a/security/selinux/selinuxfs.c +++ b/security/selinux/selinuxfs.c @@ -168,11 +168,10 @@ static ssize_t sel_write_enforce(struct file *file, const char __user *buf, goto out; audit_log(audit_context(), GFP_KERNEL, AUDIT_MAC_STATUS, "enforcing=%d old_enforcing=%d auid=%u ses=%u" - " enabled=%d old-enabled=%d lsm=selinux res=1", + " enabled=1 old-enabled=1 lsm=selinux res=1", new_value, old_value, from_kuid(&init_user_ns, audit_get_loginuid(current)), - audit_get_sessionid(current), - selinux_enabled, selinux_enabled); + audit_get_sessionid(current)); enforcing_set(state, new_value); if (new_value) avc_ss_reset(state->avc, 0); @@ -304,10 +303,10 @@ static ssize_t sel_write_disable(struct file *file, const char __user *buf, goto out; audit_log(audit_context(), GFP_KERNEL, AUDIT_MAC_STATUS, "enforcing=%d old_enforcing=%d auid=%u ses=%u" - " enabled=%d old-enabled=%d lsm=selinux res=1", + " enabled=0 old-enabled=1 lsm=selinux res=1", enforcing, enforcing, from_kuid(&init_user_ns, audit_get_loginuid(current)), - audit_get_sessionid(current), 0, 1); + audit_get_sessionid(current)); } length = count; @@ -2105,7 +2104,7 @@ static int __init init_sel_fs(void) sizeof(NULL_FILE_NAME)-1); int err; - if (!selinux_enabled) + if (!selinux_enabled_boot) return 0; err = sysfs_create_mount_point(fs_kobj, "selinux"); From 5c108d4e18f80be01965792726c81b105fbd677a Mon Sep 17 00:00:00 2001 From: Stephen Smalley Date: Fri, 13 Dec 2019 15:28:38 -0500 Subject: [PATCH 11/28] selinux: randomize layout of key structures Randomize the layout of key selinux data structures. Initially this is applied to the selinux_state, selinux_ss, policydb, and task_security_struct data structures. NB To test/use this mechanism, one must install the necessary build-time dependencies, e.g. gcc-plugin-devel on Fedora, and enable CONFIG_GCC_PLUGIN_RANDSTRUCT in the kernel configuration. Signed-off-by: Stephen Smalley Reviewed-by: Kees Cook [PM: double semi-colon fixed] Signed-off-by: Paul Moore --- security/selinux/include/objsec.h | 2 +- security/selinux/include/security.h | 2 +- security/selinux/ss/policydb.h | 2 +- security/selinux/ss/services.h | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/security/selinux/include/objsec.h b/security/selinux/include/objsec.h index a4a86cbcfb0a..330b7b6d44e0 100644 --- a/security/selinux/include/objsec.h +++ b/security/selinux/include/objsec.h @@ -35,7 +35,7 @@ struct task_security_struct { u32 create_sid; /* fscreate SID */ u32 keycreate_sid; /* keycreate SID */ u32 sockcreate_sid; /* fscreate SID */ -}; +} __randomize_layout; enum label_initialized { LABEL_INVALID, /* invalid or not initialized */ diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h index af623f03922c..ecdd610e6449 100644 --- a/security/selinux/include/security.h +++ b/security/selinux/include/security.h @@ -110,7 +110,7 @@ struct selinux_state { bool policycap[__POLICYDB_CAPABILITY_MAX]; struct selinux_avc *avc; struct selinux_ss *ss; -}; +} __randomize_layout; void selinux_ss_init(struct selinux_ss **ss); void selinux_avc_init(struct selinux_avc **avc); diff --git a/security/selinux/ss/policydb.h b/security/selinux/ss/policydb.h index bc56b14e2216..69b24191fa38 100644 --- a/security/selinux/ss/policydb.h +++ b/security/selinux/ss/policydb.h @@ -307,7 +307,7 @@ struct policydb { u16 process_class; u32 process_trans_perms; -}; +} __randomize_layout; extern void policydb_destroy(struct policydb *p); extern int policydb_load_isids(struct policydb *p, struct sidtab *s); diff --git a/security/selinux/ss/services.h b/security/selinux/ss/services.h index fc40640a9725..c5896f39e8f6 100644 --- a/security/selinux/ss/services.h +++ b/security/selinux/ss/services.h @@ -31,7 +31,7 @@ struct selinux_ss { struct selinux_map map; struct page *status_page; struct mutex status_lock; -}; +} __randomize_layout; void services_compute_xperms_drivers(struct extended_perms *xperms, struct avtab_node *node); From 030b995ad9ece9fa2d218af4429c1c78c2342096 Mon Sep 17 00:00:00 2001 From: Jaihind Yadav Date: Tue, 17 Dec 2019 17:25:47 +0530 Subject: [PATCH 12/28] selinux: ensure we cleanup the internal AVC counters on error in avc_update() In AVC update we don't call avc_node_kill() when avc_xperms_populate() fails, resulting in the avc->avc_cache.active_nodes counter having a false value. In last patch this changes was missed , so correcting it. Fixes: fa1aa143ac4a ("selinux: extended permissions for ioctls") Signed-off-by: Jaihind Yadav Signed-off-by: Ravi Kumar Siddojigari [PM: merge fuzz, minor description cleanup] Signed-off-by: Paul Moore --- security/selinux/avc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/security/selinux/avc.c b/security/selinux/avc.c index 6646300f7ccb..d18cb32a242a 100644 --- a/security/selinux/avc.c +++ b/security/selinux/avc.c @@ -891,7 +891,7 @@ static int avc_update_node(struct selinux_avc *avc, if (orig->ae.xp_node) { rc = avc_xperms_populate(node, orig->ae.xp_node); if (rc) { - kmem_cache_free(avc_node_cachep, node); + avc_node_kill(avc, node); goto out_unlock; } } From 15b590a81fcdd44ddcb4810f2a6334df8b6ca512 Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Mon, 23 Dec 2019 16:38:36 -0500 Subject: [PATCH 13/28] selinux: ensure the policy has been loaded before reading the sidtab stats Check to make sure we have loaded a policy before we query the sidtab's hash stats. Failure to do so could result in a kernel panic/oops due to a dereferenced NULL pointer. Fixes: 66f8e2f03c02 ("selinux: sidtab reverse lookup hash table") Reported-by: kernel test robot Signed-off-by: Paul Moore --- security/selinux/ss/services.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c index 743b85ede4ef..c9e38f1ede02 100644 --- a/security/selinux/ss/services.c +++ b/security/selinux/ss/services.c @@ -1286,6 +1286,12 @@ int security_sidtab_hash_stats(struct selinux_state *state, char *page) { int rc; + if (!state->initialized) { + pr_err("SELinux: %s: called before initial load_policy\n", + __func__); + return -EINVAL; + } + read_lock(&state->ss->policy_rwlock); rc = sidtab_hash_stats(state->ss->sidtab, page); read_unlock(&state->ss->policy_rwlock); From f1268534027a792f3d97a0cfff8041d314ef2fca Mon Sep 17 00:00:00 2001 From: YueHaibing Date: Tue, 24 Dec 2019 20:45:52 +0800 Subject: [PATCH 14/28] selinux: remove set but not used variable 'sidtab' security/selinux/ss/services.c: In function security_port_sid: security/selinux/ss/services.c:2346:17: warning: variable sidtab set but not used [-Wunused-but-set-variable] security/selinux/ss/services.c: In function security_ib_endport_sid: security/selinux/ss/services.c:2435:17: warning: variable sidtab set but not used [-Wunused-but-set-variable] security/selinux/ss/services.c: In function security_netif_sid: security/selinux/ss/services.c:2480:17: warning: variable sidtab set but not used [-Wunused-but-set-variable] security/selinux/ss/services.c: In function security_fs_use: security/selinux/ss/services.c:2831:17: warning: variable sidtab set but not used [-Wunused-but-set-variable] Since commit 66f8e2f03c02 ("selinux: sidtab reverse lookup hash table") 'sidtab' is not used any more, so remove it. Reported-by: Hulk Robot Signed-off-by: YueHaibing Signed-off-by: Paul Moore --- security/selinux/ss/services.c | 8 -------- 1 file changed, 8 deletions(-) diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c index c9e38f1ede02..55cf42945cba 100644 --- a/security/selinux/ss/services.c +++ b/security/selinux/ss/services.c @@ -2349,14 +2349,12 @@ int security_port_sid(struct selinux_state *state, u8 protocol, u16 port, u32 *out_sid) { struct policydb *policydb; - struct sidtab *sidtab; struct ocontext *c; int rc = 0; read_lock(&state->ss->policy_rwlock); policydb = &state->ss->policydb; - sidtab = state->ss->sidtab; c = policydb->ocontexts[OCON_PORT]; while (c) { @@ -2438,14 +2436,12 @@ int security_ib_endport_sid(struct selinux_state *state, const char *dev_name, u8 port_num, u32 *out_sid) { struct policydb *policydb; - struct sidtab *sidtab; struct ocontext *c; int rc = 0; read_lock(&state->ss->policy_rwlock); policydb = &state->ss->policydb; - sidtab = state->ss->sidtab; c = policydb->ocontexts[OCON_IBENDPORT]; while (c) { @@ -2483,14 +2479,12 @@ int security_netif_sid(struct selinux_state *state, char *name, u32 *if_sid) { struct policydb *policydb; - struct sidtab *sidtab; int rc = 0; struct ocontext *c; read_lock(&state->ss->policy_rwlock); policydb = &state->ss->policydb; - sidtab = state->ss->sidtab; c = policydb->ocontexts[OCON_NETIF]; while (c) { @@ -2834,7 +2828,6 @@ int security_genfs_sid(struct selinux_state *state, int security_fs_use(struct selinux_state *state, struct super_block *sb) { struct policydb *policydb; - struct sidtab *sidtab; int rc = 0; struct ocontext *c; struct superblock_security_struct *sbsec = sb->s_security; @@ -2843,7 +2836,6 @@ 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; c = policydb->ocontexts[OCON_FSUSE]; while (c) { From 7e78c875143b639dca887e335f7d045480ec28d8 Mon Sep 17 00:00:00 2001 From: liuyang34 Date: Tue, 7 Jan 2020 09:39:18 +0800 Subject: [PATCH 15/28] selinuxfs: use scnprintf to get real length for inode The return value of snprintf maybe over the size of TMPBUFLEN, use scnprintf instead in sel_read_class and sel_read_perm. Signed-off-by: liuyang34 [PM: cleaned up the description] Signed-off-by: Paul Moore --- security/selinux/selinuxfs.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c index 278417e67b4c..d903574f1f0d 100644 --- a/security/selinux/selinuxfs.c +++ b/security/selinux/selinuxfs.c @@ -1728,7 +1728,7 @@ static ssize_t sel_read_class(struct file *file, char __user *buf, { unsigned long ino = file_inode(file)->i_ino; char res[TMPBUFLEN]; - ssize_t len = snprintf(res, sizeof(res), "%d", sel_ino_to_class(ino)); + ssize_t len = scnprintf(res, sizeof(res), "%d", sel_ino_to_class(ino)); return simple_read_from_buffer(buf, count, ppos, res, len); } @@ -1742,7 +1742,7 @@ static ssize_t sel_read_perm(struct file *file, char __user *buf, { unsigned long ino = file_inode(file)->i_ino; char res[TMPBUFLEN]; - ssize_t len = snprintf(res, sizeof(res), "%d", sel_ino_to_perm(ino)); + ssize_t len = scnprintf(res, sizeof(res), "%d", sel_ino_to_perm(ino)); return simple_read_from_buffer(buf, count, ppos, res, len); } From 7a4b51947475a7f67e2bd06c4a4c768e2e64a975 Mon Sep 17 00:00:00 2001 From: Hridya Valsaraju Date: Mon, 6 Jan 2020 10:13:29 -0800 Subject: [PATCH 16/28] selinux: allow per-file labelling for binderfs This patch allows genfscon per-file labeling for binderfs. This is required to have separate permissions to allow access to binder, hwbinder and vndbinder devices which are relocating to binderfs. Acked-by: Jeff Vander Stoep Acked-by: Mark Salyzyn Signed-off-by: Hridya Valsaraju Acked-by: Stephen Smalley Signed-off-by: Paul Moore --- security/selinux/hooks.c | 1 + 1 file changed, 1 insertion(+) diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 659c4a81e897..63a6e36abe9f 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -752,6 +752,7 @@ static int selinux_set_mnt_opts(struct super_block *sb, if (!strcmp(sb->s_type->name, "debugfs") || !strcmp(sb->s_type->name, "tracefs") || + !strcmp(sb->s_type->name, "binderfs") || !strcmp(sb->s_type->name, "pstore")) sbsec->flags |= SE_SBGENFS; From 89b223bfb8a89731bea4c84982b5d2ad7ba460e3 Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Wed, 18 Dec 2019 21:45:08 -0500 Subject: [PATCH 17/28] selinux: deprecate disabling SELinux and runtime Deprecate the CONFIG_SECURITY_SELINUX_DISABLE functionality. The code was originally developed to make it easier for Linux distributions to support architectures where adding parameters to the kernel command line was difficult. Unfortunately, supporting runtime disable meant we had to make some security trade-offs when it came to the LSM hooks, as documented in the Kconfig help text: NOTE: selecting this option will disable the '__ro_after_init' kernel hardening feature for security hooks. Please consider using the selinux=0 boot parameter instead of enabling this option. Fortunately it looks as if that the original motivation for the runtime disable functionality is gone, and Fedora/RHEL appears to be the only major distribution enabling this capability at build time so we are now taking steps to remove it entirely from the kernel. The first step is to mark the functionality as deprecated and print an error when it is used (what this patch is doing). As Fedora/RHEL makes progress in transitioning the distribution away from runtime disable, we will introduce follow-up patches over several kernel releases which will block for increasing periods of time when the runtime disable is used. Finally we will remove the option entirely once we believe all users have moved to the kernel cmdline approach. Acked-by: Casey Schaufler Acked-by: Ondrej Mosnacek Acked-by: Stephen Smalley Signed-off-by: Paul Moore --- .../ABI/obsolete/sysfs-selinux-disable | 26 +++++++++++++++++++ MAINTAINERS | 1 + security/selinux/Kconfig | 3 +++ security/selinux/selinuxfs.c | 7 +++++ 4 files changed, 37 insertions(+) create mode 100644 Documentation/ABI/obsolete/sysfs-selinux-disable diff --git a/Documentation/ABI/obsolete/sysfs-selinux-disable b/Documentation/ABI/obsolete/sysfs-selinux-disable new file mode 100644 index 000000000000..c340278e3cf8 --- /dev/null +++ b/Documentation/ABI/obsolete/sysfs-selinux-disable @@ -0,0 +1,26 @@ +What: /sys/fs/selinux/disable +Date: April 2005 (predates git) +KernelVersion: 2.6.12-rc2 (predates git) +Contact: selinux@vger.kernel.org +Description: + + The selinuxfs "disable" node allows SELinux to be disabled at runtime + prior to a policy being loaded into the kernel. If disabled via this + mechanism, SELinux will remain disabled until the system is rebooted. + + The preferred method of disabling SELinux is via the "selinux=0" boot + parameter, but the selinuxfs "disable" node was created to make it + easier for systems with primitive bootloaders that did not allow for + easy modification of the kernel command line. Unfortunately, allowing + for SELinux to be disabled at runtime makes it difficult to secure the + kernel's LSM hooks using the "__ro_after_init" feature. + + Thankfully, the need for the SELinux runtime disable appears to be + gone, the default Kconfig configuration disables this selinuxfs node, + and only one of the major distributions, Fedora, supports disabling + SELinux at runtime. Fedora is in the process of removing the + selinuxfs "disable" node and once that is complete we will start the + slow process of removing this code from the kernel. + + More information on /sys/fs/selinux/disable can be found under the + CONFIG_SECURITY_SELINUX_DISABLE Kconfig option. diff --git a/MAINTAINERS b/MAINTAINERS index bd5847e802de..9dc5767861b5 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -14806,6 +14806,7 @@ F: include/uapi/linux/selinux_netlink.h F: security/selinux/ F: scripts/selinux/ F: Documentation/admin-guide/LSM/SELinux.rst +F: Documentation/ABI/obsolete/sysfs-selinux-disable SENSABLE PHANTOM M: Jiri Slaby diff --git a/security/selinux/Kconfig b/security/selinux/Kconfig index 996d35d950f7..580ac24c7aa1 100644 --- a/security/selinux/Kconfig +++ b/security/selinux/Kconfig @@ -42,6 +42,9 @@ config SECURITY_SELINUX_DISABLE using the selinux=0 boot parameter instead of enabling this option. + WARNING: this option is deprecated and will be removed in a future + kernel release. + If you are unsure how to answer this question, answer N. config SECURITY_SELINUX_DEVELOP diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c index d903574f1f0d..79c710911a3c 100644 --- a/security/selinux/selinuxfs.c +++ b/security/selinux/selinuxfs.c @@ -281,6 +281,13 @@ static ssize_t sel_write_disable(struct file *file, const char __user *buf, int new_value; int enforcing; + /* NOTE: we are now officially considering runtime disable as + * deprecated, and using it will become increasingly painful + * (e.g. sleeping/blocking) as we progress through future + * kernel releases until eventually it is removed + */ + pr_err("SELinux: Runtime disable is deprecated, use selinux=0 on the kernel cmdline.\n"); + if (count >= PAGE_SIZE) return -ENOMEM; From d41415eb5edae2a750940aa24924993b81947040 Mon Sep 17 00:00:00 2001 From: Stephen Smalley Date: Tue, 7 Jan 2020 11:35:04 -0500 Subject: [PATCH 18/28] Documentation,selinux: fix references to old selinuxfs mount point selinuxfs was originally mounted on /selinux, and various docs and kconfig help texts referred to nodes under it. In Linux 3.0, /sys/fs/selinux was introduced as the preferred mount point for selinuxfs. Fix all the old references to /selinux/ to /sys/fs/selinux/. While we are there, update the description of the selinux boot parameter to reflect the fact that the default value is always 1 since commit be6ec88f41ba94 ("selinux: Remove SECURITY_SELINUX_BOOTPARAM_VALUE") and drop discussion of runtime disable since it is deprecated. Signed-off-by: Stephen Smalley Signed-off-by: Paul Moore --- Documentation/admin-guide/kernel-parameters.txt | 9 ++++----- security/selinux/Kconfig | 7 ++++--- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt index ade4e6ec23e0..eed51293d6cf 100644 --- a/Documentation/admin-guide/kernel-parameters.txt +++ b/Documentation/admin-guide/kernel-parameters.txt @@ -511,7 +511,7 @@ 1 -- check protection requested by application. Default value is set via a kernel config option. Value can be changed at runtime via - /selinux/checkreqprot. + /sys/fs/selinux/checkreqprot. cio_ignore= [S390] See Documentation/s390/common_io.rst for details. @@ -1245,7 +1245,8 @@ 0 -- permissive (log only, no denials). 1 -- enforcing (deny and log). Default value is 0. - Value can be changed at runtime via /selinux/enforce. + Value can be changed at runtime via + /sys/fs/selinux/enforce. erst_disable [ACPI] Disable Error Record Serialization Table (ERST) @@ -4348,9 +4349,7 @@ See security/selinux/Kconfig help text. 0 -- disable. 1 -- enable. - Default value is set via kernel config option. - If enabled at boot time, /selinux/disable can be used - later to disable prior to initial policy load. + Default value is 1. apparmor= [APPARMOR] Disable or enable AppArmor at boot time Format: { "0" | "1" } diff --git a/security/selinux/Kconfig b/security/selinux/Kconfig index 580ac24c7aa1..1014cb0ee956 100644 --- a/security/selinux/Kconfig +++ b/security/selinux/Kconfig @@ -58,7 +58,8 @@ config SECURITY_SELINUX_DEVELOP kernel will start in permissive mode (log everything, deny nothing) unless you specify enforcing=1 on the kernel command line. You can interactively toggle the kernel between enforcing mode and - permissive mode (if permitted by the policy) via /selinux/enforce. + permissive mode (if permitted by the policy) via + /sys/fs/selinux/enforce. config SECURITY_SELINUX_AVC_STATS bool "NSA SELinux AVC Statistics" @@ -66,7 +67,7 @@ config SECURITY_SELINUX_AVC_STATS default y help This option collects access vector cache statistics to - /selinux/avc/cache_stats, which may be monitored via + /sys/fs/selinux/avc/cache_stats, which may be monitored via tools such as avcstat. config SECURITY_SELINUX_CHECKREQPROT_VALUE @@ -85,7 +86,7 @@ config SECURITY_SELINUX_CHECKREQPROT_VALUE default to checking the protection requested by the application. The checkreqprot flag may be changed from the default via the 'checkreqprot=' boot parameter. It may also be changed at runtime - via /selinux/checkreqprot if authorized by policy. + via /sys/fs/selinux/checkreqprot if authorized by policy. If you are unsure how to answer this question, answer 0. From b82f3f6894ec68dfada736bdf87997b1e0d3b53f Mon Sep 17 00:00:00 2001 From: Huaisheng Ye Date: Fri, 10 Jan 2020 17:58:56 +0800 Subject: [PATCH 19/28] selinux: remove redundant msg_msg_alloc_security selinux_msg_msg_alloc_security only calls msg_msg_alloc_security but do nothing else. And also msg_msg_alloc_security is just used by the former. Remove the redundant function to simplify the code. Signed-off-by: Huaisheng Ye Acked-by: Stephen Smalley Signed-off-by: Paul Moore --- security/selinux/hooks.c | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 63a6e36abe9f..81f570cffee5 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -5894,16 +5894,6 @@ static void ipc_init_security(struct ipc_security_struct *isec, u16 sclass) isec->sid = current_sid(); } -static int msg_msg_alloc_security(struct msg_msg *msg) -{ - struct msg_security_struct *msec; - - msec = selinux_msg_msg(msg); - msec->sid = SECINITSID_UNLABELED; - - return 0; -} - static int ipc_has_perm(struct kern_ipc_perm *ipc_perms, u32 perms) { @@ -5922,7 +5912,12 @@ static int ipc_has_perm(struct kern_ipc_perm *ipc_perms, static int selinux_msg_msg_alloc_security(struct msg_msg *msg) { - return msg_msg_alloc_security(msg); + struct msg_security_struct *msec; + + msec = selinux_msg_msg(msg); + msec->sid = SECINITSID_UNLABELED; + + return 0; } /* message queue security operations */ From fe49c7e4f85a9b2c3628e2f21e973ea6e26d2be7 Mon Sep 17 00:00:00 2001 From: Ravi Kumar Siddojigari Date: Thu, 9 Jan 2020 16:40:47 +0530 Subject: [PATCH 20/28] selinux: move ibpkeys code under CONFIG_SECURITY_INFINIBAND. Move cache based pkey sid retrieval code which was added with commit "409dcf31" under CONFIG_SECURITY_INFINIBAND. As its going to alloc a new cache which impacts low RAM devices which was enabled by default. Suggested-by: Paul Moore Signed-off-by: Ravi Kumar Siddojigari [PM: checkpatch.pl cleanups, fixed capitalization in the description] Signed-off-by: Paul Moore --- security/selinux/Makefile | 4 +++- security/selinux/include/ibpkey.h | 13 ++++++++++++- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/security/selinux/Makefile b/security/selinux/Makefile index ccf950409384..2000f95fb197 100644 --- a/security/selinux/Makefile +++ b/security/selinux/Makefile @@ -6,7 +6,7 @@ obj-$(CONFIG_SECURITY_SELINUX) := selinux.o selinux-y := avc.o hooks.o selinuxfs.o netlink.o nlmsgtab.o netif.o \ - netnode.o netport.o ibpkey.o \ + netnode.o netport.o \ ss/ebitmap.o ss/hashtab.o ss/symtab.o ss/sidtab.o ss/avtab.o \ ss/policydb.o ss/services.o ss/conditional.o ss/mls.o ss/status.o @@ -14,6 +14,8 @@ selinux-$(CONFIG_SECURITY_NETWORK_XFRM) += xfrm.o selinux-$(CONFIG_NETLABEL) += netlabel.o +selinux-$(CONFIG_SECURITY_INFINIBAND) += ibpkey.o + ccflags-y := -I$(srctree)/security/selinux -I$(srctree)/security/selinux/include $(addprefix $(obj)/,$(selinux-y)): $(obj)/flask.h diff --git a/security/selinux/include/ibpkey.h b/security/selinux/include/ibpkey.h index a2ebe397bcb7..e6ac1d23320b 100644 --- a/security/selinux/include/ibpkey.h +++ b/security/selinux/include/ibpkey.h @@ -14,8 +14,19 @@ #ifndef _SELINUX_IB_PKEY_H #define _SELINUX_IB_PKEY_H +#ifdef CONFIG_SECURITY_INFINIBAND void sel_ib_pkey_flush(void); - int sel_ib_pkey_sid(u64 subnet_prefix, u16 pkey, u32 *sid); +#else +static inline void sel_ib_pkey_flush(void) +{ + return; +} +static inline int sel_ib_pkey_sid(u64 subnet_prefix, u16 pkey, u32 *sid) +{ + *sid = SECINITSID_UNLABELED; + return 0; +} +#endif #endif From b78b7d59bdbe6028ab362c2551dc684872f2052a Mon Sep 17 00:00:00 2001 From: Stephen Smalley Date: Wed, 8 Jan 2020 12:23:56 -0500 Subject: [PATCH 21/28] selinux: make default_noexec read-only after init SELinux checks whether VM_EXEC is set in the VM_DATA_DEFAULT_FLAGS during initialization and saves the result in default_noexec for use in its mmap and mprotect hook function implementations to decide whether to apply EXECMEM, EXECHEAP, EXECSTACK, and EXECMOD checks. Mark default_noexec as ro_after_init to prevent later clearing it and thereby disabling these checks. It is only set legitimately from init code. Signed-off-by: Stephen Smalley Signed-off-by: Paul Moore --- security/selinux/hooks.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 81f570cffee5..921283f47862 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -3647,7 +3647,7 @@ static int selinux_file_ioctl(struct file *file, unsigned int cmd, return error; } -static int default_noexec; +static int default_noexec __ro_after_init; static int file_map_prot_check(struct file *file, unsigned long prot, int shared) { From 65cddd50980be8c9c27ad7518a0dc812eccb25d5 Mon Sep 17 00:00:00 2001 From: Ondrej Mosnacek Date: Tue, 7 Jan 2020 14:31:53 +0100 Subject: [PATCH 22/28] selinux: treat atomic flags more carefully The disabled/enforcing/initialized flags are all accessed concurrently by threads so use the appropriate accessors that ensure atomicity and document that it is expected. Use smp_load/acquire...() helpers (with memory barriers) for the initialized flag, since it gates access to the rest of the state structures. Note that the disabled flag is currently not used for anything other than avoiding double disable, but it will be used for bailing out of hooks once security_delete_hooks() is removed. Signed-off-by: Ondrej Mosnacek Acked-by: Stephen Smalley Reviewed-by: Kees Cook Reviewed-by: James Morris Signed-off-by: Paul Moore --- security/selinux/hooks.c | 21 ++++++++-------- security/selinux/include/security.h | 33 +++++++++++++++++++++++-- security/selinux/ss/services.c | 38 ++++++++++++++--------------- 3 files changed, 61 insertions(+), 31 deletions(-) diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 921283f47862..a81631f8cc5d 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -272,7 +272,7 @@ static int __inode_security_revalidate(struct inode *inode, might_sleep_if(may_sleep); - if (selinux_state.initialized && + if (selinux_initialized(&selinux_state) && isec->initialized != LABEL_INITIALIZED) { if (!may_sleep) return -ECHILD; @@ -659,7 +659,7 @@ static int selinux_set_mnt_opts(struct super_block *sb, mutex_lock(&sbsec->lock); - if (!selinux_state.initialized) { + if (!selinux_initialized(&selinux_state)) { if (!opts) { /* Defer initialization until selinux_complete_init, after the initial policy is loaded and the security @@ -929,7 +929,7 @@ static int selinux_sb_clone_mnt_opts(const struct super_block *oldsb, * if the parent was able to be mounted it clearly had no special lsm * mount options. thus we can safely deal with this superblock later */ - if (!selinux_state.initialized) + if (!selinux_initialized(&selinux_state)) return 0; /* @@ -1104,7 +1104,7 @@ static int selinux_sb_show_options(struct seq_file *m, struct super_block *sb) if (!(sbsec->flags & SE_SBINITIALIZED)) return 0; - if (!selinux_state.initialized) + if (!selinux_initialized(&selinux_state)) return 0; if (sbsec->flags & FSCONTEXT_MNT) { @@ -2921,7 +2921,8 @@ static int selinux_inode_init_security(struct inode *inode, struct inode *dir, isec->initialized = LABEL_INITIALIZED; } - if (!selinux_state.initialized || !(sbsec->flags & SBLABEL_MNT)) + if (!selinux_initialized(&selinux_state) || + !(sbsec->flags & SBLABEL_MNT)) return -EOPNOTSUPP; if (name) @@ -3144,7 +3145,7 @@ static int selinux_inode_setxattr(struct dentry *dentry, const char *name, return dentry_has_perm(current_cred(), dentry, FILE__SETATTR); } - if (!selinux_state.initialized) + if (!selinux_initialized(&selinux_state)) return (inode_owner_or_capable(inode) ? 0 : -EPERM); sbsec = inode->i_sb->s_security; @@ -3230,7 +3231,7 @@ static void selinux_inode_post_setxattr(struct dentry *dentry, const char *name, return; } - if (!selinux_state.initialized) { + if (!selinux_initialized(&selinux_state)) { /* If we haven't even been initialized, then we can't validate * against a policy, so leave the label as invalid. It may * resolve to a valid label on the next revalidation try if @@ -7300,17 +7301,17 @@ static void selinux_nf_ip_exit(void) #ifdef CONFIG_SECURITY_SELINUX_DISABLE int selinux_disable(struct selinux_state *state) { - if (state->initialized) { + if (selinux_initialized(state)) { /* Not permitted after initial policy load. */ return -EINVAL; } - if (state->disabled) { + if (selinux_disabled(state)) { /* Only do this once. */ return -EINVAL; } - state->disabled = 1; + selinux_mark_disabled(state); pr_info("SELinux: Disabled at runtime.\n"); diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h index ecdd610e6449..a39f9565d80b 100644 --- a/security/selinux/include/security.h +++ b/security/selinux/include/security.h @@ -117,15 +117,27 @@ void selinux_avc_init(struct selinux_avc **avc); extern struct selinux_state selinux_state; +static inline bool selinux_initialized(const struct selinux_state *state) +{ + /* do a synchronized load to avoid race conditions */ + return smp_load_acquire(&state->initialized); +} + +static inline void selinux_mark_initialized(struct selinux_state *state) +{ + /* do a synchronized write to avoid race conditions */ + smp_store_release(&state->initialized, true); +} + #ifdef CONFIG_SECURITY_SELINUX_DEVELOP static inline bool enforcing_enabled(struct selinux_state *state) { - return state->enforcing; + return READ_ONCE(state->enforcing); } static inline void enforcing_set(struct selinux_state *state, bool value) { - state->enforcing = value; + WRITE_ONCE(state->enforcing, value); } #else static inline bool enforcing_enabled(struct selinux_state *state) @@ -138,6 +150,23 @@ static inline void enforcing_set(struct selinux_state *state, bool value) } #endif +#ifdef CONFIG_SECURITY_SELINUX_DISABLE +static inline bool selinux_disabled(struct selinux_state *state) +{ + return READ_ONCE(state->disabled); +} + +static inline void selinux_mark_disabled(struct selinux_state *state) +{ + WRITE_ONCE(state->disabled, true); +} +#else +static inline bool selinux_disabled(struct selinux_state *state) +{ + return false; +} +#endif + static inline bool selinux_policycap_netpeer(void) { struct selinux_state *state = &selinux_state; diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c index 55cf42945cba..0e8b94e8e156 100644 --- a/security/selinux/ss/services.c +++ b/security/selinux/ss/services.c @@ -767,7 +767,7 @@ static int security_compute_validatetrans(struct selinux_state *state, int rc = 0; - if (!state->initialized) + if (!selinux_initialized(state)) return 0; read_lock(&state->ss->policy_rwlock); @@ -868,7 +868,7 @@ int security_bounded_transition(struct selinux_state *state, int index; int rc; - if (!state->initialized) + if (!selinux_initialized(state)) return 0; read_lock(&state->ss->policy_rwlock); @@ -1027,7 +1027,7 @@ void security_compute_xperms_decision(struct selinux_state *state, memset(xpermd->dontaudit->p, 0, sizeof(xpermd->dontaudit->p)); read_lock(&state->ss->policy_rwlock); - if (!state->initialized) + if (!selinux_initialized(state)) goto allow; policydb = &state->ss->policydb; @@ -1112,7 +1112,7 @@ void security_compute_av(struct selinux_state *state, read_lock(&state->ss->policy_rwlock); avd_init(state, avd); xperms->len = 0; - if (!state->initialized) + if (!selinux_initialized(state)) goto allow; policydb = &state->ss->policydb; @@ -1166,7 +1166,7 @@ void security_compute_av_user(struct selinux_state *state, read_lock(&state->ss->policy_rwlock); avd_init(state, avd); - if (!state->initialized) + if (!selinux_initialized(state)) goto allow; policydb = &state->ss->policydb; @@ -1286,7 +1286,7 @@ int security_sidtab_hash_stats(struct selinux_state *state, char *page) { int rc; - if (!state->initialized) { + if (!selinux_initialized(state)) { pr_err("SELinux: %s: called before initial load_policy\n", __func__); return -EINVAL; @@ -1320,7 +1320,7 @@ static int security_sid_to_context_core(struct selinux_state *state, *scontext = NULL; *scontext_len = 0; - if (!state->initialized) { + if (!selinux_initialized(state)) { if (sid <= SECINITSID_NUM) { char *scontextp; @@ -1549,7 +1549,7 @@ static int security_context_to_sid_core(struct selinux_state *state, if (!scontext2) return -ENOMEM; - if (!state->initialized) { + if (!selinux_initialized(state)) { int i; for (i = 1; i < SECINITSID_NUM; i++) { @@ -1736,7 +1736,7 @@ static int security_compute_sid(struct selinux_state *state, int rc = 0; bool sock; - if (!state->initialized) { + if (!selinux_initialized(state)) { switch (orig_tclass) { case SECCLASS_PROCESS: /* kernel value */ *out_sid = ssid; @@ -2198,7 +2198,7 @@ int security_load_policy(struct selinux_state *state, void *data, size_t len) goto out; } - if (!state->initialized) { + if (!selinux_initialized(state)) { rc = policydb_read(policydb, fp); if (rc) { kfree(newsidtab); @@ -2223,7 +2223,7 @@ int security_load_policy(struct selinux_state *state, void *data, size_t len) state->ss->sidtab = newsidtab; security_load_policycaps(state); - state->initialized = 1; + selinux_mark_initialized(state); seqno = ++state->ss->latest_granting; selinux_complete_init(); avc_ss_reset(state->avc, seqno); @@ -2639,7 +2639,7 @@ int security_get_user_sids(struct selinux_state *state, *sids = NULL; *nel = 0; - if (!state->initialized) + if (!selinux_initialized(state)) goto out; read_lock(&state->ss->policy_rwlock); @@ -2875,7 +2875,7 @@ int security_get_bools(struct selinux_state *state, struct policydb *policydb; int i, rc; - if (!state->initialized) { + if (!selinux_initialized(state)) { *len = 0; *names = NULL; *values = NULL; @@ -3050,7 +3050,7 @@ int security_sid_mls_copy(struct selinux_state *state, int rc; rc = 0; - if (!state->initialized || !policydb->mls_enabled) { + if (!selinux_initialized(state) || !policydb->mls_enabled) { *new_sid = sid; goto out; } @@ -3217,7 +3217,7 @@ int security_get_classes(struct selinux_state *state, struct policydb *policydb = &state->ss->policydb; int rc; - if (!state->initialized) { + if (!selinux_initialized(state)) { *nclasses = 0; *classes = NULL; return 0; @@ -3366,7 +3366,7 @@ int selinux_audit_rule_init(u32 field, u32 op, char *rulestr, void **vrule) *rule = NULL; - if (!state->initialized) + if (!selinux_initialized(state)) return -EOPNOTSUPP; switch (field) { @@ -3665,7 +3665,7 @@ int security_netlbl_secattr_to_sid(struct selinux_state *state, struct context *ctx; struct context ctx_new; - if (!state->initialized) { + if (!selinux_initialized(state)) { *sid = SECSID_NULL; return 0; } @@ -3732,7 +3732,7 @@ int security_netlbl_sid_to_secattr(struct selinux_state *state, int rc; struct context *ctx; - if (!state->initialized) + if (!selinux_initialized(state)) return 0; read_lock(&state->ss->policy_rwlock); @@ -3771,7 +3771,7 @@ int security_read_policy(struct selinux_state *state, int rc; struct policy_file fp; - if (!state->initialized) + if (!selinux_initialized(state)) return -EINVAL; *len = security_policydb_len(state); From cfff75d8973ae4a90b3df3ae7fbba1ce9af9c8f0 Mon Sep 17 00:00:00 2001 From: Ondrej Mosnacek Date: Wed, 8 Jan 2020 15:09:58 +0100 Subject: [PATCH 23/28] selinux: reorder hooks to make runtime disable less broken Commit b1d9e6b0646d ("LSM: Switch to lists of hooks") switched the LSM infrastructure to use per-hook lists, which meant that removing the hooks for a given module was no longer atomic. Even though the commit clearly documents that modules implementing runtime revmoval of hooks (only SELinux attempts this madness) need to take special precautions to avoid race conditions, SELinux has never addressed this. By inserting an artificial delay between the loop iterations of security_delete_hooks() (I used 100 ms), booting to a state where SELinux is enabled, but policy is not yet loaded, and running these commands: while true; do ping -c 1 ; done & echo -n 1 >/sys/fs/selinux/disable kill %1 wait ...I was able to trigger NULL pointer dereferences in various places. I also have a report of someone getting panics on a stock RHEL-8 kernel after setting SELINUX=disabled in /etc/selinux/config and rebooting (without adding "selinux=0" to kernel command-line). Reordering the SELinux hooks such that those that allocate structures are removed last seems to prevent these panics. It is very much possible that this doesn't make the runtime disable completely race-free, but at least it makes the operation much less fragile. Cc: stable@vger.kernel.org Fixes: b1d9e6b0646d ("LSM: Switch to lists of hooks") Signed-off-by: Ondrej Mosnacek Reviewed-by: Stephen Smalley Signed-off-by: Paul Moore --- security/selinux/hooks.c | 101 +++++++++++++++++++++++++++------------ 1 file changed, 70 insertions(+), 31 deletions(-) diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index a81631f8cc5d..2c84b12d50bc 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -6892,6 +6892,21 @@ static int selinux_perf_event_write(struct perf_event *event) } #endif +/* + * IMPORTANT NOTE: When adding new hooks, please be careful to keep this order: + * 1. any hooks that don't belong to (2.) or (3.) below, + * 2. hooks that both access structures allocated by other hooks, and allocate + * structures that can be later accessed by other hooks (mostly "cloning" + * hooks), + * 3. hooks that only allocate structures that can be later accessed by other + * hooks ("allocating" hooks). + * + * Please follow block comment delimiters in the list to keep this order. + * + * This ordering is needed for SELinux runtime disable to work at least somewhat + * safely. Breaking the ordering rules above might lead to NULL pointer derefs + * when disabling SELinux at runtime. + */ static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = { LSM_HOOK_INIT(binder_set_context_mgr, selinux_binder_set_context_mgr), LSM_HOOK_INIT(binder_transaction, selinux_binder_transaction), @@ -6914,12 +6929,7 @@ static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = { LSM_HOOK_INIT(bprm_committing_creds, selinux_bprm_committing_creds), LSM_HOOK_INIT(bprm_committed_creds, selinux_bprm_committed_creds), - LSM_HOOK_INIT(fs_context_dup, selinux_fs_context_dup), - LSM_HOOK_INIT(fs_context_parse_param, selinux_fs_context_parse_param), - - LSM_HOOK_INIT(sb_alloc_security, selinux_sb_alloc_security), LSM_HOOK_INIT(sb_free_security, selinux_sb_free_security), - LSM_HOOK_INIT(sb_eat_lsm_opts, selinux_sb_eat_lsm_opts), LSM_HOOK_INIT(sb_free_mnt_opts, selinux_free_mnt_opts), LSM_HOOK_INIT(sb_remount, selinux_sb_remount), LSM_HOOK_INIT(sb_kern_mount, selinux_sb_kern_mount), @@ -6929,12 +6939,10 @@ static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = { LSM_HOOK_INIT(sb_umount, selinux_umount), LSM_HOOK_INIT(sb_set_mnt_opts, selinux_set_mnt_opts), LSM_HOOK_INIT(sb_clone_mnt_opts, selinux_sb_clone_mnt_opts), - LSM_HOOK_INIT(sb_add_mnt_opt, selinux_add_mnt_opt), LSM_HOOK_INIT(dentry_init_security, selinux_dentry_init_security), LSM_HOOK_INIT(dentry_create_files_as, selinux_dentry_create_files_as), - LSM_HOOK_INIT(inode_alloc_security, selinux_inode_alloc_security), LSM_HOOK_INIT(inode_free_security, selinux_inode_free_security), LSM_HOOK_INIT(inode_init_security, selinux_inode_init_security), LSM_HOOK_INIT(inode_create, selinux_inode_create), @@ -7006,21 +7014,15 @@ static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = { LSM_HOOK_INIT(ipc_permission, selinux_ipc_permission), LSM_HOOK_INIT(ipc_getsecid, selinux_ipc_getsecid), - LSM_HOOK_INIT(msg_msg_alloc_security, selinux_msg_msg_alloc_security), - - LSM_HOOK_INIT(msg_queue_alloc_security, - selinux_msg_queue_alloc_security), LSM_HOOK_INIT(msg_queue_associate, selinux_msg_queue_associate), LSM_HOOK_INIT(msg_queue_msgctl, selinux_msg_queue_msgctl), LSM_HOOK_INIT(msg_queue_msgsnd, selinux_msg_queue_msgsnd), LSM_HOOK_INIT(msg_queue_msgrcv, selinux_msg_queue_msgrcv), - LSM_HOOK_INIT(shm_alloc_security, selinux_shm_alloc_security), LSM_HOOK_INIT(shm_associate, selinux_shm_associate), LSM_HOOK_INIT(shm_shmctl, selinux_shm_shmctl), LSM_HOOK_INIT(shm_shmat, selinux_shm_shmat), - LSM_HOOK_INIT(sem_alloc_security, selinux_sem_alloc_security), LSM_HOOK_INIT(sem_associate, selinux_sem_associate), LSM_HOOK_INIT(sem_semctl, selinux_sem_semctl), LSM_HOOK_INIT(sem_semop, selinux_sem_semop), @@ -7031,13 +7033,11 @@ static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = { LSM_HOOK_INIT(setprocattr, selinux_setprocattr), LSM_HOOK_INIT(ismaclabel, selinux_ismaclabel), - LSM_HOOK_INIT(secid_to_secctx, selinux_secid_to_secctx), LSM_HOOK_INIT(secctx_to_secid, selinux_secctx_to_secid), LSM_HOOK_INIT(release_secctx, selinux_release_secctx), LSM_HOOK_INIT(inode_invalidate_secctx, selinux_inode_invalidate_secctx), LSM_HOOK_INIT(inode_notifysecctx, selinux_inode_notifysecctx), LSM_HOOK_INIT(inode_setsecctx, selinux_inode_setsecctx), - LSM_HOOK_INIT(inode_getsecctx, selinux_inode_getsecctx), LSM_HOOK_INIT(unix_stream_connect, selinux_socket_unix_stream_connect), LSM_HOOK_INIT(unix_may_send, selinux_socket_unix_may_send), @@ -7060,7 +7060,6 @@ static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = { LSM_HOOK_INIT(socket_getpeersec_stream, selinux_socket_getpeersec_stream), LSM_HOOK_INIT(socket_getpeersec_dgram, selinux_socket_getpeersec_dgram), - LSM_HOOK_INIT(sk_alloc_security, selinux_sk_alloc_security), LSM_HOOK_INIT(sk_free_security, selinux_sk_free_security), LSM_HOOK_INIT(sk_clone_security, selinux_sk_clone_security), LSM_HOOK_INIT(sk_getsecid, selinux_sk_getsecid), @@ -7075,7 +7074,6 @@ static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = { LSM_HOOK_INIT(secmark_refcount_inc, selinux_secmark_refcount_inc), LSM_HOOK_INIT(secmark_refcount_dec, selinux_secmark_refcount_dec), LSM_HOOK_INIT(req_classify_flow, selinux_req_classify_flow), - LSM_HOOK_INIT(tun_dev_alloc_security, selinux_tun_dev_alloc_security), LSM_HOOK_INIT(tun_dev_free_security, selinux_tun_dev_free_security), LSM_HOOK_INIT(tun_dev_create, selinux_tun_dev_create), LSM_HOOK_INIT(tun_dev_attach_queue, selinux_tun_dev_attach_queue), @@ -7085,17 +7083,11 @@ static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = { LSM_HOOK_INIT(ib_pkey_access, selinux_ib_pkey_access), LSM_HOOK_INIT(ib_endport_manage_subnet, selinux_ib_endport_manage_subnet), - LSM_HOOK_INIT(ib_alloc_security, selinux_ib_alloc_security), LSM_HOOK_INIT(ib_free_security, selinux_ib_free_security), #endif #ifdef CONFIG_SECURITY_NETWORK_XFRM - LSM_HOOK_INIT(xfrm_policy_alloc_security, selinux_xfrm_policy_alloc), - LSM_HOOK_INIT(xfrm_policy_clone_security, selinux_xfrm_policy_clone), LSM_HOOK_INIT(xfrm_policy_free_security, selinux_xfrm_policy_free), LSM_HOOK_INIT(xfrm_policy_delete_security, selinux_xfrm_policy_delete), - LSM_HOOK_INIT(xfrm_state_alloc, selinux_xfrm_state_alloc), - LSM_HOOK_INIT(xfrm_state_alloc_acquire, - selinux_xfrm_state_alloc_acquire), LSM_HOOK_INIT(xfrm_state_free_security, selinux_xfrm_state_free), LSM_HOOK_INIT(xfrm_state_delete_security, selinux_xfrm_state_delete), LSM_HOOK_INIT(xfrm_policy_lookup, selinux_xfrm_policy_lookup), @@ -7105,14 +7097,12 @@ static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = { #endif #ifdef CONFIG_KEYS - LSM_HOOK_INIT(key_alloc, selinux_key_alloc), LSM_HOOK_INIT(key_free, selinux_key_free), LSM_HOOK_INIT(key_permission, selinux_key_permission), LSM_HOOK_INIT(key_getsecurity, selinux_key_getsecurity), #endif #ifdef CONFIG_AUDIT - LSM_HOOK_INIT(audit_rule_init, selinux_audit_rule_init), LSM_HOOK_INIT(audit_rule_known, selinux_audit_rule_known), LSM_HOOK_INIT(audit_rule_match, selinux_audit_rule_match), LSM_HOOK_INIT(audit_rule_free, selinux_audit_rule_free), @@ -7122,21 +7112,66 @@ static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = { LSM_HOOK_INIT(bpf, selinux_bpf), LSM_HOOK_INIT(bpf_map, selinux_bpf_map), LSM_HOOK_INIT(bpf_prog, selinux_bpf_prog), - LSM_HOOK_INIT(bpf_map_alloc_security, selinux_bpf_map_alloc), - LSM_HOOK_INIT(bpf_prog_alloc_security, selinux_bpf_prog_alloc), LSM_HOOK_INIT(bpf_map_free_security, selinux_bpf_map_free), LSM_HOOK_INIT(bpf_prog_free_security, selinux_bpf_prog_free), #endif #ifdef CONFIG_PERF_EVENTS LSM_HOOK_INIT(perf_event_open, selinux_perf_event_open), - LSM_HOOK_INIT(perf_event_alloc, selinux_perf_event_alloc), LSM_HOOK_INIT(perf_event_free, selinux_perf_event_free), LSM_HOOK_INIT(perf_event_read, selinux_perf_event_read), LSM_HOOK_INIT(perf_event_write, selinux_perf_event_write), #endif LSM_HOOK_INIT(locked_down, selinux_lockdown), + + /* + * PUT "CLONING" (ACCESSING + ALLOCATING) HOOKS HERE + */ + LSM_HOOK_INIT(fs_context_dup, selinux_fs_context_dup), + LSM_HOOK_INIT(fs_context_parse_param, selinux_fs_context_parse_param), + LSM_HOOK_INIT(sb_eat_lsm_opts, selinux_sb_eat_lsm_opts), + LSM_HOOK_INIT(sb_add_mnt_opt, selinux_add_mnt_opt), +#ifdef CONFIG_SECURITY_NETWORK_XFRM + LSM_HOOK_INIT(xfrm_policy_clone_security, selinux_xfrm_policy_clone), +#endif + + /* + * PUT "ALLOCATING" HOOKS HERE + */ + LSM_HOOK_INIT(msg_msg_alloc_security, selinux_msg_msg_alloc_security), + LSM_HOOK_INIT(msg_queue_alloc_security, + selinux_msg_queue_alloc_security), + LSM_HOOK_INIT(shm_alloc_security, selinux_shm_alloc_security), + LSM_HOOK_INIT(sb_alloc_security, selinux_sb_alloc_security), + LSM_HOOK_INIT(inode_alloc_security, selinux_inode_alloc_security), + LSM_HOOK_INIT(sem_alloc_security, selinux_sem_alloc_security), + LSM_HOOK_INIT(secid_to_secctx, selinux_secid_to_secctx), + LSM_HOOK_INIT(inode_getsecctx, selinux_inode_getsecctx), + LSM_HOOK_INIT(sk_alloc_security, selinux_sk_alloc_security), + LSM_HOOK_INIT(tun_dev_alloc_security, selinux_tun_dev_alloc_security), +#ifdef CONFIG_SECURITY_INFINIBAND + LSM_HOOK_INIT(ib_alloc_security, selinux_ib_alloc_security), +#endif +#ifdef CONFIG_SECURITY_NETWORK_XFRM + LSM_HOOK_INIT(xfrm_policy_alloc_security, selinux_xfrm_policy_alloc), + LSM_HOOK_INIT(xfrm_state_alloc, selinux_xfrm_state_alloc), + LSM_HOOK_INIT(xfrm_state_alloc_acquire, + selinux_xfrm_state_alloc_acquire), +#endif +#ifdef CONFIG_KEYS + LSM_HOOK_INIT(key_alloc, selinux_key_alloc), +#endif +#ifdef CONFIG_AUDIT + LSM_HOOK_INIT(audit_rule_init, selinux_audit_rule_init), +#endif +#ifdef CONFIG_BPF_SYSCALL + LSM_HOOK_INIT(bpf_map_alloc_security, selinux_bpf_map_alloc), + LSM_HOOK_INIT(bpf_prog_alloc_security, selinux_bpf_prog_alloc), +#endif +#ifdef CONFIG_PERF_EVENTS + LSM_HOOK_INIT(perf_event_alloc, selinux_perf_event_alloc), +#endif }; static __init int selinux_init(void) @@ -7315,14 +7350,18 @@ int selinux_disable(struct selinux_state *state) pr_info("SELinux: Disabled at runtime.\n"); + /* + * Unregister netfilter hooks. + * Must be done before security_delete_hooks() to avoid breaking + * runtime disable. + */ + selinux_nf_ip_exit(); + security_delete_hooks(selinux_hooks, ARRAY_SIZE(selinux_hooks)); /* Try to destroy the avc node cache */ avc_disable(); - /* Unregister netfilter hooks. */ - selinux_nf_ip_exit(); - /* Unregister selinuxfs. */ exit_sel_fs(); From ae3d8c2e27bf95dbd2977f247123f88955a761db Mon Sep 17 00:00:00 2001 From: Ondrej Mosnacek Date: Thu, 16 Jan 2020 19:45:57 +0100 Subject: [PATCH 24/28] selinux: fix wrong buffer types in policydb.c Two places used u32 where there should have been __le32. Fixes sparse warnings: CHECK [...]/security/selinux/ss/services.c [...]/security/selinux/ss/policydb.c:2669:16: warning: incorrect type in assignment (different base types) [...]/security/selinux/ss/policydb.c:2669:16: expected unsigned int [...]/security/selinux/ss/policydb.c:2669:16: got restricted __le32 [usertype] [...]/security/selinux/ss/policydb.c:2674:24: warning: incorrect type in assignment (different base types) [...]/security/selinux/ss/policydb.c:2674:24: expected unsigned int [...]/security/selinux/ss/policydb.c:2674:24: got restricted __le32 [usertype] [...]/security/selinux/ss/policydb.c:2675:24: warning: incorrect type in assignment (different base types) [...]/security/selinux/ss/policydb.c:2675:24: expected unsigned int [...]/security/selinux/ss/policydb.c:2675:24: got restricted __le32 [usertype] [...]/security/selinux/ss/policydb.c:2676:24: warning: incorrect type in assignment (different base types) [...]/security/selinux/ss/policydb.c:2676:24: expected unsigned int [...]/security/selinux/ss/policydb.c:2676:24: got restricted __le32 [usertype] [...]/security/selinux/ss/policydb.c:2681:32: warning: incorrect type in assignment (different base types) [...]/security/selinux/ss/policydb.c:2681:32: expected unsigned int [...]/security/selinux/ss/policydb.c:2681:32: got restricted __le32 [usertype] [...]/security/selinux/ss/policydb.c:2701:16: warning: incorrect type in assignment (different base types) [...]/security/selinux/ss/policydb.c:2701:16: expected unsigned int [...]/security/selinux/ss/policydb.c:2701:16: got restricted __le32 [usertype] [...]/security/selinux/ss/policydb.c:2706:24: warning: incorrect type in assignment (different base types) [...]/security/selinux/ss/policydb.c:2706:24: expected unsigned int [...]/security/selinux/ss/policydb.c:2706:24: got restricted __le32 [usertype] [...]/security/selinux/ss/policydb.c:2707:24: warning: incorrect type in assignment (different base types) [...]/security/selinux/ss/policydb.c:2707:24: expected unsigned int [...]/security/selinux/ss/policydb.c:2707:24: got restricted __le32 [usertype] Signed-off-by: Ondrej Mosnacek Reviewed-by: Stephen Smalley Signed-off-by: Paul Moore --- security/selinux/ss/policydb.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c index e369b0092cdf..2aa7f2e1a8e7 100644 --- a/security/selinux/ss/policydb.c +++ b/security/selinux/ss/policydb.c @@ -2659,7 +2659,7 @@ static int role_trans_write(struct policydb *p, void *fp) { struct role_trans *r = p->role_tr; struct role_trans *tr; - u32 buf[3]; + __le32 buf[3]; size_t nel; int rc; @@ -2691,7 +2691,7 @@ static int role_trans_write(struct policydb *p, void *fp) static int role_allow_write(struct role_allow *r, void *fp) { struct role_allow *ra; - u32 buf[2]; + __le32 buf[2]; size_t nel; int rc; From df4779b5d2732eb7550fe35249e82db1b3d4fb95 Mon Sep 17 00:00:00 2001 From: Huaisheng Ye Date: Mon, 13 Jan 2020 23:03:31 +0800 Subject: [PATCH 25/28] selinux: remove redundant selinux_nlmsg_perm selinux_nlmsg_perm is used for only by selinux_netlink_send. Remove the redundant function to simplify the code. Fix a typo by suggestion from Stephen. Signed-off-by: Huaisheng Ye Acked-by: Stephen Smalley Signed-off-by: Paul Moore --- security/selinux/hooks.c | 73 +++++++++++++++++++--------------------- 1 file changed, 34 insertions(+), 39 deletions(-) diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 2c84b12d50bc..2d5352d35b5d 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -5520,44 +5520,6 @@ static int selinux_tun_dev_open(void *security) return 0; } -static int selinux_nlmsg_perm(struct sock *sk, struct sk_buff *skb) -{ - int err = 0; - u32 perm; - struct nlmsghdr *nlh; - struct sk_security_struct *sksec = sk->sk_security; - - if (skb->len < NLMSG_HDRLEN) { - err = -EINVAL; - goto out; - } - nlh = nlmsg_hdr(skb); - - err = selinux_nlmsg_lookup(sksec->sclass, nlh->nlmsg_type, &perm); - if (err) { - if (err == -EINVAL) { - pr_warn_ratelimited("SELinux: unrecognized netlink" - " message: protocol=%hu nlmsg_type=%hu sclass=%s" - " pig=%d comm=%s\n", - sk->sk_protocol, nlh->nlmsg_type, - secclass_map[sksec->sclass - 1].name, - task_pid_nr(current), current->comm); - if (!enforcing_enabled(&selinux_state) || - security_get_allow_unknown(&selinux_state)) - err = 0; - } - - /* Ignore */ - if (err == -ENOENT) - err = 0; - goto out; - } - - err = sock_has_perm(sk, perm); -out: - return err; -} - #ifdef CONFIG_NETFILTER static unsigned int selinux_ip_forward(struct sk_buff *skb, @@ -5886,7 +5848,40 @@ static unsigned int selinux_ipv6_postroute(void *priv, static int selinux_netlink_send(struct sock *sk, struct sk_buff *skb) { - return selinux_nlmsg_perm(sk, skb); + int err = 0; + u32 perm; + struct nlmsghdr *nlh; + struct sk_security_struct *sksec = sk->sk_security; + + if (skb->len < NLMSG_HDRLEN) { + err = -EINVAL; + goto out; + } + nlh = nlmsg_hdr(skb); + + err = selinux_nlmsg_lookup(sksec->sclass, nlh->nlmsg_type, &perm); + if (err) { + if (err == -EINVAL) { + pr_warn_ratelimited("SELinux: unrecognized netlink" + " message: protocol=%hu nlmsg_type=%hu sclass=%s" + " pid=%d comm=%s\n", + sk->sk_protocol, nlh->nlmsg_type, + secclass_map[sksec->sclass - 1].name, + task_pid_nr(current), current->comm); + if (!enforcing_enabled(&selinux_state) || + security_get_allow_unknown(&selinux_state)) + err = 0; + } + + /* Ignore */ + if (err == -ENOENT) + err = 0; + goto out; + } + + err = sock_has_perm(sk, perm); +out: + return err; } static void ipc_init_security(struct ipc_security_struct *isec, u16 sclass) From cb89e2465896f30d4247ac9ff47d30522e39745a Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Fri, 10 Jan 2020 16:32:10 -0500 Subject: [PATCH 26/28] selinux: remove redundant allocation and helper functions This patch removes the inode, file, and superblock security blob allocation functions and moves the associated code into the respective LSM hooks. This patch also removes the inode_doinit() function as it was a trivial wrapper around inode_doinit_with_dentry() and called from one location in the code. Reviewed-by: Casey Schaufler Acked-by: Stephen Smalley Signed-off-by: Paul Moore --- security/selinux/hooks.c | 94 +++++++++++++++------------------------- 1 file changed, 36 insertions(+), 58 deletions(-) diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 2d5352d35b5d..61085eb3cd24 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -238,24 +238,6 @@ static inline u32 task_sid(const struct task_struct *task) return sid; } -/* Allocate and free functions for each kind of security blob. */ - -static int inode_alloc_security(struct inode *inode) -{ - struct inode_security_struct *isec = selinux_inode(inode); - u32 sid = current_sid(); - - spin_lock_init(&isec->lock); - INIT_LIST_HEAD(&isec->list); - isec->inode = inode; - isec->sid = SECINITSID_UNLABELED; - isec->sclass = SECCLASS_FILE; - isec->task_sid = sid; - isec->initialized = LABEL_INVALID; - - return 0; -} - static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dentry); /* @@ -354,37 +336,6 @@ static void inode_free_security(struct inode *inode) } } -static int file_alloc_security(struct file *file) -{ - struct file_security_struct *fsec = selinux_file(file); - u32 sid = current_sid(); - - fsec->sid = sid; - fsec->fown_sid = sid; - - return 0; -} - -static int superblock_alloc_security(struct super_block *sb) -{ - struct superblock_security_struct *sbsec; - - sbsec = kzalloc(sizeof(struct superblock_security_struct), GFP_KERNEL); - if (!sbsec) - return -ENOMEM; - - mutex_init(&sbsec->lock); - INIT_LIST_HEAD(&sbsec->isec_head); - spin_lock_init(&sbsec->isec_lock); - sbsec->sb = sb; - sbsec->sid = SECINITSID_UNLABELED; - sbsec->def_sid = SECINITSID_FILE; - sbsec->mntpoint_sid = SECINITSID_UNLABELED; - sb->s_security = sbsec; - - return 0; -} - static void superblock_free_security(struct super_block *sb) { struct superblock_security_struct *sbsec = sb->s_security; @@ -406,11 +357,6 @@ static void selinux_free_mnt_opts(void *mnt_opts) kfree(opts); } -static inline int inode_doinit(struct inode *inode) -{ - return inode_doinit_with_dentry(inode, NULL); -} - enum { Opt_error = -1, Opt_context = 0, @@ -598,7 +544,7 @@ static int sb_finish_set_opts(struct super_block *sb) inode = igrab(inode); if (inode) { if (!IS_PRIVATE(inode)) - inode_doinit(inode); + inode_doinit_with_dentry(inode, NULL); iput(inode); } spin_lock(&sbsec->isec_lock); @@ -2593,7 +2539,22 @@ static void selinux_bprm_committed_creds(struct linux_binprm *bprm) static int selinux_sb_alloc_security(struct super_block *sb) { - return superblock_alloc_security(sb); + struct superblock_security_struct *sbsec; + + sbsec = kzalloc(sizeof(struct superblock_security_struct), GFP_KERNEL); + if (!sbsec) + return -ENOMEM; + + mutex_init(&sbsec->lock); + INIT_LIST_HEAD(&sbsec->isec_head); + spin_lock_init(&sbsec->isec_lock); + sbsec->sb = sb; + sbsec->sid = SECINITSID_UNLABELED; + sbsec->def_sid = SECINITSID_FILE; + sbsec->mntpoint_sid = SECINITSID_UNLABELED; + sb->s_security = sbsec; + + return 0; } static void selinux_sb_free_security(struct super_block *sb) @@ -2845,7 +2806,18 @@ static int selinux_fs_context_parse_param(struct fs_context *fc, static int selinux_inode_alloc_security(struct inode *inode) { - return inode_alloc_security(inode); + struct inode_security_struct *isec = selinux_inode(inode); + u32 sid = current_sid(); + + spin_lock_init(&isec->lock); + INIT_LIST_HEAD(&isec->list); + isec->inode = inode; + isec->sid = SECINITSID_UNLABELED; + isec->sclass = SECCLASS_FILE; + isec->task_sid = sid; + isec->initialized = LABEL_INVALID; + + return 0; } static void selinux_inode_free_security(struct inode *inode) @@ -3555,7 +3527,13 @@ static int selinux_file_permission(struct file *file, int mask) static int selinux_file_alloc_security(struct file *file) { - return file_alloc_security(file); + struct file_security_struct *fsec = selinux_file(file); + u32 sid = current_sid(); + + fsec->sid = sid; + fsec->fown_sid = sid; + + return 0; } /* From dd89b9d9f37decab85e000384d229abdcd9944ae Mon Sep 17 00:00:00 2001 From: Ondrej Mosnacek Date: Thu, 16 Jan 2020 13:04:34 +0100 Subject: [PATCH 27/28] selinux: do not allocate ancillary buffer on first load In security_load_policy(), we can defer allocating the newpolicydb ancillary array to after checking state->initialized, thereby avoiding the pointless allocation when loading policy the first time. Signed-off-by: Ondrej Mosnacek [PM: merged portions by hand] Reviewed-by: Stephen Smalley Signed-off-by: Paul Moore --- security/selinux/ss/services.c | 28 +++++++++++++--------------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c index 0e8b94e8e156..216ce602a2b5 100644 --- a/security/selinux/ss/services.c +++ b/security/selinux/ss/services.c @@ -2183,26 +2183,17 @@ int security_load_policy(struct selinux_state *state, void *data, size_t len) int rc = 0; struct policy_file file = { data, len }, *fp = &file; - oldpolicydb = kcalloc(2, sizeof(*oldpolicydb), GFP_KERNEL); - if (!oldpolicydb) { - rc = -ENOMEM; - goto out; - } - newpolicydb = oldpolicydb + 1; - policydb = &state->ss->policydb; newsidtab = kmalloc(sizeof(*newsidtab), GFP_KERNEL); - if (!newsidtab) { - rc = -ENOMEM; - goto out; - } + if (!newsidtab) + return -ENOMEM; if (!selinux_initialized(state)) { rc = policydb_read(policydb, fp); if (rc) { kfree(newsidtab); - goto out; + return rc; } policydb->len = len; @@ -2211,14 +2202,14 @@ int security_load_policy(struct selinux_state *state, void *data, size_t len) if (rc) { kfree(newsidtab); policydb_destroy(policydb); - goto out; + return rc; } rc = policydb_load_isids(policydb, newsidtab); if (rc) { kfree(newsidtab); policydb_destroy(policydb); - goto out; + return rc; } state->ss->sidtab = newsidtab; @@ -2231,9 +2222,16 @@ int security_load_policy(struct selinux_state *state, void *data, size_t len) selinux_status_update_policyload(state, seqno); selinux_netlbl_cache_invalidate(); selinux_xfrm_notify_policyload(); - goto out; + return 0; } + oldpolicydb = kcalloc(2, sizeof(*oldpolicydb), GFP_KERNEL); + if (!oldpolicydb) { + kfree(newsidtab); + return -ENOMEM; + } + newpolicydb = oldpolicydb + 1; + rc = policydb_read(newpolicydb, fp); if (rc) { kfree(newsidtab); From 98aa00345de54b8340dc2ddcd87f446d33387b5e Mon Sep 17 00:00:00 2001 From: Stephen Smalley Date: Fri, 17 Jan 2020 15:24:07 -0500 Subject: [PATCH 28/28] selinux: fix regression introduced by move_mount(2) syscall commit 2db154b3ea8e ("vfs: syscall: Add move_mount(2) to move mounts around") introduced a new move_mount(2) system call and a corresponding new LSM security_move_mount hook but did not implement this hook for any existing LSM. This creates a regression for SELinux with respect to consistent checking of mounts; the existing selinux_mount hook checks mounton permission to the mount point path. Provide a SELinux hook implementation for move_mount that applies this same check for consistency. In the future we may wish to add a new move_mount filesystem permission and check as well, but this addresses the immediate regression. Fixes: 2db154b3ea8e ("vfs: syscall: Add move_mount(2) to move mounts around") Signed-off-by: Stephen Smalley Reviewed-by: Ondrej Mosnacek Signed-off-by: Paul Moore --- security/selinux/hooks.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 61085eb3cd24..d9e8b2131a65 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -2724,6 +2724,14 @@ static int selinux_mount(const char *dev_name, return path_has_perm(cred, path, FILE__MOUNTON); } +static int selinux_move_mount(const struct path *from_path, + const struct path *to_path) +{ + const struct cred *cred = current_cred(); + + return path_has_perm(cred, to_path, FILE__MOUNTON); +} + static int selinux_umount(struct vfsmount *mnt, int flags) { const struct cred *cred = current_cred(); @@ -6913,6 +6921,8 @@ static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = { LSM_HOOK_INIT(sb_set_mnt_opts, selinux_set_mnt_opts), LSM_HOOK_INIT(sb_clone_mnt_opts, selinux_sb_clone_mnt_opts), + LSM_HOOK_INIT(move_mount, selinux_move_mount), + LSM_HOOK_INIT(dentry_init_security, selinux_dentry_init_security), LSM_HOOK_INIT(dentry_create_files_as, selinux_dentry_create_files_as),