Re: Fix Checkmodule when Writing down to older Module Versions

[Petr Lautrbach <plautrba@redhat.com>]

Matthew Ife <matthew@ife.onl> 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);