Skip to content

Commit

Permalink
selinux: refactor changing booleans
Browse files Browse the repository at this point in the history
Refactor the logic for changing SELinux policy booleans in a similar
manner to the refactoring of policy load, thereby reducing the
size of the critical section when the policy write-lock is held
and making it easier to convert the policy rwlock to RCU in the
future.  Instead of directly modifying the policydb in place, modify
a copy and then swap it into place through a single pointer update.
Only fully copy the portions of the policydb that are affected by
boolean changes to avoid the full cost of a deep policydb copy.
Introduce another level of indirection for the sidtab since changing
booleans does not require updating the sidtab, unlike policy load.
While we are here, create a common helper for notifying
other kernel components and userspace of a policy change and call it
from both security_set_bools() and selinux_policy_commit().

Based on an old (2004) patch by Kaigai Kohei [1] to convert the policy
rwlock to RCU that was deferred at the time since it did not
significantly improve performance and introduced complexity. Peter
Enderborg later submitted a patch series to convert to RCU [2] that
would have made changing booleans a much more expensive operation
by requiring a full policydb_write();policydb_read(); sequence to
deep copy the entire policydb and also had concerns regarding
atomic allocations.

This change is now simplified by the earlier work to encapsulate
policy state in the selinux_policy struct and to refactor
policy load.  After this change, the last major obstacle to
converting the policy rwlock to RCU is likely the sidtab live
convert support.

[1] https://lore.kernel.org/selinux/[email protected]/
[2] https://lore.kernel.org/selinux/[email protected]/

Signed-off-by: Stephen Smalley <[email protected]>
Signed-off-by: Paul Moore <[email protected]>
  • Loading branch information
stephensmalley authored and pcmoore committed Aug 18, 2020
1 parent 02a52c5 commit c7c556f
Show file tree
Hide file tree
Showing 8 changed files with 368 additions and 64 deletions.
49 changes: 48 additions & 1 deletion security/selinux/ss/avtab.c
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,6 @@ void avtab_destroy(struct avtab *h)

void avtab_init(struct avtab *h)
{
kvfree(h->htable);
h->htable = NULL;
h->nel = 0;
}
Expand Down Expand Up @@ -340,6 +339,54 @@ int avtab_alloc(struct avtab *h, u32 nrules)
return 0;
}

int avtab_duplicate(struct avtab *new, struct avtab *orig)
{
int i;
struct avtab_node *node, *tmp, *tail;

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

new->htable = kvcalloc(orig->nslot, sizeof(void *), GFP_KERNEL);
if (!new->htable)
return -ENOMEM;
new->nslot = orig->nslot;
new->mask = orig->mask;

for (i = 0; i < orig->nslot; i++) {
tail = NULL;
for (node = orig->htable[i]; node; node = node->next) {
tmp = kmem_cache_zalloc(avtab_node_cachep, GFP_KERNEL);
if (!tmp)
goto error;
tmp->key = node->key;
if (tmp->key.specified & AVTAB_XPERMS) {
tmp->datum.u.xperms =
kmem_cache_zalloc(avtab_xperms_cachep,
GFP_KERNEL);
if (!tmp->datum.u.xperms) {
kmem_cache_free(avtab_node_cachep, tmp);
goto error;
}
tmp->datum.u.xperms = node->datum.u.xperms;
} else
tmp->datum.u.data = node->datum.u.data;

if (tail)
tail->next = tmp;
else
new->htable[i] = tmp;

tail = tmp;
new->nel++;
}
}

return 0;
error:
avtab_destroy(new);
return -ENOMEM;
}

