From ee4e1edbb0c3f1a623866f45bc07088365628747 Mon Sep 17 00:00:00 2001 From: Matthew Ife Date: Mon, 30 Nov 2020 10:01:21 +0000 Subject: [PATCH 1/3] Fix bug in policydb_write when attempting to correct policy for older versions. The current implementation fails as it doesn't remove scope declarations further in the modular policy. This fixes the problem by removing the offending entries in the hash table and the scope table. Steps to reproduce: Try to build the following module then make a module from an older release: module test 1.0.0; require { type default_t; } attribute_role new_atrole; checkmodule -M -m -c 12 -o test.mod test.te semodule_package -o test.pp -m test.mod semodule_package: Error while reading policy module from test.mod Failure occurs when the current module gets written out with the scope declaration intact. This is due to policydb.c:3913 doing a hashtab search on a scope key that is not in the symbol table. Signed-off-by: Matthew Ife --- libsepol/src/hashtab.c | 7 ++- libsepol/src/write.c | 126 +++++++++++++++++++++++++++-------------- 2 files changed, 89 insertions(+), 44 deletions(-) diff --git a/libsepol/src/hashtab.c b/libsepol/src/hashtab.c index 21143b7699..76b977a903 100644 --- a/libsepol/src/hashtab.c +++ b/libsepol/src/hashtab.c @@ -223,18 +223,19 @@ int hashtab_map(hashtab_t h, hashtab_datum_t d, void *args), void *args) { unsigned int i, ret; - hashtab_ptr_t cur; + hashtab_ptr_t cur, next; if (!h) return SEPOL_OK; for (i = 0; i < h->size; i++) { cur = h->htable[i]; - while (cur != NULL) { + while (curi != NULL) { + next = cur->next; ret = apply(cur->key, cur->datum, args); if (ret) return ret; - cur = cur->next; + cur = next; } } return SEPOL_OK; diff --git a/libsepol/src/write.c b/libsepol/src/write.c index 84bcaf3f57..dd418317a3 100644 --- a/libsepol/src/write.c +++ b/libsepol/src/write.c @@ -2143,30 +2143,67 @@ static int scope_write(hashtab_key_t key, hashtab_datum_t datum, void *ptr) return rc; } -static int type_attr_uncount(hashtab_key_t key __attribute__ ((unused)), - hashtab_datum_t datum, void *args) +static void role_write_destroy(hashtab_key_t key __attribute__ ((unused)), + hashtab_datum_t datum, + void *p __attribute__ ((unused))) { - type_datum_t *typdatum = datum; - uint32_t *p_nel = args; + role_datum_destroy((role_datum_t *) datum); + free(datum); +} - if (typdatum->flavor == TYPE_ATTRIB) { - /* uncount attribute from total number of types */ - (*p_nel)--; - } - return 0; +static void type_write_destroy(hashtab_key_t key __attribute__ ((unused)), + hashtab_datum_t datum, + void *p __attribute__ ((unused))) +{ + type_datum_destroy((type_datum_t *) datum); + free(datum); } -static int role_attr_uncount(hashtab_key_t key __attribute__ ((unused)), - hashtab_datum_t datum, void *args) +static void scope_write_destroy(hashtab_key_t key __attribute__ ((unused)), + hashtab_datum_t datum, + void *p __attribute__ ((unused))) { - role_datum_t *role = datum; - uint32_t *p_nel = args; + scope_datum_t *cur = (scope_datum_t *) datum; + if (cur != NULL) { + free(cur->decl_ids); + } + free(cur); +} - if (role->flavor == ROLE_ATTRIB) { - /* uncount attribute from total number of roles */ - (*p_nel)--; - } - return 0; +static void type_attr_filter(hashtab_key_t key, + hashtab_datum_t datum, void *args) +{ + type_datum_t *typdatum = datum; + scope_datum_t *scope = NULL; + policydb_t *p = (policydb_t *)args; + hashtab_t typetbl = p->symtab[SYM_TYPES].table; + hashtab_t scopetbl = p->scope[SYM_TYPES].table; + + if (typdatum->flavor == TYPE_ATTRIB) { + /* Remove the entry from the hash table and scope table */ + hashtab_remove(typetbl, key, type_write_destroy, typdatum); + scope = (scope_datum_t *)hashtab_search(scopetbl, key); + if (scope) + hashtab_remove(scopetbl, key, scope_write_destroy, scope); + } +} + +static void role_attr_filter(hashtab_key_t key, + hashtab_datum_t datum, void *args) +{ + role_datum_t *role = datum; + scope_datum_t *scope = NULL; + policydb_t *p = (policydb_t *)args; + hashtab_t roletbl = p->symtab[SYM_ROLES].table; + hashtab_t scopetbl = p->scope[SYM_ROLES].table; + + if (role->flavor == ROLE_ATTRIB) { + /* Remove the entry from the hash table and scope table */ + hashtab_remove(roletbl, key, role_write_destroy, role); + scope = (scope_datum_t *)hashtab_search(scopetbl, key); + if (scope) + hashtab_remove(scopetbl, key, scope_write_destroy, scope); + } } /* @@ -2300,29 +2337,36 @@ int policydb_write(policydb_t * p, struct policy_file *fp) buf[0] = cpu_to_le32(p->symtab[i].nprim); buf[1] = p->symtab[i].table->nel; - /* - * A special case when writing type/attribute symbol table. - * The kernel policy version less than 24 does not support - * to load entries of attribute, so we have to re-calculate - * the actual number of types except for attributes. - */ - if (i == SYM_TYPES && - p->policyvers < POLICYDB_VERSION_BOUNDARY && - p->policy_type == POLICY_KERN) { - hashtab_map(p->symtab[i].table, type_attr_uncount, &buf[1]); - } - - /* - * Another special case when writing role/attribute symbol - * table, role attributes are redundant for policy.X, or - * when the pp's version is not big enough. So deduct - * their numbers from p_roles.table->nel. - */ - if ((i == SYM_ROLES) && - ((p->policy_type == POLICY_KERN) || - (p->policy_type != POLICY_KERN && - p->policyvers < MOD_POLICYDB_VERSION_ROLEATTRIB))) - (void)hashtab_map(p->symtab[i].table, role_attr_uncount, &buf[1]); + /* + * A special case when writing type/attribute symbol table. + * The kernel policy version less than 24 does not support + * to load entries of attribute, so we filter the entries + * from the table. + */ + if (i == SYM_TYPES && + p->policyvers < POLICYDB_VERSION_BOUNDARY && + p->policy_type == POLICY_KERN) { + (void)hashtab_map(p->symtab[i].table, type_attr_filter, p); + if (buf[1] != p->symtab[i].table->nel) + WARN(fp->handle, "Discarding type attribute rules"); + buf[1] = p->symtab[i].table->nel; + } + + /* + * Another special case when writing role/attribute symbol + * table, role attributes are redundant for policy.X, or + * when the pp's version is not big enough. So filter the entries + * from the table. + */ + if ((i == SYM_ROLES) && + ((p->policy_type == POLICY_KERN) || + (p->policy_type != POLICY_KERN && + p->policyvers < MOD_POLICYDB_VERSION_ROLEATTRIB))) { + (void)hashtab_map(p->symtab[i].table, role_attr_filter, p); + if (buf[1] != p->symtab[i].table->nel) + WARN(fp->handle, "Discarding role attribute rules"); + buf[1] = p->symtab[i].table->nel; + } buf[1] = cpu_to_le32(buf[1]); items = put_entry(buf, sizeof(uint32_t), 2, fp); From 5c8d72771b35de798b11edae1a75e8eccdba5757 Mon Sep 17 00:00:00 2001 From: Matthew Ife Date: Mon, 30 Nov 2020 11:02:05 +0000 Subject: [PATCH 2/3] Smaller bugfixes and testing module works as imagined. Signed-off-by: Matthew Ife --- libsepol/src/hashtab.c | 2 +- libsepol/src/write.c | 8 ++++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/libsepol/src/hashtab.c b/libsepol/src/hashtab.c index 76b977a903..ff7ef63fcd 100644 --- a/libsepol/src/hashtab.c +++ b/libsepol/src/hashtab.c @@ -230,7 +230,7 @@ int hashtab_map(hashtab_t h, for (i = 0; i < h->size; i++) { cur = h->htable[i]; - while (curi != NULL) { + while (cur != NULL) { next = cur->next; ret = apply(cur->key, cur->datum, args); if (ret) diff --git a/libsepol/src/write.c b/libsepol/src/write.c index dd418317a3..5f6356b469 100644 --- a/libsepol/src/write.c +++ b/libsepol/src/write.c @@ -2170,7 +2170,7 @@ static void scope_write_destroy(hashtab_key_t key __attribute__ ((unused)), free(cur); } -static void type_attr_filter(hashtab_key_t key, +static int type_attr_filter(hashtab_key_t key, hashtab_datum_t datum, void *args) { type_datum_t *typdatum = datum; @@ -2186,9 +2186,11 @@ static void type_attr_filter(hashtab_key_t key, if (scope) hashtab_remove(scopetbl, key, scope_write_destroy, scope); } + + return 0; } -static void role_attr_filter(hashtab_key_t key, +static int role_attr_filter(hashtab_key_t key, hashtab_datum_t datum, void *args) { role_datum_t *role = datum; @@ -2204,6 +2206,8 @@ static void role_attr_filter(hashtab_key_t key, if (scope) hashtab_remove(scopetbl, key, scope_write_destroy, scope); } + + return 0; } /* From 2299634fd49482b8a044a057ff3d043cf664f152 Mon Sep 17 00:00:00 2001 From: Matthew Ife Date: Mon, 30 Nov 2020 11:37:24 +0000 Subject: [PATCH 3/3] Retab me. Signed-off-by: Matthew Ife --- libsepol/src/write.c | 54 ++++++++++++++++++++++---------------------- 1 file changed, 27 insertions(+), 27 deletions(-) diff --git a/libsepol/src/write.c b/libsepol/src/write.c index 5f6356b469..6a59a0c34b 100644 --- a/libsepol/src/write.c +++ b/libsepol/src/write.c @@ -2341,36 +2341,36 @@ int policydb_write(policydb_t * p, struct policy_file *fp) buf[0] = cpu_to_le32(p->symtab[i].nprim); buf[1] = p->symtab[i].table->nel; - /* - * A special case when writing type/attribute symbol table. - * The kernel policy version less than 24 does not support - * to load entries of attribute, so we filter the entries - * from the table. - */ - if (i == SYM_TYPES && - p->policyvers < POLICYDB_VERSION_BOUNDARY && - p->policy_type == POLICY_KERN) { - (void)hashtab_map(p->symtab[i].table, type_attr_filter, p); - if (buf[1] != p->symtab[i].table->nel) + /* + * A special case when writing type/attribute symbol table. + * The kernel policy version less than 24 does not support + * to load entries of attribute, so we filter the entries + * from the table. + */ + if (i == SYM_TYPES && + p->policyvers < POLICYDB_VERSION_BOUNDARY && + p->policy_type == POLICY_KERN) { + (void)hashtab_map(p->symtab[i].table, type_attr_filter, p); + if (buf[1] != p->symtab[i].table->nel) WARN(fp->handle, "Discarding type attribute rules"); - buf[1] = p->symtab[i].table->nel; - } - - /* - * Another special case when writing role/attribute symbol - * table, role attributes are redundant for policy.X, or - * when the pp's version is not big enough. So filter the entries - * from the table. - */ - if ((i == SYM_ROLES) && - ((p->policy_type == POLICY_KERN) || - (p->policy_type != POLICY_KERN && - p->policyvers < MOD_POLICYDB_VERSION_ROLEATTRIB))) { - (void)hashtab_map(p->symtab[i].table, role_attr_filter, p); + buf[1] = p->symtab[i].table->nel; + } + + /* + * Another special case when writing role/attribute symbol + * table, role attributes are redundant for policy.X, or + * when the pp's version is not big enough. So filter the entries + * from the table. + */ + if ((i == SYM_ROLES) && + ((p->policy_type == POLICY_KERN) || + (p->policy_type != POLICY_KERN && + p->policyvers < MOD_POLICYDB_VERSION_ROLEATTRIB))) { + (void)hashtab_map(p->symtab[i].table, role_attr_filter, p); if (buf[1] != p->symtab[i].table->nel) WARN(fp->handle, "Discarding role attribute rules"); - buf[1] = p->symtab[i].table->nel; - } + buf[1] = p->symtab[i].table->nel; + } buf[1] = cpu_to_le32(buf[1]); items = put_entry(buf, sizeof(uint32_t), 2, fp);