Email: kaigai@ak.jp.nec.com
Subject: Thread/Child-Domain Assignment (rev.6)
Date: Tue, 07 Oct 2008 15:39:45 +0900
>> Hmm....
>> It seems to me what you pointed out is a bug of my patch. It prevents to deliver
>> actual number of type/attribute symbols to policy file, but it is unclear why does
>> it makes libsepol ignore the policyvers.
>> (I guess it may be a separated matter.)
>>
>>> Rather than trying to calculate the length without attributes I just removed
>>> the attribute check. This causes attributes to be written for all versions,
>>> but this should not cause any problems at all.
>> The reason why I injected such an ad-hoc code is that we cannot decide the policy
>> version written when type_attr_remove() is invoked.
>> Is it impossible to move it to policydb_write()?
>> It is invoked after the policyvers is fixed by caller.
>
> It isn't impossible. You are going to have to make it walk to type
> symbol table to calculate the length without attributes, then write
> that length instead of the total symtab length.
The attached patch enables to fixup the number of type/attribute entries
to be written. The type_attr_uncount() decrements the number of attribute
entries skipped at type_write().
At first, I had a plan to invoke type_attr_remove() with
hashtab_map_remove_on_error(), but it means the given policydb structure
is modified at policydb_write() and implicit changes to external interface.
Differences from the previous version are here:
Signed-off-by: Joshua Brindle <method@manicmethod.com>
Email: method@manicmethod.com
Subject: BUGREPORT: A type alias of invisible primary one
Date: Mon, 22 Sep 2008 16:43:04 -0400
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:
Send again with the right date and time ;)
This removes the (apparently) unused expand_rule function
Signed-off-by: Joshua Brindle <method@manicmethod.com>
This is the same off-by-one bug that was already fixed in the kernel.
(According to my understanding neither of these bugs has security
implications)
Signed-off-by: Vesa-Matti Kari <vmkari@cc.helsinki.fi>
Signed-off-by: Joshua Brindle <method@manicmethod.com>
