The boundry format mapped the primary field to a boolean in the
properties bitmap. This is appropriate for the kernel policy, but in
modular policy the primary field may be an integer that indicates the
primary type that is being aliased. In this case, the primary value cannot
be assumed to be boolean.
This patch creates a new module format that writes out the primary value
as was done before the boundry format.
Signed-off-by: Caleb Case <ccase@tresys.com>
Signed-off-by: Joshua Brindle <method@manicmethod.com>
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>
