Matthew Ife <matthew@xxxxxxx> writes:
> Stumbled over this one when writing a module from F33 to EL6.
>
> 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;
>
> Build
>
> $ 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
>
> With fix:
>
> $ checkmodule -o test.mod -M -m -c12 test.te
> libsepol.policydb_write: Discarding role attribute rules
> $ semodule_package -o test.pp -m test.mod
>
> Failure occurs when the current module gets written out as the scope
> declaration remains intact.
> semodule_package files correctly at policydb.c:3913 doing a hash table
> search on a scope key that is not
> in the symbol table.
>
> This patch fixes the problem by removing the hashtable entries and
> scope declarations properly prior to module write and emits a warning
> to the user of the unsupported statements.
>
> Also altered hashtab_map slightly to allow it to be used for
> hashtab_remove calls in order to support the patch.
>
> I submitted a pull request for this (there is some spacing/tabbing
> issues actually so I can resent a new pull request if necessary).
>
> https://github.com/SELinuxProject/selinux/pull/273
Hello,
This particular patch can't be currently applied to master's HEAD and
it's also missing Signed-of-by: line
It looks like you've rebased it in
https://github.com/SELinuxProject/selinux/pull/273 , added Signed-of-by
and also added more commits. When you think it's a final version, please
merge/squash related commits - e.g. "Retab me" looks like fix up of the first
commit - and resend the patches or patches again to this list for
review.
Thanks!
> diff --git a/libsepol/src/hashtab.c b/libsepol/src/hashtab.c
> index 76b977a9..ff7ef63f 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 dd418317..6a59a0c3 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;
> }
>
> /*
> @@ -2337,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);
