KaiGai Kohei wrote:
> Joshua Brindle wrote:
>> KaiGai Kohei wrote:
>>> I found a strange type_datum_t object which has 0 for its s.value
>>> during development of new type hierarchy checks.
>>>
>>> The strange one is "xguest_javaplugin_default_xproperty_t" which
>>> is an alias type of "xguest_javaplugin_xproperty_t".
>>>
>>> I doubted my patch at first, but it can be reproduced on the normal
>>> libsepol. It seems to me an original matter which is not exposed yet,
>>> and I am innocence. :-)
>>>
>>> During tracing the matter, I noticed the primary type is invisible
>>> at expand_module(), but the aliased one is visible. It can make the
>>> strange type_datum_t object.
>>>
>>> * at the expand_module()
>>> 1. The expand_state_t which includes typemap is initialized.
>>>
>>> 2. The type_copy_callback is invoked for any types via hashtab_map.
>>> It only copies primary and visible types into newer hashtab,
>>> and set up typemap to translate between old and new s.value.
>>> Thus, the given primary type is invisible, its slot of typemap
>>> is kept to zero.
>>> (*) is_id_enabled() for "xguest_javaplugin_xproperty_t" returned false.
>>>
>>> 3. The alias_copy_callback is invoked for any types via hashtab_map.
>>> It only copies alias and visible types into newer hashtab.
>>> Here is no check whether the primary side is visible, or not.
>>> A copied type_datum_t object for the given alias has new s.value
>>> which is picked up from state->typemap.
>>>
>>> 4. However, the target slot of state->typemap was zero, because
>>> its primary one is invisible. The aliased type has a strange
>>> s.value.
>>>
>>> 5. Type hierarchy checks got a segmentation fault, due to
>>> "p->type_val_to_name[datum->s.value - 1]".
>>> ^^^^^^^^^^^^^^^^^^ == -1
>>> Yes, we can identify cause of the matter.
>> Do you have a policy that can be used to reproduce this?
>
> Yes, the following policy can reproduce the matter.
> - - - - [ cut here ] - - - -
> policy_module(baz, 1.0)
>
> optional_policy(`
> gen_require(`
> type invisible_primary_t;
> ')
> typealias invisible_primary_t alias visible_alias_t;
> ')
> - - - - - - - - - - - - - - -
>
> The attached patch can inject some of printf()'s.
> You can see that invisible_primary_t is skipped at type_copy_callback()
> and an incorrect s.value is assigned at alias_copy_callback().
>
> Thanks,
>
This should fix it. I tested with and without your patchset on a few policies. Let me know if it doesn't work for you:
diff --git a/libsepol/src/expand.c b/libsepol/src/expand.c
index 14dc4fc..2e54177 100644
--- a/libsepol/src/expand.c
+++ b/libsepol/src/expand.c
@@ -478,6 +478,7 @@ static int alias_copy_callback(hashtab_key_t key, hashtab_datum_t datum,
char *id, *new_id;
type_datum_t *alias, *new_alias;
expand_state_t *state;
+ uint32_t prival;
id = (char *)key;
alias = (type_datum_t *) datum;
@@ -491,6 +492,18 @@ static int alias_copy_callback(hashtab_key_t key, hashtab_datum_t datum,
if (alias->flavor == TYPE_ATTRIB)
return 0;
+ if (alias->flavor == TYPE_ALIAS)
+ prival = alias->primary;
+ else
+ prival = alias->s.value;
+
+ if (!is_id_enabled(state->base->p_type_val_to_name[prival - 1],
+ state->base, SYM_TYPES)) {
+ /* The primary type for this alias is not enabled, the alias
+ * shouldn't be either */
+ return 0;
+ }
+
if (state->verbose)
INFO(state->handle, "copying alias %s", id);
diff --git a/libsepol/src/expand.c b/libsepol/src/expand.c
index 14dc4fc..2e54177 100644
--- a/libsepol/src/expand.c
+++ b/libsepol/src/expand.c
@@ -478,6 +478,7 @@ static int alias_copy_callback(hashtab_key_t key, hashtab_datum_t datum,
char *id, *new_id;
type_datum_t *alias, *new_alias;
expand_state_t *state;
+ uint32_t prival;
id = (char *)key;
alias = (type_datum_t *) datum;
@@ -491,6 +492,18 @@ static int alias_copy_callback(hashtab_key_t key, hashtab_datum_t datum,
if (alias->flavor == TYPE_ATTRIB)
return 0;
+ if (alias->flavor == TYPE_ALIAS)
+ prival = alias->primary;
+ else
+ prival = alias->s.value;
+
+ if (!is_id_enabled(state->base->p_type_val_to_name[prival - 1],
+ state->base, SYM_TYPES)) {
+ /* The primary type for this alias is not enabled, the alias
+ * shouldn't be either */
+ return 0;
+ }
+
if (state->verbose)
INFO(state->handle, "copying alias %s", id);