void avtab_hash_eval(struct avtab *h, char *tag)
{
int i, chain_len, slots_used, max_chain_len;
Expand Down
1 change: 1 addition & 0 deletions security/selinux/ss/avtab.h
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ struct avtab {

void avtab_init(struct avtab *h);
int avtab_alloc(struct avtab *, u32);
int avtab_duplicate(struct avtab *new, struct avtab *orig);
struct avtab_datum *avtab_search(struct avtab *h, struct avtab_key *k);
void avtab_destroy(struct avtab *h);
void avtab_hash_eval(struct avtab *h, char *tag);
Expand Down
156 changes: 156 additions & 0 deletions security/selinux/ss/conditional.c
Original file line number Diff line number Diff line change
Expand Up @@ -600,3 +600,159 @@ void cond_compute_av(struct avtab *ctab, struct avtab_key *key,
services_compute_xperms_drivers(xperms, node);
}
}

static int cond_dup_av_list(struct cond_av_list *new,
struct cond_av_list *orig,
struct avtab *avtab)
{
struct avtab_node *avnode;
u32 i;

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

new->nodes = kcalloc(orig->len, sizeof(*new->nodes), GFP_KERNEL);
if (!new->nodes)
return -ENOMEM;

for (i = 0; i < orig->len; i++) {
avnode = avtab_search_node(avtab, &orig->nodes[i]->key);
if (WARN_ON(!avnode))
return -EINVAL;
new->nodes[i] = avnode;
new->len++;
}

return 0;
}

static int duplicate_policydb_cond_list(struct policydb *newp,
struct policydb *origp)
{
int rc, i, j;

rc = avtab_duplicate(&newp->te_cond_avtab, &origp->te_cond_avtab);
if (rc)
return rc;

newp->cond_list_len = 0;
newp->cond_list = kcalloc(origp->cond_list_len,
sizeof(*newp->cond_list),
GFP_KERNEL);
if (!newp->cond_list)
goto error;

for (i = 0; i < origp->cond_list_len; i++) {
struct cond_node *newn = &newp->cond_list[i];
struct cond_node *orign = &origp->cond_list[i];

newp->cond_list_len++;

newn->cur_state = orign->cur_state;
newn->expr.nodes = kcalloc(orign->expr.len,
sizeof(*newn->expr.nodes), GFP_KERNEL);
if (!newn->expr.nodes)
goto error;
for (j = 0; j < orign->expr.len; j++)
newn->expr.nodes[j] = orign->expr.nodes[j];
newn->expr.len = orign->expr.len;

rc = cond_dup_av_list(&newn->true_list, &orign->true_list,
&newp->te_cond_avtab);
if (rc)
goto error;

rc = cond_dup_av_list(&newn->false_list, &orign->false_list,
&newp->te_cond_avtab);
if (rc)
goto error;
}

return 0;

error:
avtab_destroy(&newp->te_cond_avtab);
cond_list_destroy(newp);
return -ENOMEM;
}

static int cond_bools_destroy(void *key, void *datum, void *args)
{
/* key was not copied so no need to free here */
kfree(datum);
return 0;
}

static int cond_bools_copy(struct hashtab_node *new, struct hashtab_node *orig, void *args)
{
struct cond_bool_datum *datum;

datum = kmalloc(sizeof(struct cond_bool_datum), GFP_KERNEL);
if (!datum)
return -ENOMEM;

memcpy(datum, orig->datum, sizeof(struct cond_bool_datum));

new->key = orig->key; /* No need to copy, never modified */
new->datum = datum;
return 0;
}

static int cond_bools_index(void *key, void *datum, void *args)
{
struct cond_bool_datum *booldatum, **cond_bool_array;

booldatum = datum;
cond_bool_array = args;
cond_bool_array[booldatum->value - 1] = booldatum;

return 0;
}

static int duplicate_policydb_bools(struct policydb *newdb,
struct policydb *orig)
{
struct cond_bool_datum **cond_bool_array;
int rc;

cond_bool_array = kmalloc_array(orig->p_bools.nprim,
sizeof(*orig->bool_val_to_struct),
GFP_KERNEL);
if (!cond_bool_array)
return -ENOMEM;

rc = hashtab_duplicate(&newdb->p_bools.table, &orig->p_bools.table,
cond_bools_copy, cond_bools_destroy, NULL);
if (rc) {
kfree(cond_bool_array);
return -ENOMEM;
}

hashtab_map(&newdb->p_bools.table, cond_bools_index, cond_bool_array);
newdb->bool_val_to_struct = cond_bool_array;

newdb->p_bools.nprim = orig->p_bools.nprim;

return 0;
}

void cond_policydb_destroy_dup(struct policydb *p)
{
hashtab_map(&p->p_bools.table, cond_bools_destroy, NULL);
hashtab_destroy(&p->p_bools.table);
cond_policydb_destroy(p);
}

int cond_policydb_dup(struct policydb *new, struct policydb *orig)
{
cond_policydb_init(new);

if (duplicate_policydb_bools(new, orig))
return -ENOMEM;

if (duplicate_policydb_cond_list(new, orig)) {
cond_policydb_destroy_dup(new);
return -ENOMEM;
}

return 0;
}
2 changes: 2 additions & 0 deletions security/selinux/ss/conditional.h
Original file line number Diff line number Diff line change
Expand Up @@ -79,5 +79,7 @@ void cond_compute_av(struct avtab *ctab, struct avtab_key *key,
void cond_compute_xperms(struct avtab *ctab, struct avtab_key *key,
struct extended_perms_decision *xpermd);
void evaluate_cond_nodes(struct policydb *p);
void cond_policydb_destroy_dup(struct policydb *p);
int cond_policydb_dup(struct policydb *new, struct policydb *orig);

#endif /* _CONDITIONAL_H_ */
53 changes: 53 additions & 0 deletions security/selinux/ss/hashtab.c
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,59 @@ void hashtab_stat(struct hashtab *h, struct hashtab_info *info)
info->max_chain_len = max_chain_len;
}

int hashtab_duplicate(struct hashtab *new, struct hashtab *orig,
int (*copy)(struct hashtab_node *new,
struct hashtab_node *orig, void *args),
int (*destroy)(void *k, void *d, void *args),
void *args)
{
struct hashtab_node *cur, *tmp, *tail;
int i, rc;

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

new->htable = kcalloc(orig->size, sizeof(*new->htable), GFP_KERNEL);
if (!new->htable)
return -ENOMEM;

new->size = orig->size;

for (i = 0; i < orig->size; i++) {
tail = NULL;
for (cur = orig->htable[i]; cur; cur = cur->next) {
tmp = kmem_cache_zalloc(hashtab_node_cachep,
GFP_KERNEL);
if (!tmp)
goto error;
rc = copy(tmp, cur, args);
if (rc) {
kmem_cache_free(hashtab_node_cachep, tmp);
goto error;
}
tmp->next = NULL;
if (!tail)
new->htable[i] = tmp;
else
tail->next = tmp;
tail = tmp;
new->nel++;
}
}

return 0;

error:
for (i = 0; i < new->size; i++) {
for (cur = new->htable[i]; cur; cur = tmp) {
tmp = cur->next;
destroy(cur->key, cur->datum, args);
kmem_cache_free(hashtab_node_cachep, cur);
}
}
kmem_cache_free(hashtab_node_cachep, new);
return -ENOMEM;
}

void __init hashtab_cache_init(void)
{
hashtab_node_cachep = kmem_cache_create("hashtab_node",
Expand Down
6 changes: 6 additions & 0 deletions security/selinux/ss/hashtab.h
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,12 @@ int hashtab_map(struct hashtab *h,
int (*apply)(void *k, void *d, void *args),
void *args);

int hashtab_duplicate(struct hashtab *new, struct hashtab *orig,
int (*copy)(struct hashtab_node *new,
struct hashtab_node *orig, void *args),
int (*destroy)(void *k, void *d, void *args),
void *args);

/* Fill info with some hash table statistics */
void hashtab_stat(struct hashtab *h, struct hashtab_info *info);

Expand Down
Loading

0 comments on commit c7c556f

Please sign in to comment.