BUGREPORT: A type alias of invisible primary one

BUGREPORT: A type alias of invisible primary one

[KaiGai Kohei]

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.

How should it be fixed on the next?
Please give me a suggestion.

Thanks,
-- 
OSS Platform Development Division, NEC
KaiGai Kohei <kaigai@ak.jp.nec.com>

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: BUGREPORT: A type alias of invisible primary one

[Joshua Brindle]

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?

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: BUGREPORT: A type alias of invisible primary one

[KaiGai Kohei]

[-- Attachment #1: Type: text/plain, Size: 2432 bytes --]

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,
-- 
OSS Platform Development Division, NEC
KaiGai Kohei <kaigai@ak.jp.nec.com>

[-- Attachment #2: libsepol-debugging.patch --]
[-- Type: text/x-patch, Size: 1455 bytes --]

Index: libsepol/src/expand.c
===================================================================
--- libsepol/src/expand.c	(revision 2950)
+++ libsepol/src/expand.c	(working copy)
@@ -90,6 +90,8 @@
 	}
 	if (!is_id_enabled(id, state->base, SYM_TYPES)) {
 		/* identifier's scope is not enabled */
+		INFO(state->handle, "type %s is skipped (s.value=%u primary=%u)",
+		     (char *)key, type->s.value, type->primary);
 		return 0;
 	}
 
@@ -516,6 +518,14 @@
 
 	new_alias->flags = alias->flags;
 
+	if (!new_alias->s.value) {
+		INFO(state->handle, "strange alias type : "
+		     "%s (a.value=%u, primary=%u, flavor=%u)"
+		     " => (a.value=%u, primary=%u, flavor=%u)",
+		     (char *) key, alias->s.value, alias->primary, alias->flavor,
+		     new_alias->s.value, new_alias->primary, new_alias->flavor);
+	}
+
 	ret = hashtab_insert(state->out->p_types.table,
 			     (hashtab_key_t) new_id,
 			     (hashtab_datum_t) new_alias);
Index: libsepol/src/write.c
===================================================================
--- libsepol/src/write.c	(revision 2950)
+++ libsepol/src/write.c	(working copy)
@@ -952,6 +952,12 @@
 
 	typdatum = (type_datum_t *) datum;
 
+	if (!typdatum->s.value) {
+		printf("%s: strange type %s (s.value=%u, primary=%u, flavor=%u)\n",
+		       __FUNCTION__, (char *)key, typdatum->s.value,
+		       typdatum->primary, typdatum->flavor);
+	}
+
 	len = strlen(key);
 	items = 0;
 	buf[items++] = cpu_to_le32(len);

Re: BUGREPORT: A type alias of invisible primary one

[Joshua Brindle]

[-- Attachment #1: Type: text/plain, Size: 3498 bytes --]

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);
 

[-- Attachment #2: alias-fix --]
[-- Type: text/plain, Size: 926 bytes --]

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);
 

Re: BUGREPORT: A type alias of invisible primary one

[KaiGai Kohei]

Joshua Brindle wrote:
> 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:

Your patch also works for me fine.
I injected a printf() at write_type, but no strange type can be found.

Thanks,
-- 
OSS Platform Development Division, NEC
KaiGai Kohei <kaigai@ak.jp.nec.com>

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: BUGREPORT: A type alias of invisible primary one

[Joshua Brindle]

Joshua Brindle wrote:
> 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:
>
>   

Merged in to libsepol-2.0.34


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.