On 04/15/2014 02:43 PM, Eric Paris wrote:
> On Tue, 2014-04-15 at 14:34 -0400, Stephen Smalley wrote:
>> On 04/15/2014 02:27 PM, Eric Paris wrote:
>>> On Tue, 2014-04-15 at 14:21 -0400, Stephen Smalley wrote:
>>>> On 04/15/2014 12:40 PM, Stephen Smalley wrote:
>>>>> On 04/15/2014 12:27 PM, Richard Haines wrote:
>>>>>> The current detection of duplicate rules does not cover the state->out
>>>>>> policy and therefore will duplicate filename transition rules if already
>>>>>> present.
>>>>>>
>>>>>> Signed-off-by: Richard Haines <richard_c_haines@xxxxxxxxxxxxxx>
>>>>>> ---
>>>>>> libsepol/src/expand.c | 14 ++++++++++++++
>>>>>> 1 file changed, 14 insertions(+)
>>>>>>
>>>>>> diff --git a/libsepol/src/expand.c b/libsepol/src/expand.c
>>>>>> index acb6906..e908fdb 100644
>>>>>> --- a/libsepol/src/expand.c
>>>>>> +++ b/libsepol/src/expand.c
>>>>>> @@ -1534,6 +1534,20 @@ static int expand_filename_trans(expand_state_t *state, filename_trans_rule_t *r
>>>>>> if (cur_trans)
>>>>>> continue;
>>>>>>
>>>>>> + /* Now check if duplicate rule in state->out policy */
>>>>>> + cur_trans = state->out->filename_trans;
>>>>>> +
>>>>>> + while (cur_trans) {
>>>>>> + if (cur_trans->stype == (i + 1) &&
>>>>>> + cur_trans->ttype == (j + 1) &&
>>>>>> + cur_trans->tclass == cur_rule->tclass &&
>>>>>> + !strcmp(cur_trans->name, cur_rule->name))
>>>>>> + break;
>>>>>> + cur_trans = cur_trans->next;
>>>>>> + }
>>>>>> + if (cur_trans)
>>>>>> + continue;
>>>>>> +
>>>>>> new_trans = malloc(sizeof(*new_trans));
>>>>>> if (!new_trans) {
>>>>>> ERR(state->handle, "Out of memory!");
>>>>>
>>>>> Isn't this effectively a revert of:
>>>>>
>>>>> commit a29f6820c52b60b9028298cde9962dd140bbf9ea
>>>>> Author: Adam Tkac <atkac@xxxxxxxxxx>
>>>>> Date: Fri May 25 17:55:08 2012 +0200
>>>>>
>>>>> libsepol: filename_trans: use some better sorting to compare and merge
>>>>>
>>>>> The kernel switched to using a hashtab for filename_trans rules in
>>>>> commit 2463c26d50adc282d19317013ba0ff473823ca47
>>>>> Author: Eric Paris <eparis@xxxxxxxxxx>
>>>>> Date: Thu Apr 28 15:11:21 2011 -0400
>>>>>
>>>>> SELinux: put name based create rules in a hashtable
>>>>>
>>>>> Is there a reason we don't do this in libsepol too?
>>>>
>>>> So if I am reading a29f68 correctly, it is completely wrong and should
>>>> just be reverted. That will fix the duplicate filename transition rules
>>>> if I am not mistaken. Then separately, we can look at bringing over the
>>>> switch to using a hashtab that was already done in the kernel and use
>>>> that to speed up this checking? Comments?
>>>
>>> I think that's a good idea. The kernel hashtab and this 'fix' were done
>>> about the same time. I intended to bring the kernel hashtab over and
>>> got distracted and then forgot about it...
>>>
>>> Shouldn't be a hard thing, and I believe should get us back to having
>>> bitwise the same policy in /sys/fs/selinux/policy as we have on disk...
>>
>> Looks like we have some other areas of divergence, e.g. range_tr hashtab
>> conversion, ebitmap optimizations, possibly others. The latter would
>> require a policy binary format change (new version) to make it fully
>> identical.
>>
>> Not sure how well even the hashtab is doing at this point as Fedora has
>>> 150,000 file name transition rules in its policy per sedispol output?
>> That seems a bit excessive.
>
> My understanding is that MANY of these are because of the expansion of
> attributes in the source and the parent object. If we could leave these
> as attributes it would shrink things and incredible amount...
Hmmm...Fedora policy doesn't build with that change reverted, since it
was failing to check all filename transition rules for conflicts:
# semodule -B
libsepol.expand_filename_trans: Conflicting filename trans rules
keyrings staff_gkeyringd_t gnome_home_t : dir otype1:data_home_t
otype2:gkeyringd_gnome_home_t
libsepol.expand_module: Error during expand
libsemanage.semanage_expand_sandbox: Expand module failed
semodule: Failed!
That's a bug in policy not in libsepol; it was just hidden by the bug in
libsepol.
