Re: [PATCH] checkpolicy: Fix bug in handling type declaration in optional block.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 01/18/2017 10:15 PM, Dominick Grift wrote:
> On 01/18/2017 10:09 PM, James Carter wrote:
>> On 01/18/2017 03:58 PM, Dominick Grift wrote:
>>> On 01/18/2017 09:53 PM, James Carter wrote:
>>>> Nicolas Iooss discovered that requiring a type in an optional block
>>>> after the type has already been declared in another optional block
>>>> results in a duplicate declaration error.
>>>>
>>>
>>> from what i have been told and from experience, types cannot, reliably,
>>> be declared in optional blocks.
>>>
>>> if the above is true, then the compiler should not allow one to declare
>>> a type in an optional block in the first place
>>>
>>
>> Well, this ordering issue reported by Nicolas would definitely cause
>> their declaration to be unreliable. ;)
>>
>> I hope to make declaring types in optional blocks reliable.
> 
> That would be nice, but just so you know. This was not the issue i was
> encountering with declaring types in optionals. So even though this
> might have made things unreliable. It was not what made it unreliable
> for me.
> 

Actually, sorry. I do actually recognize the second scenario (duplicate
declaration). So this might in fact be related. I can't believe that
this ancient bug would finally be fixed. Ive been lead to believe that
this was just a fundamental limitation of module policy all this time.
My whole policy was structured on the assumption that types cannot
reliably be declared in optionals....

>>
>> Jim
>>
>>>> The following policy will trigger the error.
>>>>
>>>> optional {
>>>>   type T1;
>>>> }
>>>>
>>>> optional {
>>>>   require { type T1; }
>>>> }
>>>>
>>>> In this case, although symtab_insert() in libsepol properly identifies
>>>> that the first T1 is a declaration while the second is a require, it
>>>> will return -2 which is the return value for a duplicate declaration
>>>> and expect the calling code to properly handle the situation. The
>>>> caller is require_symbol() in checkpolicy and it checks if the previous
>>>> declaration is in the current scope. If it is, then the require can be
>>>> ignored. It also checks to see if the declaration is a type and the
>>>> require an attribute or vice versa which is an error. The problem is
>>>> that -2 is returned if the declaration is not in scope which is
>>>> interpreted as a duplicate declaration error.
>>>>
>>>> The function should return 1 instead which means that they symbol was
>>>> not
>>>> added and needs to be freed later.
>>>>
>>>> Signed-off-by: James Carter <jwcart2@xxxxxxxxxxxxx>
>>>> ---
>>>>  checkpolicy/module_compiler.c | 16 +++-------------
>>>>  1 file changed, 3 insertions(+), 13 deletions(-)
>>>>
>>>> diff --git a/checkpolicy/module_compiler.c
>>>> b/checkpolicy/module_compiler.c
>>>> index 6e5483c..bb60f34 100644
>>>> --- a/checkpolicy/module_compiler.c
>>>> +++ b/checkpolicy/module_compiler.c
>>>> @@ -647,7 +647,6 @@ int require_symbol(uint32_t symbol_type,
>>>>      } else if (retval == -2) {
>>>>          /* ignore require statements if that symbol was
>>>>           * previously declared and is in current scope */
>>>> -        int prev_declaration_ok = 0;
>>>>          if (is_id_in_scope(symbol_type, key)) {
>>>>              if (symbol_type == SYM_TYPES) {
>>>>                  /* check that previous symbol has same
>>>> @@ -661,23 +660,14 @@ int require_symbol(uint32_t symbol_type,
>>>>                                      table, key);
>>>>                  assert(old_datum != NULL);
>>>>                  unsigned char old_isattr = old_datum->flavor;
>>>> -                prev_declaration_ok =
>>>> -                    (old_isattr == new_isattr ? 1 : 0);
>>>> -            } else {
>>>> -                prev_declaration_ok = 1;
>>>> +                if (old_isattr != new_isattr)
>>>> +                    return -2;
>>>>              }
>>>> -        }
>>>> -        if (prev_declaration_ok) {
>>>>              /* ignore this require statement because it
>>>>               * was already declared within my scope */
>>>>              stack_top->require_given = 1;
>>>> -            return 1;
>>>> -        } else {
>>>> -            /* previous declaration was not in scope or
>>>> -             * had a mismatched type/attribute, so
>>>> -             * generate an error */
>>>> -            return -2;
>>>>          }
>>>> +        return 1;
>>>>      } else if (retval < 0) {
>>>>          return -3;
>>>>      } else {        /* fall through possible if retval is 0 or 1 */
>>>>
>>>
>>>
>>>
>>>
>>> _______________________________________________
>>> Selinux mailing list
>>> Selinux@xxxxxxxxxxxxx
>>> To unsubscribe, send email to Selinux-leave@xxxxxxxxxxxxx.
>>> To get help, send an email containing "help" to
>>> Selinux-request@xxxxxxxxxxxxx.
>>>
>>
>>
> 
> 


-- 
Key fingerprint = 5F4D 3CDB D3F8 3652 FBD8  02D5 3B6C 5F1D 2C7B 6B02
https://sks-keyservers.net/pks/lookup?op=get&search=0x3B6C5F1D2C7B6B02
Dominick Grift

Attachment: signature.asc
Description: OpenPGP digital signature

_______________________________________________
Selinux mailing list
Selinux@xxxxxxxxxxxxx
To unsubscribe, send email to Selinux-leave@xxxxxxxxxxxxx.
To get help, send an email containing "help" to Selinux-request@xxxxxxxxxxxxx.

[Index of Archives]     [Selinux Refpolicy]     [Linux SGX]     [Fedora Users]     [Fedora Desktop]     [Yosemite Photos]     [Yosemite Camping]     [Yosemite Campsites]     [KDE Users]     [Gnome Users]

  Powered by Linux