Skip to content

Commit

Permalink
selinux: reduce the use of hard-coded hash sizes
Browse files Browse the repository at this point in the history 
Instead allocate hash tables with just the right size based on the
actual number of elements (which is almost always known beforehand, we
just need to defer the hashtab allocation to the right time). The only
case when we don't know the size (with the current policy format) is the
new filename transitions hashtable. Here I just left the existing value.

After this patch, the time to load Fedora policy on x86_64 decreases
from 790 ms to 167 ms. If the unconfined module is removed, it decreases
from 750 ms to 122 ms. It is also likely that other operations are going
to be faster, mainly string_to_context_struct() or mls_compute_sid(),
but I didn't try to quantify that.

The memory usage of all hash table arrays increases from ~58 KB to
~163 KB (with Fedora policy on x86_64).

Signed-off-by: Ondrej Mosnacek <[email protected]>
Acked-by: Stephen Smalley <[email protected]>
Signed-off-by: Paul Moore <[email protected]>
  • Loading branch information
@WOnder93 @pcmoore
WOnder93 authored and pcmoore committed Feb 28, 2020
1 parent e4cfa05 commit e0ac568
Show file tree
Hide file tree
Showing 4 changed files with 45 additions and 40 deletions.
28 changes: 24 additions & 4 deletions security/selinux/ss/hashtab.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,29 @@

static struct kmem_cache *hashtab_node_cachep;

/*
* Here we simply round the number of elements up to the nearest power of two.
* I tried also other options like rouding down or rounding to the closest
* power of two (up or down based on which is closer), but I was unable to
* find any significant difference in lookup/insert performance that would
* justify switching to a different (less intuitive) formula. It could be that
* a different formula is actually more optimal, but any future changes here
* should be supported with performance/memory usage data.
*
* The total memory used by the htable arrays (only) with Fedora policy loaded
* is approximately 163 KB at the time of writing.
*/
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 size)
u32 nel_hint)
{
struct hashtab *p;
u32 i;
u32 i, size = hashtab_compute_size(nel_hint);

p = kzalloc(sizeof(*p), GFP_KERNEL);
if (!p)
Expand All @@ -27,6 +44,9 @@ struct hashtab *hashtab_create(u32 (*hash_value)(struct hashtab *h, const void *
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);
Expand All @@ -46,7 +66,7 @@ int hashtab_insert(struct hashtab *h, void *key, void *datum)

cond_resched();

if (!h || h->nel == HASHTAB_MAX_NODES)
if (!h || !h->size || h->nel == HASHTAB_MAX_NODES)
return -EINVAL;

hvalue = h->hash_value(h, key);
Expand Down Expand Up @@ -82,7 +102,7 @@ void *hashtab_search(struct hashtab *h, const void *key)
u32 hvalue;
struct hashtab_node *cur;

if (!h)
if (!h || !h->size)
return NULL;

hvalue = h->hash_value(h, key);
Expand Down
2 changes: 1 addition & 1 deletion security/selinux/ss/hashtab.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ struct hashtab_info {
*/
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 size);
u32 nel_hint);

/*
* Inserts the specified (key, datum) pair into the specified hash table.
Expand Down
53 changes: 20 additions & 33 deletions security/selinux/ss/policydb.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,17 +56,6 @@ static const char *symtab_name[SYM_NUM] = {
};
#endif

static unsigned int symtab_sizes[SYM_NUM] = {
2,
32,
16,
512,
128,
16,
16,
16,
};

struct policydb_compat_info {
int version;
int sym_num;
Expand Down Expand Up @@ -478,20 +467,10 @@ static int policydb_init(struct policydb *p)

memset(p, 0, sizeof(*p));

for (i = 0; i < SYM_NUM; i++) {
rc = symtab_init(&p->symtab[i], symtab_sizes[i]);
if (rc)
goto out;
}

rc = avtab_init(&p->te_avtab);
if (rc)
goto out;

rc = roles_init(p);
if (rc)
goto out;

rc = cond_policydb_init(p);
if (rc)
goto out;
Expand All @@ -503,20 +482,12 @@ static int policydb_init(struct policydb *p)
goto out;
}

p->range_tr = hashtab_create(rangetr_hash, rangetr_cmp, 256);
if (!p->range_tr) {
rc = -ENOMEM;
goto out;
}

ebitmap_init(&p->filename_trans_ttypes);
ebitmap_init(&p->policycaps);
ebitmap_init(&p->permissive_map);

return 0;
out:
hashtab_destroy(p->filename_trans);
hashtab_destroy(p->range_tr);
for (i = 0; i < SYM_NUM; i++) {
hashtab_map(p->symtab[i].table, destroy_f[i], NULL);
hashtab_destroy(p->symtab[i].table);
Expand Down Expand Up @@ -1142,12 +1113,12 @@ static int common_read(struct policydb *p, struct hashtab *h, void *fp)

len = le32_to_cpu(buf[0]);
comdatum->value = le32_to_cpu(buf[1]);
nel = le32_to_cpu(buf[3]);

rc = symtab_init(&comdatum->permissions, PERM_SYMTAB_SIZE);
rc = symtab_init(&comdatum->permissions, nel);
if (rc)
goto bad;
comdatum->permissions.nprim = le32_to_cpu(buf[2]);
nel = le32_to_cpu(buf[3]);

rc = str_read(&key, GFP_KERNEL, fp, len);
if (rc)
Expand Down Expand Up @@ -1308,12 +1279,12 @@ static int class_read(struct policydb *p, struct hashtab *h, void *fp)
len = le32_to_cpu(buf[0]);
len2 = le32_to_cpu(buf[1]);
cladatum->value = le32_to_cpu(buf[2]);
nel = le32_to_cpu(buf[4]);

rc = symtab_init(&cladatum->permissions, PERM_SYMTAB_SIZE);
rc = symtab_init(&cladatum->permissions, nel);
if (rc)
goto bad;
cladatum->permissions.nprim = le32_to_cpu(buf[3]);
nel = le32_to_cpu(buf[4]);

ncons = le32_to_cpu(buf[5]);

Expand Down Expand Up @@ -1826,6 +1797,11 @@ static int range_read(struct policydb *p, void *fp)
return rc;

nel = le32_to_cpu(buf[0]);

p->range_tr = hashtab_create(rangetr_hash, rangetr_cmp, nel);
if (!p->range_tr)
return -ENOMEM;

for (i = 0; i < nel; i++) {
rc = -ENOMEM;
rt = kzalloc(sizeof(*rt), GFP_KERNEL);
Expand Down Expand Up @@ -2418,6 +2394,17 @@ int policydb_read(struct policydb *p, void *fp)
goto bad;
nprim = le32_to_cpu(buf[0]);
nel = le32_to_cpu(buf[1]);

rc = symtab_init(&p->symtab[i], nel);
if (rc)
goto out;

if (i == SYM_ROLES) {
rc = roles_init(p);
if (rc)
goto out;
}

for (j = 0; j < nel; j++) {
rc = read_f[i](p, p->symtab[i].table, fp);
if (rc)
Expand Down
2 changes: 0 additions & 2 deletions security/selinux/ss/policydb.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -321,8 +321,6 @@ extern int policydb_role_isvalid(struct policydb *p, unsigned int role);
extern int policydb_read(struct policydb *p, void *fp);
extern int policydb_write(struct policydb *p, void *fp);

#define PERM_SYMTAB_SIZE 32

#define POLICYDB_CONFIG_MLS 1

/* the config flags related to unknown classes/perms are bits 2 and 3 */
Expand Down

0 comments on commit e0ac568

Please sign in to comment.