From 4b8503967ef5d1123d6e0a87d5723bdaeddf8b3f Mon Sep 17 00:00:00 2001 From: Zou Wei Date: Tue, 14 Apr 2020 16:18:07 +0800 Subject: [PATCH 01/12] selinux: fix warning Comparison to bool fix below warnings reported by coccicheck security/selinux/ss/mls.c:539:39-43: WARNING: Comparison to bool security/selinux/ss/services.c:1815:46-50: WARNING: Comparison to bool security/selinux/ss/services.c:1827:46-50: WARNING: Comparison to bool Reported-by: Hulk Robot Signed-off-by: Zou Wei Signed-off-by: Paul Moore --- security/selinux/ss/mls.c | 2 +- security/selinux/ss/services.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/security/selinux/ss/mls.c b/security/selinux/ss/mls.c index ec5e3d1da9ac..6a5d7d08933d 100644 --- a/security/selinux/ss/mls.c +++ b/security/selinux/ss/mls.c @@ -536,7 +536,7 @@ int mls_compute_sid(struct policydb *p, /* Fallthrough */ case AVTAB_CHANGE: - if ((tclass == p->process_class) || (sock == true)) + if ((tclass == p->process_class) || sock) /* Use the process MLS attributes. */ return mls_context_cpy(newcontext, scontext); else diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c index 8ad34fd031d1..3b592d17d2d3 100644 --- a/security/selinux/ss/services.c +++ b/security/selinux/ss/services.c @@ -1812,7 +1812,7 @@ static int security_compute_sid(struct selinux_state *state, } else if (cladatum && cladatum->default_role == DEFAULT_TARGET) { newcontext.role = tcontext->role; } else { - if ((tclass == policydb->process_class) || (sock == true)) + if ((tclass == policydb->process_class) || sock) newcontext.role = scontext->role; else newcontext.role = OBJECT_R_VAL; @@ -1824,7 +1824,7 @@ static int security_compute_sid(struct selinux_state *state, } else if (cladatum && cladatum->default_type == DEFAULT_TARGET) { newcontext.type = tcontext->type; } else { - if ((tclass == policydb->process_class) || (sock == true)) { + if ((tclass == policydb->process_class) || sock) { /* Use the type of process. */ newcontext.type = scontext->type; } else { From 433e3aa37773e8a36858b9417c3e345eff79a079 Mon Sep 17 00:00:00 2001 From: Ondrej Mosnacek Date: Wed, 8 Apr 2020 11:08:08 +0200 Subject: [PATCH 02/12] selinux: drop unnecessary smp_load_acquire() call In commit 66f8e2f03c02 ("selinux: sidtab reverse lookup hash table") the corresponding load is moved under the spin lock, so there is no race possible and we can read the count directly. The smp_store_release() is still needed to avoid racing with the lock-free readers. Signed-off-by: Ondrej Mosnacek Signed-off-by: Paul Moore --- security/selinux/ss/sidtab.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/security/selinux/ss/sidtab.c b/security/selinux/ss/sidtab.c index f511ffccb131..98d5ea3fcde4 100644 --- a/security/selinux/ss/sidtab.c +++ b/security/selinux/ss/sidtab.c @@ -276,8 +276,7 @@ int sidtab_context_to_sid(struct sidtab *s, struct context *context, if (*sid) goto out_unlock; - /* read entries only after reading count */ - count = smp_load_acquire(&s->count); + count = s->count; convert = s->convert; /* bail out if we already reached max entries */ From e67b2ec9f6171895e774f6543626913960e019df Mon Sep 17 00:00:00 2001 From: Ondrej Mosnacek Date: Tue, 7 Apr 2020 20:28:58 +0200 Subject: [PATCH 03/12] selinux: store role transitions in a hash table Currently, they are stored in a linked list, which adds significant overhead to security_transition_sid(). On Fedora, with 428 role transitions in policy, converting this list to a hash table cuts down its run time by about 50%. This was measured by running 'stress-ng --msg 1 --msg-ops 100000' under perf with and without this patch. Signed-off-by: Ondrej Mosnacek Signed-off-by: Paul Moore --- security/selinux/ss/policydb.c | 142 ++++++++++++++++++++++----------- security/selinux/ss/policydb.h | 8 +- security/selinux/ss/services.c | 21 +++-- 3 files changed, 109 insertions(+), 62 deletions(-) diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c index 70ecdc78efbd..4f0cfffd008d 100644 --- a/security/selinux/ss/policydb.c +++ b/security/selinux/ss/policydb.c @@ -352,6 +352,13 @@ static int range_tr_destroy(void *key, void *datum, void *p) return 0; } +static int role_tr_destroy(void *key, void *datum, void *p) +{ + kfree(key); + kfree(datum); + return 0; +} + static void ocontext_destroy(struct ocontext *c, int i) { if (!c) @@ -458,6 +465,30 @@ static int rangetr_cmp(struct hashtab *h, const void *k1, const void *k2) return v; } +static u32 role_trans_hash(struct hashtab *h, const void *k) +{ + const struct role_trans_key *key = k; + + return (key->role + (key->type << 3) + (key->tclass << 5)) & + (h->size - 1); +} + +static int role_trans_cmp(struct hashtab *h, const void *k1, const void *k2) +{ + const struct role_trans_key *key1 = k1, *key2 = k2; + int v; + + v = key1->role - key2->role; + if (v) + return v; + + v = key1->type - key2->type; + if (v) + return v; + + return key1->tclass - key2->tclass; +} + /* * Initialize a policy database structure. */ @@ -728,7 +759,6 @@ void policydb_destroy(struct policydb *p) struct genfs *g, *gtmp; int i; struct role_allow *ra, *lra = NULL; - struct role_trans *tr, *ltr = NULL; for (i = 0; i < SYM_NUM; i++) { cond_resched(); @@ -775,12 +805,8 @@ void policydb_destroy(struct policydb *p) cond_policydb_destroy(p); - for (tr = p->role_tr; tr; tr = tr->next) { - cond_resched(); - kfree(ltr); - ltr = tr; - } - kfree(ltr); + hashtab_map(p->role_tr, role_tr_destroy, NULL); + hashtab_destroy(p->role_tr); for (ra = p->role_allow; ra; ra = ra->next) { cond_resched(); @@ -2251,7 +2277,8 @@ out: int policydb_read(struct policydb *p, void *fp) { struct role_allow *ra, *lra; - struct role_trans *tr, *ltr; + struct role_trans_key *rtk = NULL; + struct role_trans_datum *rtd = NULL; int i, j, rc; __le32 buf[4]; u32 len, nprim, nel; @@ -2416,39 +2443,50 @@ int policydb_read(struct policydb *p, void *fp) if (rc) goto bad; nel = le32_to_cpu(buf[0]); - ltr = NULL; + + p->role_tr = hashtab_create(role_trans_hash, role_trans_cmp, nel); + if (!p->role_tr) + goto bad; for (i = 0; i < nel; i++) { rc = -ENOMEM; - tr = kzalloc(sizeof(*tr), GFP_KERNEL); - if (!tr) + rtk = kmalloc(sizeof(*rtk), GFP_KERNEL); + if (!rtk) goto bad; - if (ltr) - ltr->next = tr; - else - p->role_tr = tr; + + rc = -ENOMEM; + rtd = kmalloc(sizeof(*rtd), GFP_KERNEL); + if (!rtd) + goto bad; + rc = next_entry(buf, fp, sizeof(u32)*3); if (rc) goto bad; rc = -EINVAL; - tr->role = le32_to_cpu(buf[0]); - tr->type = le32_to_cpu(buf[1]); - tr->new_role = le32_to_cpu(buf[2]); + rtk->role = le32_to_cpu(buf[0]); + rtk->type = le32_to_cpu(buf[1]); + rtd->new_role = le32_to_cpu(buf[2]); if (p->policyvers >= POLICYDB_VERSION_ROLETRANS) { rc = next_entry(buf, fp, sizeof(u32)); if (rc) goto bad; - tr->tclass = le32_to_cpu(buf[0]); + rtk->tclass = le32_to_cpu(buf[0]); } else - tr->tclass = p->process_class; + rtk->tclass = p->process_class; rc = -EINVAL; - if (!policydb_role_isvalid(p, tr->role) || - !policydb_type_isvalid(p, tr->type) || - !policydb_class_isvalid(p, tr->tclass) || - !policydb_role_isvalid(p, tr->new_role)) + if (!policydb_role_isvalid(p, rtk->role) || + !policydb_type_isvalid(p, rtk->type) || + !policydb_class_isvalid(p, rtk->tclass) || + !policydb_role_isvalid(p, rtd->new_role)) goto bad; - ltr = tr; + + rc = hashtab_insert(p->role_tr, rtk, rtd); + if (rc) + goto bad; + + rtk = NULL; + rtd = NULL; } rc = next_entry(buf, fp, sizeof(u32)); @@ -2536,6 +2574,8 @@ int policydb_read(struct policydb *p, void *fp) out: return rc; bad: + kfree(rtk); + kfree(rtd); policydb_destroy(p); goto out; } @@ -2653,37 +2693,43 @@ static int cat_write(void *vkey, void *datum, void *ptr) return 0; } -static int role_trans_write(struct policydb *p, void *fp) +static int role_trans_write_one(void *key, void *datum, void *ptr) { - struct role_trans *r = p->role_tr; - struct role_trans *tr; + struct role_trans_key *rtk = key; + struct role_trans_datum *rtd = datum; + struct policy_data *pd = ptr; + void *fp = pd->fp; + struct policydb *p = pd->p; __le32 buf[3]; - size_t nel; int rc; - nel = 0; - for (tr = r; tr; tr = tr->next) - nel++; - buf[0] = cpu_to_le32(nel); + buf[0] = cpu_to_le32(rtk->role); + buf[1] = cpu_to_le32(rtk->type); + buf[2] = cpu_to_le32(rtd->new_role); + rc = put_entry(buf, sizeof(u32), 3, fp); + if (rc) + return rc; + if (p->policyvers >= POLICYDB_VERSION_ROLETRANS) { + buf[0] = cpu_to_le32(rtk->tclass); + rc = put_entry(buf, sizeof(u32), 1, fp); + if (rc) + return rc; + } + return 0; +} + +static int role_trans_write(struct policydb *p, void *fp) +{ + struct policy_data pd = { .p = p, .fp = fp }; + __le32 buf[1]; + int rc; + + buf[0] = cpu_to_le32(p->role_tr->nel); rc = put_entry(buf, sizeof(u32), 1, fp); if (rc) return rc; - for (tr = r; tr; tr = tr->next) { - buf[0] = cpu_to_le32(tr->role); - buf[1] = cpu_to_le32(tr->type); - buf[2] = cpu_to_le32(tr->new_role); - rc = put_entry(buf, sizeof(u32), 3, fp); - if (rc) - return rc; - if (p->policyvers >= POLICYDB_VERSION_ROLETRANS) { - buf[0] = cpu_to_le32(tr->tclass); - rc = put_entry(buf, sizeof(u32), 1, fp); - if (rc) - return rc; - } - } - return 0; + return hashtab_map(p->role_tr, role_trans_write_one, &pd); } static int role_allow_write(struct role_allow *r, void *fp) diff --git a/security/selinux/ss/policydb.h b/security/selinux/ss/policydb.h index 72e2932fb12d..d3adb522d3f3 100644 --- a/security/selinux/ss/policydb.h +++ b/security/selinux/ss/policydb.h @@ -81,12 +81,14 @@ struct role_datum { struct ebitmap types; /* set of authorized types for role */ }; -struct role_trans { +struct role_trans_key { u32 role; /* current role */ u32 type; /* program executable type, or new object type */ u32 tclass; /* process class, or new object class */ +}; + +struct role_trans_datum { u32 new_role; /* new role */ - struct role_trans *next; }; struct filename_trans_key { @@ -261,7 +263,7 @@ struct policydb { struct avtab te_avtab; /* role transitions */ - struct role_trans *role_tr; + struct hashtab *role_tr; /* file transitions with the last path component */ /* quickly exclude lookups when parent ttype has no rules */ diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c index 3b592d17d2d3..07cdda2ff49c 100644 --- a/security/selinux/ss/services.c +++ b/security/selinux/ss/services.c @@ -1731,7 +1731,6 @@ static int security_compute_sid(struct selinux_state *state, struct class_datum *cladatum = NULL; struct context *scontext, *tcontext, newcontext; struct sidtab_entry *sentry, *tentry; - struct role_trans *roletr = NULL; struct avtab_key avkey; struct avtab_datum *avdatum; struct avtab_node *node; @@ -1864,16 +1863,16 @@ static int security_compute_sid(struct selinux_state *state, /* Check for class-specific changes. */ if (specified & AVTAB_TRANSITION) { /* Look for a role transition rule. */ - for (roletr = policydb->role_tr; roletr; - roletr = roletr->next) { - if ((roletr->role == scontext->role) && - (roletr->type == tcontext->type) && - (roletr->tclass == tclass)) { - /* Use the role transition rule. */ - newcontext.role = roletr->new_role; - break; - } - } + struct role_trans_datum *rtd; + struct role_trans_key rtk = { + .role = scontext->role, + .type = tcontext->type, + .tclass = tclass, + }; + + rtd = hashtab_search(policydb->role_tr, &rtk); + if (rtd) + newcontext.role = rtd->new_role; } /* Set the MLS attributes. From 50077289804c9bd4e6cfd5b3a10d4da0487f7e42 Mon Sep 17 00:00:00 2001 From: Ondrej Mosnacek Date: Fri, 17 Apr 2020 10:11:56 +0200 Subject: [PATCH 04/12] selinux: hash context structure directly Always hashing the string representation is inefficient. Just hash the contents of the structure directly (using jhash). If the context is invalid (str & len are set), then hash the string as before, otherwise hash the structured data. Since the context hashing function is now faster (about 10 times), this patch decreases the overhead of security_transition_sid(), which is called from many hooks. The jhash function seemed as a good choice, since it is used as the default hashing algorithm in rhashtable. Signed-off-by: Ondrej Mosnacek Reviewed-by: Jeff Vander Stoep Tested-by: Jeff Vander Stoep [PM: fixed some spelling errors in the comments pointed out by JVS] Signed-off-by: Paul Moore --- security/selinux/Makefile | 2 +- security/selinux/ss/context.c | 32 +++++++++++++++++++++++++++++++ security/selinux/ss/context.h | 6 ++++-- security/selinux/ss/ebitmap.c | 14 ++++++++++++++ security/selinux/ss/ebitmap.h | 1 + security/selinux/ss/mls.h | 11 +++++++++++ security/selinux/ss/policydb.c | 7 ++----- security/selinux/ss/services.c | 35 ++++------------------------------ security/selinux/ss/services.h | 3 --- 9 files changed, 69 insertions(+), 42 deletions(-) create mode 100644 security/selinux/ss/context.c diff --git a/security/selinux/Makefile b/security/selinux/Makefile index 0c77ede1cc11..4d8e0e8adf0b 100644 --- a/security/selinux/Makefile +++ b/security/selinux/Makefile @@ -8,7 +8,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 status.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/policydb.o ss/services.o ss/conditional.o ss/mls.o ss/context.o selinux-$(CONFIG_SECURITY_NETWORK_XFRM) += xfrm.o diff --git a/security/selinux/ss/context.c b/security/selinux/ss/context.c new file mode 100644 index 000000000000..38bc0aa524a6 --- /dev/null +++ b/security/selinux/ss/context.c @@ -0,0 +1,32 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Implementations of the security context functions. + * + * Author: Ondrej Mosnacek + * Copyright (C) 2020 Red Hat, Inc. + */ + +#include + +#include "context.h" +#include "mls.h" + +u32 context_compute_hash(const struct context *c) +{ + u32 hash = 0; + + /* + * If a context is invalid, it will always be represented by a + * context struct with only the len & str set (and vice versa) + * under a given policy. Since context structs from different + * policies should never meet, it is safe to hash valid and + * invalid contexts differently. The context_cmp() function + * already operates under the same assumption. + */ + if (c->len) + return full_name_hash(NULL, c->str, c->len); + + hash = jhash_3words(c->user, c->role, c->type, hash); + hash = mls_range_hash(&c->range, hash); + return hash; +} diff --git a/security/selinux/ss/context.h b/security/selinux/ss/context.h index 3ba044fe02ed..e7ae7e21449b 100644 --- a/security/selinux/ss/context.h +++ b/security/selinux/ss/context.h @@ -196,9 +196,11 @@ 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) +u32 context_compute_hash(const struct context *c); + +static inline void context_add_hash(struct context *context) { - return full_name_hash(NULL, s, strlen(s)); + context->hash = context_compute_hash(context); } #endif /* _SS_CONTEXT_H_ */ diff --git a/security/selinux/ss/ebitmap.c b/security/selinux/ss/ebitmap.c index c8c3663111e2..14bedc95c6dc 100644 --- a/security/selinux/ss/ebitmap.c +++ b/security/selinux/ss/ebitmap.c @@ -19,6 +19,7 @@ #include #include #include +#include #include #include "ebitmap.h" #include "policydb.h" @@ -542,6 +543,19 @@ int ebitmap_write(struct ebitmap *e, void *fp) return 0; } +u32 ebitmap_hash(const struct ebitmap *e, u32 hash) +{ + struct ebitmap_node *node; + + /* need to change hash even if ebitmap is empty */ + hash = jhash_1word(e->highbit, hash); + for (node = e->node; node; node = node->next) { + hash = jhash_1word(node->startbit, hash); + hash = jhash(node->maps, sizeof(node->maps), hash); + } + return hash; +} + void __init ebitmap_cache_init(void) { ebitmap_node_cachep = kmem_cache_create("ebitmap_node", diff --git a/security/selinux/ss/ebitmap.h b/security/selinux/ss/ebitmap.h index 9a23b81b8832..9eb2d0af2805 100644 --- a/security/selinux/ss/ebitmap.h +++ b/security/selinux/ss/ebitmap.h @@ -131,6 +131,7 @@ int ebitmap_set_bit(struct ebitmap *e, unsigned long bit, int value); void ebitmap_destroy(struct ebitmap *e); int ebitmap_read(struct ebitmap *e, void *fp); int ebitmap_write(struct ebitmap *e, void *fp); +u32 ebitmap_hash(const struct ebitmap *e, u32 hash); #ifdef CONFIG_NETLABEL int ebitmap_netlbl_export(struct ebitmap *ebmap, diff --git a/security/selinux/ss/mls.h b/security/selinux/ss/mls.h index 7954b1e60b64..15cacde0ff61 100644 --- a/security/selinux/ss/mls.h +++ b/security/selinux/ss/mls.h @@ -22,7 +22,10 @@ #ifndef _SS_MLS_H_ #define _SS_MLS_H_ +#include + #include "context.h" +#include "ebitmap.h" #include "policydb.h" int mls_compute_context_len(struct policydb *p, struct context *context); @@ -101,5 +104,13 @@ static inline int mls_import_netlbl_cat(struct policydb *p, } #endif +static inline u32 mls_range_hash(const struct mls_range *r, u32 hash) +{ + hash = jhash_2words(r->level[0].sens, r->level[1].sens, hash); + hash = ebitmap_hash(&r->level[0].cat, hash); + hash = ebitmap_hash(&r->level[1].cat, hash); + return hash; +} + #endif /* _SS_MLS_H */ diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c index 4f0cfffd008d..2849bc362828 100644 --- a/security/selinux/ss/policydb.c +++ b/security/selinux/ss/policydb.c @@ -862,11 +862,8 @@ int policydb_load_isids(struct policydb *p, struct sidtab *s) if (!name) continue; - rc = context_add_hash(p, &c->context[0]); - if (rc) { - sidtab_destroy(s); - goto out; - } + context_add_hash(&c->context[0]); + rc = sidtab_set_initial(s, sid, &c->context[0]); if (rc) { pr_err("SELinux: unable to load initial SID %s.\n", diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c index 07cdda2ff49c..ed3306914309 100644 --- a/security/selinux/ss/services.c +++ b/security/selinux/ss/services.c @@ -1490,38 +1490,13 @@ 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; - } + if (!context->hash) + context_add_hash(context); return sidtab_context_to_sid(sidtab, context, sid); } @@ -2119,9 +2094,7 @@ 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; + context_add_hash(newc); return 0; bad: @@ -2132,7 +2105,7 @@ bad: context_destroy(newc); newc->str = s; newc->len = len; - newc->hash = context_compute_hash(s); + context_add_hash(newc); pr_info("SELinux: Context %s became invalid (unmapped).\n", newc->str); return 0; diff --git a/security/selinux/ss/services.h b/security/selinux/ss/services.h index e9bddf33e53d..a06f3d835216 100644 --- a/security/selinux/ss/services.h +++ b/security/selinux/ss/services.h @@ -8,7 +8,6 @@ #define _SS_SERVICES_H_ #include "policydb.h" -#include "context.h" /* Mapping for a single class */ struct selinux_mapping { @@ -37,6 +36,4 @@ 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_ */ From 225621c9348d2a759db141024d5986d48e8c50dc Mon Sep 17 00:00:00 2001 From: Ondrej Mosnacek Date: Fri, 17 Apr 2020 10:11:57 +0200 Subject: [PATCH 05/12] selinux: move context hashing under sidtab Now that context hash computation no longer depends on policydb, we can simplify things by moving the context hashing completely under sidtab. The hash is still cached in sidtab entries, but not for the in-flight context structures. Signed-off-by: Ondrej Mosnacek Signed-off-by: Paul Moore --- security/selinux/ss/context.h | 11 +------ security/selinux/ss/policydb.c | 2 -- security/selinux/ss/services.c | 59 +++++++++++++++------------------- security/selinux/ss/sidtab.c | 32 ++++++++++-------- security/selinux/ss/sidtab.h | 1 + 5 files changed, 47 insertions(+), 58 deletions(-) diff --git a/security/selinux/ss/context.h b/security/selinux/ss/context.h index e7ae7e21449b..62990aa1ec9e 100644 --- a/security/selinux/ss/context.h +++ b/security/selinux/ss/context.h @@ -31,7 +31,6 @@ 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) @@ -169,13 +168,12 @@ 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 = c->hash = 0; + c->user = c->role = c->type = 0; kfree(c->str); c->str = NULL; c->len = 0; @@ -184,8 +182,6 @@ 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) @@ -198,10 +194,5 @@ static inline int context_cmp(struct context *c1, struct context *c2) u32 context_compute_hash(const struct context *c); -static inline void context_add_hash(struct context *context) -{ - context->hash = context_compute_hash(context); -} - #endif /* _SS_CONTEXT_H_ */ diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c index 2849bc362828..dc6729860bd6 100644 --- a/security/selinux/ss/policydb.c +++ b/security/selinux/ss/policydb.c @@ -862,8 +862,6 @@ int policydb_load_isids(struct policydb *p, struct sidtab *s) if (!name) continue; - context_add_hash(&c->context[0]); - rc = sidtab_set_initial(s, sid, &c->context[0]); if (rc) { pr_err("SELinux: unable to load initial SID %s.\n", diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c index ed3306914309..b49a336b1e6e 100644 --- a/security/selinux/ss/services.c +++ b/security/selinux/ss/services.c @@ -1490,17 +1490,6 @@ out: return rc; } -static int context_struct_to_sid(struct selinux_state *state, - struct context *context, u32 *sid) -{ - struct sidtab *sidtab = state->ss->sidtab; - - if (!context->hash) - context_add_hash(context); - - 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, @@ -1555,7 +1544,7 @@ static int security_context_to_sid_core(struct selinux_state *state, str = NULL; } else if (rc) goto out_unlock; - rc = context_struct_to_sid(state, &context, sid); + rc = sidtab_context_to_sid(sidtab, &context, sid); context_destroy(&context); out_unlock: read_unlock(&state->ss->policy_rwlock); @@ -1865,7 +1854,7 @@ static int security_compute_sid(struct selinux_state *state, goto out_unlock; } /* Obtain the sid for the context. */ - rc = context_struct_to_sid(state, &newcontext, out_sid); + rc = sidtab_context_to_sid(sidtab, &newcontext, out_sid); out_unlock: read_unlock(&state->ss->policy_rwlock); context_destroy(&newcontext); @@ -2017,7 +2006,6 @@ 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); @@ -2094,8 +2082,6 @@ static int convert_context(struct context *oldc, struct context *newc, void *p) goto bad; } - context_add_hash(newc); - return 0; bad: /* Map old representation to string and save it. */ @@ -2105,7 +2091,6 @@ bad: context_destroy(newc); newc->str = s; newc->len = len; - context_add_hash(newc); pr_info("SELinux: Context %s became invalid (unmapped).\n", newc->str); return 0; @@ -2322,12 +2307,14 @@ 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) { @@ -2340,7 +2327,7 @@ int security_port_sid(struct selinux_state *state, if (c) { if (!c->sid[0]) { - rc = context_struct_to_sid(state, &c->context[0], + rc = sidtab_context_to_sid(sidtab, &c->context[0], &c->sid[0]); if (rc) goto out; @@ -2365,12 +2352,14 @@ 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) { @@ -2384,7 +2373,7 @@ int security_ib_pkey_sid(struct selinux_state *state, if (c) { if (!c->sid[0]) { - rc = context_struct_to_sid(state, + rc = sidtab_context_to_sid(sidtab, &c->context[0], &c->sid[0]); if (rc) @@ -2409,12 +2398,14 @@ 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) { @@ -2429,7 +2420,7 @@ int security_ib_endport_sid(struct selinux_state *state, if (c) { if (!c->sid[0]) { - rc = context_struct_to_sid(state, &c->context[0], + rc = sidtab_context_to_sid(sidtab, &c->context[0], &c->sid[0]); if (rc) goto out; @@ -2452,12 +2443,14 @@ 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) { @@ -2468,11 +2461,11 @@ int security_netif_sid(struct selinux_state *state, if (c) { if (!c->sid[0] || !c->sid[1]) { - rc = context_struct_to_sid(state, &c->context[0], + rc = sidtab_context_to_sid(sidtab, &c->context[0], &c->sid[0]); if (rc) goto out; - rc = context_struct_to_sid(state, &c->context[1], + rc = sidtab_context_to_sid(sidtab, &c->context[1], &c->sid[1]); if (rc) goto out; @@ -2513,12 +2506,14 @@ 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: { @@ -2560,7 +2555,7 @@ int security_node_sid(struct selinux_state *state, if (c) { if (!c->sid[0]) { - rc = context_struct_to_sid(state, + rc = sidtab_context_to_sid(sidtab, &c->context[0], &c->sid[0]); if (rc) @@ -2644,17 +2639,12 @@ 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 = context_struct_to_sid(state, &usercon, &sid); + rc = sidtab_context_to_sid(sidtab, &usercon, &sid); if (rc) goto out_unlock; if (mynel < maxnel) { @@ -2725,6 +2715,7 @@ 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; @@ -2759,7 +2750,7 @@ static inline int __security_genfs_sid(struct selinux_state *state, goto out; if (!c->sid[0]) { - rc = context_struct_to_sid(state, &c->context[0], &c->sid[0]); + rc = sidtab_context_to_sid(sidtab, &c->context[0], &c->sid[0]); if (rc) goto out; } @@ -2801,6 +2792,7 @@ 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; @@ -2809,6 +2801,7 @@ 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) { @@ -2820,7 +2813,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 = context_struct_to_sid(state, &c->context[0], + rc = sidtab_context_to_sid(sidtab, &c->context[0], &c->sid[0]); if (rc) goto out; @@ -3068,7 +3061,7 @@ int security_sid_mls_copy(struct selinux_state *state, goto out_unlock; } } - rc = context_struct_to_sid(state, &newcon, new_sid); + rc = sidtab_context_to_sid(sidtab, &newcon, new_sid); out_unlock: read_unlock(&state->ss->policy_rwlock); context_destroy(&newcon); @@ -3661,7 +3654,7 @@ int security_netlbl_secattr_to_sid(struct selinux_state *state, if (!mls_context_isvalid(policydb, &ctx_new)) goto out_free; - rc = context_struct_to_sid(state, &ctx_new, sid); + rc = sidtab_context_to_sid(sidtab, &ctx_new, sid); if (rc) goto out_free; diff --git a/security/selinux/ss/sidtab.c b/security/selinux/ss/sidtab.c index 98d5ea3fcde4..eb6d27b5aeb4 100644 --- a/security/selinux/ss/sidtab.c +++ b/security/selinux/ss/sidtab.c @@ -54,14 +54,15 @@ int sidtab_init(struct sidtab *s) return 0; } -static u32 context_to_sid(struct sidtab *s, struct context *context) +static u32 context_to_sid(struct sidtab *s, struct context *context, u32 hash) { struct sidtab_entry *entry; u32 sid = 0; rcu_read_lock(); - hash_for_each_possible_rcu(s->context_to_sid, entry, list, - context->hash) { + hash_for_each_possible_rcu(s->context_to_sid, entry, list, hash) { + if (entry->hash != hash) + continue; if (context_cmp(&entry->context, context)) { sid = entry->sid; break; @@ -74,6 +75,7 @@ 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 *isid; + u32 hash; int rc; if (sid == 0 || sid > SECINITSID_NUM) @@ -90,15 +92,18 @@ int sidtab_set_initial(struct sidtab *s, u32 sid, struct context *context) #endif isid->set = 1; + hash = context_compute_hash(context); + /* * 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)) { + if (!context_to_sid(s, context, hash)) { isid->entry.sid = sid; - hash_add(s->context_to_sid, &isid->entry.list, context->hash); + isid->entry.hash = hash; + hash_add(s->context_to_sid, &isid->entry.list, hash); } return 0; @@ -259,12 +264,12 @@ int sidtab_context_to_sid(struct sidtab *s, struct context *context, u32 *sid) { unsigned long flags; - u32 count; + u32 count, hash = context_compute_hash(context); struct sidtab_convert_params *convert; struct sidtab_entry *dst, *dst_convert; int rc; - *sid = context_to_sid(s, context); + *sid = context_to_sid(s, context, hash); if (*sid) return 0; @@ -272,7 +277,7 @@ int sidtab_context_to_sid(struct sidtab *s, struct context *context, spin_lock_irqsave(&s->lock, flags); rc = 0; - *sid = context_to_sid(s, context); + *sid = context_to_sid(s, context, hash); if (*sid) goto out_unlock; @@ -291,6 +296,7 @@ int sidtab_context_to_sid(struct sidtab *s, struct context *context, goto out_unlock; dst->sid = index_to_sid(count); + dst->hash = hash; rc = context_cpy(&dst->context, context); if (rc) @@ -315,10 +321,11 @@ int sidtab_context_to_sid(struct sidtab *s, struct context *context, goto out_unlock; } dst_convert->sid = index_to_sid(count); + dst_convert->hash = context_compute_hash(&dst_convert->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->hash); } if (context->len) @@ -329,7 +336,7 @@ int sidtab_context_to_sid(struct sidtab *s, struct context *context, /* write entries before updating count */ smp_store_release(&s->count, count + 1); - hash_add_rcu(s->context_to_sid, &dst->list, dst->context.hash); + hash_add_rcu(s->context_to_sid, &dst->list, dst->hash); rc = 0; out_unlock: @@ -345,10 +352,9 @@ static void sidtab_convert_hashtable(struct sidtab *s, u32 count) for (i = 0; i < count; i++) { entry = sidtab_do_lookup(s, i, 0); entry->sid = index_to_sid(i); + entry->hash = context_compute_hash(&entry->context); - hash_add_rcu(s->context_to_sid, &entry->list, - entry->context.hash); - + hash_add_rcu(s->context_to_sid, &entry->list, entry->hash); } } diff --git a/security/selinux/ss/sidtab.h b/security/selinux/ss/sidtab.h index 3311d9f236c0..f2a84560b8b3 100644 --- a/security/selinux/ss/sidtab.h +++ b/security/selinux/ss/sidtab.h @@ -19,6 +19,7 @@ struct sidtab_entry { u32 sid; + u32 hash; struct context context; #if CONFIG_SECURITY_SELINUX_SID2STR_CACHE_SIZE > 0 struct sidtab_str_cache __rcu *cache; From 4300590243895ac39e8c97a2f5acd004dad8a42f Mon Sep 17 00:00:00 2001 From: Ondrej Mosnacek Date: Thu, 16 Apr 2020 19:13:55 +0200 Subject: [PATCH 06/12] selinux: implement new format of filename transitions Implement a new, more space-efficient way of storing filename transitions in the binary policy. The internal structures have already been converted to this new representation; this patch just implements reading/writing an equivalent represntation from/to the binary policy. This new format reduces the size of Fedora policy from 7.6 MB to only 3.3 MB (with policy optimization enabled in both cases). With the unconfined module disabled, the size is reduced from 3.3 MB to 2.4 MB. The time to load policy into kernel is also shorter with the new format. On Fedora Rawhide x86_64 it dropped from 157 ms to 106 ms; without the unconfined module from 115 ms to 105 ms. Signed-off-by: Ondrej Mosnacek Signed-off-by: Paul Moore --- security/selinux/include/security.h | 3 +- security/selinux/ss/policydb.c | 214 ++++++++++++++++++++++++---- 2 files changed, 190 insertions(+), 27 deletions(-) diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h index d6036c018cf2..b0e02cfe3ce1 100644 --- a/security/selinux/include/security.h +++ b/security/selinux/include/security.h @@ -41,10 +41,11 @@ #define POLICYDB_VERSION_XPERMS_IOCTL 30 #define POLICYDB_VERSION_INFINIBAND 31 #define POLICYDB_VERSION_GLBLUB 32 +#define POLICYDB_VERSION_COMP_FTRANS 33 /* compressed filename transitions */ /* Range of policy versions we understand*/ #define POLICYDB_VERSION_MIN POLICYDB_VERSION_BASE -#define POLICYDB_VERSION_MAX POLICYDB_VERSION_GLBLUB +#define POLICYDB_VERSION_MAX POLICYDB_VERSION_COMP_FTRANS /* Mask for just the mount related flags */ #define SE_MNTMASK 0x0f diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c index dc6729860bd6..ef8d5b46b05b 100644 --- a/security/selinux/ss/policydb.c +++ b/security/selinux/ss/policydb.c @@ -154,6 +154,11 @@ static struct policydb_compat_info policydb_compat[] = { .sym_num = SYM_NUM, .ocon_num = OCON_NUM, }, + { + .version = POLICYDB_VERSION_COMP_FTRANS, + .sym_num = SYM_NUM, + .ocon_num = OCON_NUM, + }, }; static struct policydb_compat_info *policydb_lookup_compat(int version) @@ -492,23 +497,16 @@ static int role_trans_cmp(struct hashtab *h, const void *k1, const void *k2) /* * Initialize a policy database structure. */ -static int policydb_init(struct policydb *p) +static void policydb_init(struct policydb *p) { memset(p, 0, sizeof(*p)); avtab_init(&p->te_avtab); cond_policydb_init(p); - p->filename_trans = hashtab_create(filenametr_hash, filenametr_cmp, - (1 << 11)); - if (!p->filename_trans) - return -ENOMEM; - ebitmap_init(&p->filename_trans_ttypes); ebitmap_init(&p->policycaps); ebitmap_init(&p->permissive_map); - - return 0; } /* @@ -1862,7 +1860,7 @@ out: return rc; } -static int filename_trans_read_one(struct policydb *p, void *fp) +static int filename_trans_read_helper_compat(struct policydb *p, void *fp) { struct filename_trans_key key, *ft = NULL; struct filename_trans_datum *last, *datum = NULL; @@ -1945,6 +1943,99 @@ out: return rc; } +static int filename_trans_read_helper(struct policydb *p, void *fp) +{ + struct filename_trans_key *ft = NULL; + struct filename_trans_datum **dst, *datum, *first = NULL; + char *name = NULL; + u32 len, ttype, tclass, ndatum, i; + __le32 buf[3]; + int rc; + + /* length of the path component string */ + rc = next_entry(buf, fp, sizeof(u32)); + if (rc) + return rc; + len = le32_to_cpu(buf[0]); + + /* path component string */ + rc = str_read(&name, GFP_KERNEL, fp, len); + if (rc) + return rc; + + rc = next_entry(buf, fp, sizeof(u32) * 3); + if (rc) + goto out; + + ttype = le32_to_cpu(buf[0]); + tclass = le32_to_cpu(buf[1]); + + ndatum = le32_to_cpu(buf[2]); + if (ndatum == 0) { + pr_err("SELinux: Filename transition key with no datum\n"); + rc = -ENOENT; + goto out; + } + + dst = &first; + for (i = 0; i < ndatum; i++) { + rc = -ENOMEM; + datum = kmalloc(sizeof(*datum), GFP_KERNEL); + if (!datum) + goto out; + + *dst = datum; + + /* ebitmap_read() will at least init the bitmap */ + rc = ebitmap_read(&datum->stypes, fp); + if (rc) + goto out; + + rc = next_entry(buf, fp, sizeof(u32)); + if (rc) + goto out; + + datum->otype = le32_to_cpu(buf[0]); + datum->next = NULL; + + dst = &datum->next; + } + + rc = -ENOMEM; + ft = kmalloc(sizeof(*ft), GFP_KERNEL); + if (!ft) + goto out; + + ft->ttype = ttype; + ft->tclass = tclass; + ft->name = name; + + rc = hashtab_insert(p->filename_trans, ft, first); + if (rc == -EEXIST) + pr_err("SELinux: Duplicate filename transition key\n"); + if (rc) + goto out; + + rc = ebitmap_set_bit(&p->filename_trans_ttypes, ttype, 1); + if (rc) + return rc; + + p->filename_trans_count += ndatum; + return 0; + +out: + kfree(ft); + kfree(name); + while (first) { + datum = first; + first = first->next; + + ebitmap_destroy(&datum->stypes); + kfree(datum); + } + return rc; +} + static int filename_trans_read(struct policydb *p, void *fp) { u32 nel; @@ -1959,12 +2050,29 @@ static int filename_trans_read(struct policydb *p, void *fp) return rc; nel = le32_to_cpu(buf[0]); - p->filename_trans_count = nel; + if (p->policyvers < POLICYDB_VERSION_COMP_FTRANS) { + p->filename_trans_count = nel; + p->filename_trans = hashtab_create(filenametr_hash, + filenametr_cmp, (1 << 11)); + if (!p->filename_trans) + return -ENOMEM; - for (i = 0; i < nel; i++) { - rc = filename_trans_read_one(p, fp); - if (rc) - return rc; + for (i = 0; i < nel; i++) { + rc = filename_trans_read_helper_compat(p, fp); + if (rc) + return rc; + } + } else { + p->filename_trans = hashtab_create(filenametr_hash, + filenametr_cmp, nel); + if (!p->filename_trans) + return -ENOMEM; + + for (i = 0; i < nel; i++) { + rc = filename_trans_read_helper(p, fp); + if (rc) + return rc; + } } hash_eval(p->filename_trans, "filenametr"); return 0; @@ -2281,9 +2389,7 @@ int policydb_read(struct policydb *p, void *fp) char *policydb_str; struct policydb_compat_info *info; - rc = policydb_init(p); - if (rc) - return rc; + policydb_init(p); /* Read the magic number and string length. */ rc = next_entry(buf, fp, sizeof(u32) * 2); @@ -3367,7 +3473,7 @@ static int range_write(struct policydb *p, void *fp) return 0; } -static int filename_write_helper(void *key, void *data, void *ptr) +static int filename_write_helper_compat(void *key, void *data, void *ptr) { struct filename_trans_key *ft = key; struct filename_trans_datum *datum = data; @@ -3404,6 +3510,55 @@ static int filename_write_helper(void *key, void *data, void *ptr) return 0; } +static int filename_write_helper(void *key, void *data, void *ptr) +{ + struct filename_trans_key *ft = key; + struct filename_trans_datum *datum; + void *fp = ptr; + __le32 buf[3]; + int rc; + u32 ndatum, len = strlen(ft->name); + + buf[0] = cpu_to_le32(len); + rc = put_entry(buf, sizeof(u32), 1, fp); + if (rc) + return rc; + + rc = put_entry(ft->name, sizeof(char), len, fp); + if (rc) + return rc; + + ndatum = 0; + datum = data; + do { + ndatum++; + datum = datum->next; + } while (unlikely(datum)); + + buf[0] = cpu_to_le32(ft->ttype); + buf[1] = cpu_to_le32(ft->tclass); + buf[2] = cpu_to_le32(ndatum); + rc = put_entry(buf, sizeof(u32), 3, fp); + if (rc) + return rc; + + datum = data; + do { + rc = ebitmap_write(&datum->stypes, fp); + if (rc) + return rc; + + buf[0] = cpu_to_le32(datum->otype); + rc = put_entry(buf, sizeof(u32), 1, fp); + if (rc) + return rc; + + datum = datum->next; + } while (unlikely(datum)); + + return 0; +} + static int filename_trans_write(struct policydb *p, void *fp) { __le32 buf[1]; @@ -3412,16 +3567,23 @@ static int filename_trans_write(struct policydb *p, void *fp) if (p->policyvers < POLICYDB_VERSION_FILENAME_TRANS) return 0; - buf[0] = cpu_to_le32(p->filename_trans_count); - rc = put_entry(buf, sizeof(u32), 1, fp); - if (rc) - return rc; + if (p->policyvers < POLICYDB_VERSION_COMP_FTRANS) { + buf[0] = cpu_to_le32(p->filename_trans_count); + rc = put_entry(buf, sizeof(u32), 1, fp); + if (rc) + return rc; - rc = hashtab_map(p->filename_trans, filename_write_helper, fp); - if (rc) - return rc; + rc = hashtab_map(p->filename_trans, + filename_write_helper_compat, fp); + } else { + buf[0] = cpu_to_le32(p->filename_trans->nel); + rc = put_entry(buf, sizeof(u32), 1, fp); + if (rc) + return rc; - return 0; + rc = hashtab_map(p->filename_trans, filename_write_helper, fp); + } + return rc; } /* From 9521eb3ea19a828d8fd59a2785338fd742dbcf31 Mon Sep 17 00:00:00 2001 From: Ondrej Mosnacek Date: Mon, 20 Apr 2020 15:27:31 +0200 Subject: [PATCH 07/12] selinux: don't produce incorrect filename_trans_count I thought I fixed the counting in filename_trans_read_helper() to count the compat rule count correctly in the final version, but it's still wrong. To really count the same thing as in the compat path, we'd need to add up the cardinalities of stype bitmaps of all datums. Since the kernel currently doesn't implement an ebitmap_cardinality() function (and computing the proper count would just waste CPU cycles anyway), just document that we use the field only in case of the old format and stop updating it in filename_trans_read_helper(). Fixes: 430059024389 ("selinux: implement new format of filename transitions") Signed-off-by: Ondrej Mosnacek Signed-off-by: Paul Moore --- security/selinux/ss/policydb.c | 11 +++-------- security/selinux/ss/policydb.h | 3 ++- 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c index ef8d5b46b05b..1c0041576643 100644 --- a/security/selinux/ss/policydb.c +++ b/security/selinux/ss/policydb.c @@ -2016,12 +2016,7 @@ static int filename_trans_read_helper(struct policydb *p, void *fp) if (rc) goto out; - rc = ebitmap_set_bit(&p->filename_trans_ttypes, ttype, 1); - if (rc) - return rc; - - p->filename_trans_count += ndatum; - return 0; + return ebitmap_set_bit(&p->filename_trans_ttypes, ttype, 1); out: kfree(ft); @@ -2051,7 +2046,7 @@ static int filename_trans_read(struct policydb *p, void *fp) nel = le32_to_cpu(buf[0]); if (p->policyvers < POLICYDB_VERSION_COMP_FTRANS) { - p->filename_trans_count = nel; + p->compat_filename_trans_count = nel; p->filename_trans = hashtab_create(filenametr_hash, filenametr_cmp, (1 << 11)); if (!p->filename_trans) @@ -3568,7 +3563,7 @@ static int filename_trans_write(struct policydb *p, void *fp) return 0; if (p->policyvers < POLICYDB_VERSION_COMP_FTRANS) { - buf[0] = cpu_to_le32(p->filename_trans_count); + buf[0] = cpu_to_le32(p->compat_filename_trans_count); rc = put_entry(buf, sizeof(u32), 1, fp); if (rc) return rc; diff --git a/security/selinux/ss/policydb.h b/security/selinux/ss/policydb.h index d3adb522d3f3..35dc6aa7904d 100644 --- a/security/selinux/ss/policydb.h +++ b/security/selinux/ss/policydb.h @@ -270,7 +270,8 @@ struct policydb { struct ebitmap filename_trans_ttypes; /* actual set of filename_trans rules */ struct hashtab *filename_trans; - u32 filename_trans_count; + /* only used if policyvers < POLICYDB_VERSION_COMP_FTRANS */ + u32 compat_filename_trans_count; /* bools indexed by (value - 1) */ struct cond_bool_datum **bool_val_to_struct; From 4c09f8b6913a779ca0c70ea8058bf21537eebb3b Mon Sep 17 00:00:00 2001 From: Wei Yongjun Date: Wed, 29 Apr 2020 07:30:53 +0000 Subject: [PATCH 08/12] selinux: fix error return code in policydb_read() Fix to return negative error code -ENOMEM from the kvcalloc() error handling case instead of 0, as done elsewhere in this function. Fixes: acdf52d97f82 ("selinux: convert to kvmalloc") Signed-off-by: Wei Yongjun Signed-off-by: Paul Moore --- security/selinux/ss/policydb.c | 1 + 1 file changed, 1 insertion(+) diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c index 1c0041576643..a42369dd96a9 100644 --- a/security/selinux/ss/policydb.c +++ b/security/selinux/ss/policydb.c @@ -2638,6 +2638,7 @@ int policydb_read(struct policydb *p, void *fp) if (rc) goto bad; + rc = -ENOMEM; p->type_attr_map_array = kvcalloc(p->p_types.nprim, sizeof(*p->type_attr_map_array), GFP_KERNEL); From 3348bd33e8cf8a17138e8ce716ae474ec5d7001e Mon Sep 17 00:00:00 2001 From: Ondrej Mosnacek Date: Tue, 28 Apr 2020 14:55:11 +0200 Subject: [PATCH 09/12] selinux: simplify range_write() No need to traverse the hashtab to count its elements, hashtab already tracks it for us. Signed-off-by: Ondrej Mosnacek Signed-off-by: Paul Moore --- security/selinux/ss/policydb.c | 18 ++---------------- 1 file changed, 2 insertions(+), 16 deletions(-) diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c index a42369dd96a9..8a287a7afd9f 100644 --- a/security/selinux/ss/policydb.c +++ b/security/selinux/ss/policydb.c @@ -3405,14 +3405,6 @@ static int genfs_write(struct policydb *p, void *fp) return 0; } -static int hashtab_cnt(void *key, void *data, void *ptr) -{ - int *cnt = ptr; - *cnt = *cnt + 1; - - return 0; -} - static int range_write_helper(void *key, void *data, void *ptr) { __le32 buf[2]; @@ -3444,19 +3436,13 @@ static int range_write_helper(void *key, void *data, void *ptr) static int range_write(struct policydb *p, void *fp) { __le32 buf[1]; - int rc, nel; + int rc; struct policy_data pd; pd.p = p; pd.fp = fp; - /* count the number of entries in the hashtab */ - nel = 0; - rc = hashtab_map(p->range_tr, hashtab_cnt, &nel); - if (rc) - return rc; - - buf[0] = cpu_to_le32(nel); + buf[0] = cpu_to_le32(p->range_tr->nel); rc = put_entry(buf, sizeof(u32), 1, fp); if (rc) return rc; From 46619b44e431d85d64a8dfcb7166d0ae098544c8 Mon Sep 17 00:00:00 2001 From: Ondrej Mosnacek Date: Fri, 1 May 2020 21:51:11 +0200 Subject: [PATCH 10/12] selinux: fix return value on error in policydb_read() The value of rc is still zero from the last assignment when the error path is taken. Fix it by setting it to -ENOMEM before the hashtab_create() call. Reported-by: Dan Carpenter Fixes: e67b2ec9f617 ("selinux: store role transitions in a hash table") Signed-off-by: Ondrej Mosnacek Signed-off-by: Paul Moore --- security/selinux/ss/policydb.c | 1 + 1 file changed, 1 insertion(+) diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c index 8a287a7afd9f..76358c9de129 100644 --- a/security/selinux/ss/policydb.c +++ b/security/selinux/ss/policydb.c @@ -2540,6 +2540,7 @@ int policydb_read(struct policydb *p, void *fp) goto bad; nel = le32_to_cpu(buf[0]); + rc = -ENOMEM; p->role_tr = hashtab_create(role_trans_hash, role_trans_cmp, nel); if (!p->role_tr) goto bad; From 03414a49ad5f3c56988c36d2070e402ffa17feaf Mon Sep 17 00:00:00 2001 From: Ondrej Mosnacek Date: Tue, 28 Apr 2020 14:55:12 +0200 Subject: [PATCH 11/12] selinux: do not allocate hashtabs dynamically It is simpler to allocate them statically in the corresponding structure, avoiding unnecessary kmalloc() calls and pointer dereferencing. Signed-off-by: Ondrej Mosnacek [PM: manual merging required in policydb.c] Signed-off-by: Paul Moore --- security/selinux/ss/hashtab.c | 51 ++++--------- security/selinux/ss/hashtab.h | 13 ++-- security/selinux/ss/mls.c | 14 ++-- security/selinux/ss/policydb.c | 127 ++++++++++++++++----------------- security/selinux/ss/policydb.h | 6 +- security/selinux/ss/services.c | 44 ++++++------ security/selinux/ss/symtab.c | 5 +- security/selinux/ss/symtab.h | 2 +- 8 files changed, 116 insertions(+), 146 deletions(-) diff --git a/security/selinux/ss/hashtab.c b/security/selinux/ss/hashtab.c index 883f19d32c28..5ee868116d70 100644 --- a/security/selinux/ss/hashtab.c +++ b/security/selinux/ss/hashtab.c @@ -29,34 +29,21 @@ static u32 hashtab_compute_size(u32 nel) return nel == 0 ? 0 : roundup_pow_of_two(nel); } -struct hashtab *hashtab_create(u32 (*hash_value)(struct hashtab *h, const void *key), - int (*keycmp)(struct hashtab *h, const void *key1, const void *key2), - u32 nel_hint) +int hashtab_init(struct hashtab *h, + u32 (*hash_value)(struct hashtab *h, const void *key), + int (*keycmp)(struct hashtab *h, const void *key1, + const void *key2), + u32 nel_hint) { - struct hashtab *p; - u32 i, size = hashtab_compute_size(nel_hint); + h->size = hashtab_compute_size(nel_hint); + h->nel = 0; + h->hash_value = hash_value; + h->keycmp = keycmp; + if (!h->size) + return 0; - p = kzalloc(sizeof(*p), GFP_KERNEL); - if (!p) - return p; - - p->size = size; - p->nel = 0; - p->hash_value = hash_value; - p->keycmp = keycmp; - if (!size) - return p; - - p->htable = kmalloc_array(size, sizeof(*p->htable), GFP_KERNEL); - if (!p->htable) { - kfree(p); - return NULL; - } - - for (i = 0; i < size; i++) - p->htable[i] = NULL; - - return p; + h->htable = kcalloc(h->size, sizeof(*h->htable), GFP_KERNEL); + return h->htable ? 0 : -ENOMEM; } int hashtab_insert(struct hashtab *h, void *key, void *datum) @@ -66,7 +53,7 @@ int hashtab_insert(struct hashtab *h, void *key, void *datum) cond_resched(); - if (!h || !h->size || h->nel == HASHTAB_MAX_NODES) + if (!h->size || h->nel == HASHTAB_MAX_NODES) return -EINVAL; hvalue = h->hash_value(h, key); @@ -102,7 +89,7 @@ void *hashtab_search(struct hashtab *h, const void *key) u32 hvalue; struct hashtab_node *cur; - if (!h || !h->size) + if (!h->size) return NULL; hvalue = h->hash_value(h, key); @@ -121,9 +108,6 @@ void hashtab_destroy(struct hashtab *h) u32 i; struct hashtab_node *cur, *temp; - if (!h) - return; - for (i = 0; i < h->size; i++) { cur = h->htable[i]; while (cur) { @@ -136,8 +120,6 @@ void hashtab_destroy(struct hashtab *h) kfree(h->htable); h->htable = NULL; - - kfree(h); } int hashtab_map(struct hashtab *h, @@ -148,9 +130,6 @@ int hashtab_map(struct hashtab *h, int ret; struct hashtab_node *cur; - if (!h) - return 0; - for (i = 0; i < h->size; i++) { cur = h->htable[i]; while (cur) { diff --git a/security/selinux/ss/hashtab.h b/security/selinux/ss/hashtab.h index dde54d9ff01c..31c11511fe10 100644 --- a/security/selinux/ss/hashtab.h +++ b/security/selinux/ss/hashtab.h @@ -35,14 +35,15 @@ struct hashtab_info { }; /* - * Creates a new hash table with the specified characteristics. + * Initializes a new hash table with the specified characteristics. * - * Returns NULL if insufficent space is available or - * the new hash table otherwise. + * Returns -ENOMEM if insufficient space is available or 0 otherwise. */ -struct hashtab *hashtab_create(u32 (*hash_value)(struct hashtab *h, const void *key), - int (*keycmp)(struct hashtab *h, const void *key1, const void *key2), - u32 nel_hint); +int hashtab_init(struct hashtab *h, + u32 (*hash_value)(struct hashtab *h, const void *key), + int (*keycmp)(struct hashtab *h, const void *key1, + const void *key2), + u32 nel_hint); /* * Inserts the specified (key, datum) pair into the specified hash table. diff --git a/security/selinux/ss/mls.c b/security/selinux/ss/mls.c index 6a5d7d08933d..cd8734f25b39 100644 --- a/security/selinux/ss/mls.c +++ b/security/selinux/ss/mls.c @@ -165,7 +165,7 @@ int mls_level_isvalid(struct policydb *p, struct mls_level *l) if (!l->sens || l->sens > p->p_levels.nprim) return 0; - levdatum = hashtab_search(p->p_levels.table, + levdatum = hashtab_search(&p->p_levels.table, sym_name(p, SYM_LEVELS, l->sens - 1)); if (!levdatum) return 0; @@ -293,7 +293,7 @@ int mls_context_to_sid(struct policydb *pol, *(next_cat++) = '\0'; /* Parse sensitivity. */ - levdatum = hashtab_search(pol->p_levels.table, sensitivity); + levdatum = hashtab_search(&pol->p_levels.table, sensitivity); if (!levdatum) return -EINVAL; context->range.level[l].sens = levdatum->level->sens; @@ -312,7 +312,7 @@ int mls_context_to_sid(struct policydb *pol, *rngptr++ = '\0'; } - catdatum = hashtab_search(pol->p_cats.table, cur_cat); + catdatum = hashtab_search(&pol->p_cats.table, cur_cat); if (!catdatum) return -EINVAL; @@ -325,7 +325,7 @@ int mls_context_to_sid(struct policydb *pol, if (rngptr == NULL) continue; - rngdatum = hashtab_search(pol->p_cats.table, rngptr); + rngdatum = hashtab_search(&pol->p_cats.table, rngptr); if (!rngdatum) return -EINVAL; @@ -458,7 +458,7 @@ int mls_convert_context(struct policydb *oldp, return 0; for (l = 0; l < 2; l++) { - levdatum = hashtab_search(newp->p_levels.table, + levdatum = hashtab_search(&newp->p_levels.table, sym_name(oldp, SYM_LEVELS, oldc->range.level[l].sens - 1)); @@ -470,7 +470,7 @@ int mls_convert_context(struct policydb *oldp, node, i) { int rc; - catdatum = hashtab_search(newp->p_cats.table, + catdatum = hashtab_search(&newp->p_cats.table, sym_name(oldp, SYM_CATS, i)); if (!catdatum) return -EINVAL; @@ -506,7 +506,7 @@ int mls_compute_sid(struct policydb *p, rtr.source_type = scontext->type; rtr.target_type = tcontext->type; rtr.target_class = tclass; - r = hashtab_search(p->range_tr, &rtr); + r = hashtab_search(&p->range_tr, &rtr); if (r) return mls_range_set(newcontext, r); diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c index 76358c9de129..a30cad18931b 100644 --- a/security/selinux/ss/policydb.c +++ b/security/selinux/ss/policydb.c @@ -195,8 +195,8 @@ static int common_destroy(void *key, void *datum, void *p) kfree(key); if (datum) { comdatum = datum; - hashtab_map(comdatum->permissions.table, perm_destroy, NULL); - hashtab_destroy(comdatum->permissions.table); + hashtab_map(&comdatum->permissions.table, perm_destroy, NULL); + hashtab_destroy(&comdatum->permissions.table); } kfree(datum); return 0; @@ -224,8 +224,8 @@ static int cls_destroy(void *key, void *datum, void *p) kfree(key); if (datum) { cladatum = datum; - hashtab_map(cladatum->permissions.table, perm_destroy, NULL); - hashtab_destroy(cladatum->permissions.table); + hashtab_map(&cladatum->permissions.table, perm_destroy, NULL); + hashtab_destroy(&cladatum->permissions.table); constraint = cladatum->constraints; while (constraint) { e = constraint->expr; @@ -400,7 +400,7 @@ static int roles_init(struct policydb *p) if (!key) goto out; - rc = hashtab_insert(p->p_roles.table, key, role); + rc = hashtab_insert(&p->p_roles.table, key, role); if (rc) goto out; @@ -668,7 +668,7 @@ static void symtab_hash_eval(struct symtab *s) int i; for (i = 0; i < SYM_NUM; i++) - hash_eval(s[i].table, symtab_name[i]); + hash_eval(&s[i].table, symtab_name[i]); } #else @@ -739,7 +739,7 @@ static int policydb_index(struct policydb *p) if (!p->sym_val_to_name[i]) return -ENOMEM; - rc = hashtab_map(p->symtab[i].table, index_f[i], p); + rc = hashtab_map(&p->symtab[i].table, index_f[i], p); if (rc) goto out; } @@ -760,8 +760,8 @@ void policydb_destroy(struct policydb *p) for (i = 0; i < SYM_NUM; i++) { cond_resched(); - hashtab_map(p->symtab[i].table, destroy_f[i], NULL); - hashtab_destroy(p->symtab[i].table); + hashtab_map(&p->symtab[i].table, destroy_f[i], NULL); + hashtab_destroy(&p->symtab[i].table); } for (i = 0; i < SYM_NUM; i++) @@ -803,8 +803,8 @@ void policydb_destroy(struct policydb *p) cond_policydb_destroy(p); - hashtab_map(p->role_tr, role_tr_destroy, NULL); - hashtab_destroy(p->role_tr); + hashtab_map(&p->role_tr, role_tr_destroy, NULL); + hashtab_destroy(&p->role_tr); for (ra = p->role_allow; ra; ra = ra->next) { cond_resched(); @@ -813,11 +813,11 @@ void policydb_destroy(struct policydb *p) } kfree(lra); - hashtab_map(p->filename_trans, filenametr_destroy, NULL); - hashtab_destroy(p->filename_trans); + hashtab_map(&p->filename_trans, filenametr_destroy, NULL); + hashtab_destroy(&p->filename_trans); - hashtab_map(p->range_tr, range_tr_destroy, NULL); - hashtab_destroy(p->range_tr); + hashtab_map(&p->range_tr, range_tr_destroy, NULL); + hashtab_destroy(&p->range_tr); if (p->type_attr_map_array) { for (i = 0; i < p->p_types.nprim; i++) @@ -1128,7 +1128,7 @@ static int common_read(struct policydb *p, struct hashtab *h, void *fp) goto bad; for (i = 0; i < nel; i++) { - rc = perm_read(p, comdatum->permissions.table, fp); + rc = perm_read(p, &comdatum->permissions.table, fp); if (rc) goto bad; } @@ -1300,7 +1300,8 @@ static int class_read(struct policydb *p, struct hashtab *h, void *fp) goto bad; rc = -EINVAL; - cladatum->comdatum = hashtab_search(p->p_commons.table, cladatum->comkey); + cladatum->comdatum = hashtab_search(&p->p_commons.table, + cladatum->comkey); if (!cladatum->comdatum) { pr_err("SELinux: unknown common %s\n", cladatum->comkey); @@ -1308,7 +1309,7 @@ static int class_read(struct policydb *p, struct hashtab *h, void *fp) } } for (i = 0; i < nel; i++) { - rc = perm_read(p, cladatum->permissions.table, fp); + rc = perm_read(p, &cladatum->permissions.table, fp); if (rc) goto bad; } @@ -1731,18 +1732,15 @@ static int policydb_bounds_sanity_check(struct policydb *p) if (p->policyvers < POLICYDB_VERSION_BOUNDARY) return 0; - rc = hashtab_map(p->p_users.table, - user_bounds_sanity_check, p); + rc = hashtab_map(&p->p_users.table, user_bounds_sanity_check, p); if (rc) return rc; - rc = hashtab_map(p->p_roles.table, - role_bounds_sanity_check, p); + rc = hashtab_map(&p->p_roles.table, role_bounds_sanity_check, p); if (rc) return rc; - rc = hashtab_map(p->p_types.table, - type_bounds_sanity_check, p); + rc = hashtab_map(&p->p_types.table, type_bounds_sanity_check, p); if (rc) return rc; @@ -1753,7 +1751,7 @@ u16 string_to_security_class(struct policydb *p, const char *name) { struct class_datum *cladatum; - cladatum = hashtab_search(p->p_classes.table, name); + cladatum = hashtab_search(&p->p_classes.table, name); if (!cladatum) return 0; @@ -1772,11 +1770,9 @@ u32 string_to_av_perm(struct policydb *p, u16 tclass, const char *name) cladatum = p->class_val_to_struct[tclass-1]; comdatum = cladatum->comdatum; if (comdatum) - perdatum = hashtab_search(comdatum->permissions.table, - name); + perdatum = hashtab_search(&comdatum->permissions.table, name); if (!perdatum) - perdatum = hashtab_search(cladatum->permissions.table, - name); + perdatum = hashtab_search(&cladatum->permissions.table, name); if (!perdatum) return 0; @@ -1800,9 +1796,9 @@ static int range_read(struct policydb *p, void *fp) nel = le32_to_cpu(buf[0]); - p->range_tr = hashtab_create(rangetr_hash, rangetr_cmp, nel); - if (!p->range_tr) - return -ENOMEM; + rc = hashtab_init(&p->range_tr, rangetr_hash, rangetr_cmp, nel); + if (rc) + return rc; for (i = 0; i < nel; i++) { rc = -ENOMEM; @@ -1845,14 +1841,14 @@ static int range_read(struct policydb *p, void *fp) goto out; } - rc = hashtab_insert(p->range_tr, rt, r); + rc = hashtab_insert(&p->range_tr, rt, r); if (rc) goto out; rt = NULL; r = NULL; } - hash_eval(p->range_tr, "rangetr"); + hash_eval(&p->range_tr, "rangetr"); rc = 0; out: kfree(rt); @@ -1892,7 +1888,7 @@ static int filename_trans_read_helper_compat(struct policydb *p, void *fp) otype = le32_to_cpu(buf[3]); last = NULL; - datum = hashtab_search(p->filename_trans, &key); + datum = hashtab_search(&p->filename_trans, &key); while (datum) { if (unlikely(ebitmap_get_bit(&datum->stypes, stype - 1))) { /* conflicting/duplicate rules are ignored */ @@ -1922,7 +1918,7 @@ static int filename_trans_read_helper_compat(struct policydb *p, void *fp) if (!ft) goto out; - rc = hashtab_insert(p->filename_trans, ft, datum); + rc = hashtab_insert(&p->filename_trans, ft, datum); if (rc) goto out; name = NULL; @@ -2010,7 +2006,7 @@ static int filename_trans_read_helper(struct policydb *p, void *fp) ft->tclass = tclass; ft->name = name; - rc = hashtab_insert(p->filename_trans, ft, first); + rc = hashtab_insert(&p->filename_trans, ft, first); if (rc == -EEXIST) pr_err("SELinux: Duplicate filename transition key\n"); if (rc) @@ -2047,10 +2043,11 @@ static int filename_trans_read(struct policydb *p, void *fp) if (p->policyvers < POLICYDB_VERSION_COMP_FTRANS) { p->compat_filename_trans_count = nel; - p->filename_trans = hashtab_create(filenametr_hash, - filenametr_cmp, (1 << 11)); - if (!p->filename_trans) - return -ENOMEM; + + rc = hashtab_init(&p->filename_trans, filenametr_hash, + filenametr_cmp, (1 << 11)); + if (rc) + return rc; for (i = 0; i < nel; i++) { rc = filename_trans_read_helper_compat(p, fp); @@ -2058,10 +2055,10 @@ static int filename_trans_read(struct policydb *p, void *fp) return rc; } } else { - p->filename_trans = hashtab_create(filenametr_hash, - filenametr_cmp, nel); - if (!p->filename_trans) - return -ENOMEM; + rc = hashtab_init(&p->filename_trans, filenametr_hash, + filenametr_cmp, nel); + if (rc) + return rc; for (i = 0; i < nel; i++) { rc = filename_trans_read_helper(p, fp); @@ -2069,7 +2066,7 @@ static int filename_trans_read(struct policydb *p, void *fp) return rc; } } - hash_eval(p->filename_trans, "filenametr"); + hash_eval(&p->filename_trans, "filenametr"); return 0; } @@ -2512,7 +2509,7 @@ int policydb_read(struct policydb *p, void *fp) } for (j = 0; j < nel; j++) { - rc = read_f[i](p, p->symtab[i].table, fp); + rc = read_f[i](p, &p->symtab[i].table, fp); if (rc) goto bad; } @@ -2540,9 +2537,8 @@ int policydb_read(struct policydb *p, void *fp) goto bad; nel = le32_to_cpu(buf[0]); - rc = -ENOMEM; - p->role_tr = hashtab_create(role_trans_hash, role_trans_cmp, nel); - if (!p->role_tr) + rc = hashtab_init(&p->role_tr, role_trans_hash, role_trans_cmp, nel); + if (rc) goto bad; for (i = 0; i < nel; i++) { rc = -ENOMEM; @@ -2578,7 +2574,7 @@ int policydb_read(struct policydb *p, void *fp) !policydb_role_isvalid(p, rtd->new_role)) goto bad; - rc = hashtab_insert(p->role_tr, rtk, rtd); + rc = hashtab_insert(&p->role_tr, rtk, rtd); if (rc) goto bad; @@ -2822,12 +2818,12 @@ static int role_trans_write(struct policydb *p, void *fp) __le32 buf[1]; int rc; - buf[0] = cpu_to_le32(p->role_tr->nel); + buf[0] = cpu_to_le32(p->role_tr.nel); rc = put_entry(buf, sizeof(u32), 1, fp); if (rc) return rc; - return hashtab_map(p->role_tr, role_trans_write_one, &pd); + return hashtab_map(&p->role_tr, role_trans_write_one, &pd); } static int role_allow_write(struct role_allow *r, void *fp) @@ -2921,7 +2917,7 @@ static int common_write(void *vkey, void *datum, void *ptr) buf[0] = cpu_to_le32(len); buf[1] = cpu_to_le32(comdatum->value); buf[2] = cpu_to_le32(comdatum->permissions.nprim); - buf[3] = cpu_to_le32(comdatum->permissions.table->nel); + buf[3] = cpu_to_le32(comdatum->permissions.table.nel); rc = put_entry(buf, sizeof(u32), 4, fp); if (rc) return rc; @@ -2930,7 +2926,7 @@ static int common_write(void *vkey, void *datum, void *ptr) if (rc) return rc; - rc = hashtab_map(comdatum->permissions.table, perm_write, fp); + rc = hashtab_map(&comdatum->permissions.table, perm_write, fp); if (rc) return rc; @@ -3029,10 +3025,7 @@ static int class_write(void *vkey, void *datum, void *ptr) buf[1] = cpu_to_le32(len2); buf[2] = cpu_to_le32(cladatum->value); buf[3] = cpu_to_le32(cladatum->permissions.nprim); - if (cladatum->permissions.table) - buf[4] = cpu_to_le32(cladatum->permissions.table->nel); - else - buf[4] = 0; + buf[4] = cpu_to_le32(cladatum->permissions.table.nel); buf[5] = cpu_to_le32(ncons); rc = put_entry(buf, sizeof(u32), 6, fp); if (rc) @@ -3048,7 +3041,7 @@ static int class_write(void *vkey, void *datum, void *ptr) return rc; } - rc = hashtab_map(cladatum->permissions.table, perm_write, fp); + rc = hashtab_map(&cladatum->permissions.table, perm_write, fp); if (rc) return rc; @@ -3443,13 +3436,13 @@ static int range_write(struct policydb *p, void *fp) pd.p = p; pd.fp = fp; - buf[0] = cpu_to_le32(p->range_tr->nel); + buf[0] = cpu_to_le32(p->range_tr.nel); rc = put_entry(buf, sizeof(u32), 1, fp); if (rc) return rc; /* actually write all of the entries */ - rc = hashtab_map(p->range_tr, range_write_helper, &pd); + rc = hashtab_map(&p->range_tr, range_write_helper, &pd); if (rc) return rc; @@ -3556,15 +3549,15 @@ static int filename_trans_write(struct policydb *p, void *fp) if (rc) return rc; - rc = hashtab_map(p->filename_trans, + rc = hashtab_map(&p->filename_trans, filename_write_helper_compat, fp); } else { - buf[0] = cpu_to_le32(p->filename_trans->nel); + buf[0] = cpu_to_le32(p->filename_trans.nel); rc = put_entry(buf, sizeof(u32), 1, fp); if (rc) return rc; - rc = hashtab_map(p->filename_trans, filename_write_helper, fp); + rc = hashtab_map(&p->filename_trans, filename_write_helper, fp); } return rc; } @@ -3653,12 +3646,12 @@ int policydb_write(struct policydb *p, void *fp) pd.p = p; buf[0] = cpu_to_le32(p->symtab[i].nprim); - buf[1] = cpu_to_le32(p->symtab[i].table->nel); + buf[1] = cpu_to_le32(p->symtab[i].table.nel); rc = put_entry(buf, sizeof(u32), 2, fp); if (rc) return rc; - rc = hashtab_map(p->symtab[i].table, write_f[i], &pd); + rc = hashtab_map(&p->symtab[i].table, write_f[i], &pd); if (rc) return rc; } diff --git a/security/selinux/ss/policydb.h b/security/selinux/ss/policydb.h index 35dc6aa7904d..9591c9587cb6 100644 --- a/security/selinux/ss/policydb.h +++ b/security/selinux/ss/policydb.h @@ -263,13 +263,13 @@ struct policydb { struct avtab te_avtab; /* role transitions */ - struct hashtab *role_tr; + struct hashtab role_tr; /* file transitions with the last path component */ /* quickly exclude lookups when parent ttype has no rules */ struct ebitmap filename_trans_ttypes; /* actual set of filename_trans rules */ - struct hashtab *filename_trans; + struct hashtab filename_trans; /* only used if policyvers < POLICYDB_VERSION_COMP_FTRANS */ u32 compat_filename_trans_count; @@ -294,7 +294,7 @@ struct policydb { struct genfs *genfs; /* range transitions table (range_trans_key -> mls_range) */ - struct hashtab *range_tr; + struct hashtab range_tr; /* type -> attribute reverse mapping */ struct ebitmap *type_attr_map_array; diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c index b49a336b1e6e..313919bd42f8 100644 --- a/security/selinux/ss/services.c +++ b/security/selinux/ss/services.c @@ -482,11 +482,11 @@ static void security_dump_masked_av(struct policydb *policydb, /* init permission_names */ if (common_dat && - hashtab_map(common_dat->permissions.table, + hashtab_map(&common_dat->permissions.table, dump_masked_av_helper, permission_names) < 0) goto out; - if (hashtab_map(tclass_dat->permissions.table, + if (hashtab_map(&tclass_dat->permissions.table, dump_masked_av_helper, permission_names) < 0) goto out; @@ -1441,7 +1441,7 @@ static int string_to_context_struct(struct policydb *pol, *p++ = 0; - usrdatum = hashtab_search(pol->p_users.table, scontextp); + usrdatum = hashtab_search(&pol->p_users.table, scontextp); if (!usrdatum) goto out; @@ -1457,7 +1457,7 @@ static int string_to_context_struct(struct policydb *pol, *p++ = 0; - role = hashtab_search(pol->p_roles.table, scontextp); + role = hashtab_search(&pol->p_roles.table, scontextp); if (!role) goto out; ctx->role = role->value; @@ -1469,7 +1469,7 @@ static int string_to_context_struct(struct policydb *pol, oldc = *p; *p++ = 0; - typdatum = hashtab_search(pol->p_types.table, scontextp); + typdatum = hashtab_search(&pol->p_types.table, scontextp); if (!typdatum || typdatum->attribute) goto out; @@ -1671,7 +1671,7 @@ static void filename_compute_type(struct policydb *policydb, ft.tclass = tclass; ft.name = objname; - datum = hashtab_search(policydb->filename_trans, &ft); + datum = hashtab_search(&policydb->filename_trans, &ft); while (datum) { if (ebitmap_get_bit(&datum->stypes, stype - 1)) { newcontext->type = datum->otype; @@ -1834,7 +1834,7 @@ static int security_compute_sid(struct selinux_state *state, .tclass = tclass, }; - rtd = hashtab_search(policydb->role_tr, &rtk); + rtd = hashtab_search(&policydb->role_tr, &rtk); if (rtd) newcontext.role = rtd->new_role; } @@ -2024,7 +2024,7 @@ static int convert_context(struct context *oldc, struct context *newc, void *p) /* Convert the user. */ rc = -EINVAL; - usrdatum = hashtab_search(args->newp->p_users.table, + usrdatum = hashtab_search(&args->newp->p_users.table, sym_name(args->oldp, SYM_USERS, oldc->user - 1)); if (!usrdatum) @@ -2033,7 +2033,7 @@ static int convert_context(struct context *oldc, struct context *newc, void *p) /* Convert the role. */ rc = -EINVAL; - role = hashtab_search(args->newp->p_roles.table, + role = hashtab_search(&args->newp->p_roles.table, sym_name(args->oldp, SYM_ROLES, oldc->role - 1)); if (!role) goto bad; @@ -2041,7 +2041,7 @@ static int convert_context(struct context *oldc, struct context *newc, void *p) /* Convert the type. */ rc = -EINVAL; - typdatum = hashtab_search(args->newp->p_types.table, + typdatum = hashtab_search(&args->newp->p_types.table, sym_name(args->oldp, SYM_TYPES, oldc->type - 1)); if (!typdatum) @@ -2623,7 +2623,7 @@ int security_get_user_sids(struct selinux_state *state, goto out_unlock; rc = -EINVAL; - user = hashtab_search(policydb->p_users.table, username); + user = hashtab_search(&policydb->p_users.table, username); if (!user) goto out_unlock; @@ -2975,7 +2975,7 @@ static int security_preserve_bools(struct selinux_state *state, if (rc) goto out; for (i = 0; i < nbools; i++) { - booldatum = hashtab_search(policydb->p_bools.table, bnames[i]); + booldatum = hashtab_search(&policydb->p_bools.table, bnames[i]); if (booldatum) booldatum->state = bvalues[i]; } @@ -3189,8 +3189,8 @@ int security_get_classes(struct selinux_state *state, if (!*classes) goto out; - rc = hashtab_map(policydb->p_classes.table, get_classes_callback, - *classes); + rc = hashtab_map(&policydb->p_classes.table, get_classes_callback, + *classes); if (rc) { int i; for (i = 0; i < *nclasses; i++) @@ -3226,7 +3226,7 @@ int security_get_permissions(struct selinux_state *state, read_lock(&state->ss->policy_rwlock); rc = -EINVAL; - match = hashtab_search(policydb->p_classes.table, class); + match = hashtab_search(&policydb->p_classes.table, class); if (!match) { pr_err("SELinux: %s: unrecognized class %s\n", __func__, class); @@ -3240,14 +3240,14 @@ int security_get_permissions(struct selinux_state *state, goto out; if (match->comdatum) { - rc = hashtab_map(match->comdatum->permissions.table, - get_permissions_callback, *perms); + rc = hashtab_map(&match->comdatum->permissions.table, + get_permissions_callback, *perms); if (rc) goto err; } - rc = hashtab_map(match->permissions.table, get_permissions_callback, - *perms); + rc = hashtab_map(&match->permissions.table, get_permissions_callback, + *perms); if (rc) goto err; @@ -3365,7 +3365,7 @@ int selinux_audit_rule_init(u32 field, u32 op, char *rulestr, void **vrule) case AUDIT_SUBJ_USER: case AUDIT_OBJ_USER: rc = -EINVAL; - userdatum = hashtab_search(policydb->p_users.table, rulestr); + userdatum = hashtab_search(&policydb->p_users.table, rulestr); if (!userdatum) goto out; tmprule->au_ctxt.user = userdatum->value; @@ -3373,7 +3373,7 @@ int selinux_audit_rule_init(u32 field, u32 op, char *rulestr, void **vrule) case AUDIT_SUBJ_ROLE: case AUDIT_OBJ_ROLE: rc = -EINVAL; - roledatum = hashtab_search(policydb->p_roles.table, rulestr); + roledatum = hashtab_search(&policydb->p_roles.table, rulestr); if (!roledatum) goto out; tmprule->au_ctxt.role = roledatum->value; @@ -3381,7 +3381,7 @@ int selinux_audit_rule_init(u32 field, u32 op, char *rulestr, void **vrule) case AUDIT_SUBJ_TYPE: case AUDIT_OBJ_TYPE: rc = -EINVAL; - typedatum = hashtab_search(policydb->p_types.table, rulestr); + typedatum = hashtab_search(&policydb->p_types.table, rulestr); if (!typedatum) goto out; tmprule->au_ctxt.type = typedatum->value; diff --git a/security/selinux/ss/symtab.c b/security/selinux/ss/symtab.c index dc2ce94165d3..92d7a948070e 100644 --- a/security/selinux/ss/symtab.c +++ b/security/selinux/ss/symtab.c @@ -35,10 +35,7 @@ static int symcmp(struct hashtab *h, const void *key1, const void *key2) int symtab_init(struct symtab *s, unsigned int size) { - s->table = hashtab_create(symhash, symcmp, size); - if (!s->table) - return -ENOMEM; s->nprim = 0; - return 0; + return hashtab_init(&s->table, symhash, symcmp, size); } diff --git a/security/selinux/ss/symtab.h b/security/selinux/ss/symtab.h index d75fcafe7281..f145301b9d9f 100644 --- a/security/selinux/ss/symtab.h +++ b/security/selinux/ss/symtab.h @@ -13,7 +13,7 @@ #include "hashtab.h" struct symtab { - struct hashtab *table; /* hash table (keyed on a string) */ + struct hashtab table; /* hash table (keyed on a string) */ u32 nprim; /* number of primary names in table */ }; From fe5a90b8c14914397a3bb0c214d142103c1ba3bf Mon Sep 17 00:00:00 2001 From: YueHaibing Date: Sat, 9 May 2020 19:18:52 +0800 Subject: [PATCH 12/12] selinux: netlabel: Remove unused inline function There's no callers in-tree. Signed-off-by: YueHaibing Signed-off-by: Paul Moore --- security/selinux/include/netlabel.h | 6 ------ 1 file changed, 6 deletions(-) diff --git a/security/selinux/include/netlabel.h b/security/selinux/include/netlabel.h index d30d8d7cdc9c..0c58f62dc6ab 100644 --- a/security/selinux/include/netlabel.h +++ b/security/selinux/include/netlabel.h @@ -98,12 +98,6 @@ static inline int selinux_netlbl_skbuff_setsid(struct sk_buff *skb, return 0; } -static inline int selinux_netlbl_conn_setsid(struct sock *sk, - struct sockaddr *addr) -{ - return 0; -} - static inline int selinux_netlbl_sctp_assoc_request(struct sctp_endpoint *ep, struct sk_buff *skb) {