2023-06-02 14:05:25

by Christian Göttsche

[permalink] [raw]
Subject: [RFC PATCH] selinux: support name based type transitions in conditional policies

With the move of name based type transitions to the avtab structure such
statements are candidate to be used in conditional policies. They are
already read into the policy database (which might be seen as a mistake
as referencing them after a boolean change causes a use-after-free).

Copy the avtab_trans structures on boolean changes to keep them
available.

CC: Juraj Marcin <[email protected]>
Signed-off-by: Christian Göttsche <[email protected]>
---
Based on patcheset by Juraj Marcin [1]

[1]: https://patchwork.kernel.org/project/selinux/list/?series=752711
---
security/selinux/ss/avtab.c | 2 +-
security/selinux/ss/avtab.h | 2 +
security/selinux/ss/conditional.c | 96 ++++++++++++++++++++++++++++---
3 files changed, 92 insertions(+), 8 deletions(-)

diff --git a/security/selinux/ss/avtab.c b/security/selinux/ss/avtab.c
index d3c027e5c0da..8068c9b4d1e6 100644
--- a/security/selinux/ss/avtab.c
+++ b/security/selinux/ss/avtab.c
@@ -297,7 +297,7 @@ static int avtab_trans_destroy_helper(void *k, void *d, void *args)
return 0;
}

-static void avtab_trans_destroy(struct avtab_trans *trans)
+void avtab_trans_destroy(struct avtab_trans *trans)
{
hashtab_map(&trans->name_trans.table, avtab_trans_destroy_helper, NULL);
hashtab_destroy(&trans->name_trans.table);
diff --git a/security/selinux/ss/avtab.h b/security/selinux/ss/avtab.h
index 929e322715d1..eb19d7719b72 100644
--- a/security/selinux/ss/avtab.h
+++ b/security/selinux/ss/avtab.h
@@ -125,6 +125,8 @@ struct avtab_node *avtab_search_node_next(struct avtab_node *node, int specified
#define MAX_AVTAB_HASH_BITS 16
#define MAX_AVTAB_HASH_BUCKETS (1 << MAX_AVTAB_HASH_BITS)

+void avtab_trans_destroy(struct avtab_trans *trans);
+
/* policydb filename transitions compatibility */

int avtab_filename_trans_read(struct avtab *a, void *fp, struct policydb *p);
diff --git a/security/selinux/ss/conditional.c b/security/selinux/ss/conditional.c
index 91392d65563e..48b162dca3f5 100644
--- a/security/selinux/ss/conditional.c
+++ b/security/selinux/ss/conditional.c
@@ -605,11 +605,36 @@ void cond_compute_av(struct avtab *ctab, struct avtab_key *key,
}
}

+static int cond_avtab_trans_destroy_helper(void *k, void *d, __always_unused void *args)
+{
+ kfree(k);
+ kfree(d);
+ return 0;
+}
+
+static int cond_avtab_trans_copy(struct hashtab_node *new, const struct hashtab_node *orig,
+ __always_unused void *args)
+{
+ new->key = kstrdup(orig->key, GFP_KERNEL);
+ if (!new->key)
+ return -ENOMEM;
+
+ new->datum = kmemdup(orig->datum, sizeof(u32), GFP_KERNEL);
+ if (!new->datum) {
+ kfree(new->key);
+ return -ENOMEM;
+ }
+
+ return 0;
+}
+
static int cond_dup_av_list(struct cond_av_list *new,
- struct cond_av_list *orig,
- struct avtab *avtab)
+ const struct cond_av_list *orig,
+ struct avtab *avtab,
+ const struct policydb *origp)
{
u32 i;
+ int rc;

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

@@ -618,9 +643,66 @@ static int cond_dup_av_list(struct cond_av_list *new,
return -ENOMEM;

for (i = 0; i < orig->len; i++) {
- new->nodes[i] = avtab_insert_nonunique(avtab,
- &orig->nodes[i]->key,
- &orig->nodes[i]->datum);
+ const struct avtab_key *orig_key = &orig->nodes[i]->key;
+ struct avtab_datum datum = orig->nodes[i]->datum;
+
+ if (origp->policyvers >= POLICYDB_VERSION_AVTAB_FTRANS &&
+ (orig_key->specified & AVTAB_TRANSITION)) {
+ struct avtab_trans trans = {
+ .otype = datum.u.trans->otype,
+ };
+
+ rc = symtab_init(&trans.name_trans,
+ datum.u.trans->name_trans.table.nel);
+ if (rc) {
+ avtab_trans_destroy(&trans);
+ return rc;
+ }
+ rc = symtab_init(&trans.prefix_trans,
+ datum.u.trans->prefix_trans.table.nel);
+ if (rc) {
+ avtab_trans_destroy(&trans);
+ return rc;
+ }
+ rc = symtab_init(&trans.suffix_trans,
+ datum.u.trans->suffix_trans.table.nel);
+ if (rc) {
+ avtab_trans_destroy(&trans);
+ return rc;
+ }
+
+ rc = hashtab_duplicate(&trans.name_trans.table,
+ &datum.u.trans->name_trans.table,
+ cond_avtab_trans_copy,
+ cond_avtab_trans_destroy_helper,
+ NULL);
+ if (rc) {
+ avtab_trans_destroy(&trans);
+ return rc;
+ }
+ rc = hashtab_duplicate(&trans.prefix_trans.table,
+ &datum.u.trans->prefix_trans.table,
+ cond_avtab_trans_copy,
+ cond_avtab_trans_destroy_helper,
+ NULL);
+ if (rc) {
+ avtab_trans_destroy(&trans);
+ return rc;
+ }
+ rc = hashtab_duplicate(&trans.suffix_trans.table,
+ &datum.u.trans->suffix_trans.table,
+ cond_avtab_trans_copy,
+ cond_avtab_trans_destroy_helper,
+ NULL);
+ if (rc) {
+ avtab_trans_destroy(&trans);
+ return rc;
+ }
+
+ datum.u.trans = &trans;
+ }
+
+ new->nodes[i] = avtab_insert_nonunique(avtab, orig_key, &datum);
if (!new->nodes[i])
return -ENOMEM;
new->len++;
@@ -662,12 +744,12 @@ static int duplicate_policydb_cond_list(struct policydb *newp,
newn->expr.len = orign->expr.len;

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

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