Re: [PATCH v2] libsepol: fix checkpolicy dontaudit compiler bug

[Stephen Smalley]

On 11/15/2016 12:30 PM, Nick Kralevich wrote:
> Perhaps merge Stephen's change now, and address any changes in a
> followup? I'd like to get it merged into the selinux tree so I can
> merge the changes into the Android tree.

Yes, it is already merged.

> 
> -- Nick
> 
> On Tue, Nov 15, 2016 at 9:10 AM, William Roberts
> <bill.c.robe...@gmail.com> wrote:
>> For bit setting in constant time, one could always clear the bit(s) and or
>> in what you want. I think that logic might be applicable here. I could take
>> a stab at looking at it today, if no one has anything better by tomorrow
>> well just merge yours as is. Does that sound reasonable?
>>
>>
>> On Nov 15, 2016 06:18, "Stephen Smalley" <s...@tycho.nsa.gov> wrote:
>>>
>>> On 11/14/2016 02:41 PM, Roberts, William C wrote:
>>>>
>>>>
>>>>> -Original Message-
>>>>> From: Selinux [mailto:selinux-boun...@tycho.nsa.gov] On Behalf Of
>>>>> Roberts,
>>>>> William C
>>>>> Sent: Monday, November 14, 2016 10:44 AM
>>>>> To: Stephen Smalley <s...@tycho.nsa.gov>; selinux@tycho.nsa.gov
>>>>> Subject: RE: [PATCH v2] libsepol: fix checkpolicy dontaudit compiler
>>>>> bug
>>>>>
>>>>>
>>>>>
>>>>>> -Original Message-
>>>>>> From: Selinux [mailto:selinux-boun...@tycho.nsa.gov] On Behalf Of
>>>>>> Stephen Smalley
>>>>>> Sent: Monday, November 14, 2016 9:48 AM
>>>>>> To: selinux@tycho.nsa.gov
>>>>>> Cc: Stephen Smalley <s...@tycho.nsa.gov>
>>>>>> Subject: [PATCH v2] libsepol: fix checkpolicy dontaudit compiler bug
>>>>>>
>>>>>> The combining logic for dontaudit rules was wrong, causing a dontaudit
>>>>>> A B:C *; rule to be clobbered by a dontaudit A B:C p; rule.
>>>>>>
>>>>>> Reported-by: Nick Kralevich <n...@google.com>
>>>>>> Signed-off-by: Stephen Smalley <s...@tycho.nsa.gov>
>>>>>> ---
>>>>>>  libsepol/src/expand.c | 16 
>>>>>>  1 file changed, 12 insertions(+), 4 deletions(-)
>>>>>>
>>>>>> diff --git a/libsepol/src/expand.c b/libsepol/src/expand.c index
>>>>>> 004a029..d7adbf8
>>>>>> 100644
>>>>>> --- a/libsepol/src/expand.c
>>>>>> +++ b/libsepol/src/expand.c
>>>>>> @@ -1604,7 +1604,8 @@ static int expand_range_trans(expand_state_t *
>>>>>> state, static avtab_ptr_t find_avtab_node(sepol_handle_t * handle,
>>>>>>avtab_t * avtab, avtab_key_t * key,
>>>>>>cond_av_list_t ** cond,
>>>>>> -  av_extended_perms_t *xperms)
>>>>>> +  av_extended_perms_t *xperms,
>>>>>> +  char *alloced)
>>>>>>  {
>>>>>> avtab_ptr_t node;
>>>>>> avtab_datum_t avdatum;
>>>>>> @@ -1658,6 +1659,11 @@ static avtab_ptr_t
>>>>>> find_avtab_node(sepol_handle_t * handle,
>>>>>> nl->next = *cond;
>>>>>> *cond = nl;
>>>>>> }
>>>>>> +   if (alloced)
>>>>>> +   *alloced = 1;
>>>>>> +   } else {
>>>>>> +   if (alloced)
>>>>>> +   *alloced = 0;
>>>>>> }
>>>>>>
>>>>>> return node;
>>>>>> @@ -1750,7 +1756,7 @@ static int expand_terule_helper(sepol_handle_t *
>>>>>> handle,
>>>>>> return EXPAND_RULE_CONFLICT;
>>>>>> }
>>>>>>
>>>>>> -   node = find_avtab_node(handle, avtab, , cond, NULL);
>>>>>> +   node = find_avtab_node(handle, avtab, , cond, NULL,
>>>>>> NULL);
>>>>>> if (!node)
>>>>>> return -1;
>>>>>> if (enabled) {
>>>>>> @@ -1790,6 +1796,7 @@ static int expand_avrule_helper(sepol_handle_t *
>>>>>> handle,
>>>>>> class_perm_node_t *cur;
>>>>>>   

Re: [PATCH v2] libsepol: fix checkpolicy dontaudit compiler bug

[William Roberts]

On Nov 15, 2016 09:30, "Nick Kralevich" <n...@google.com> wrote:
>
> Perhaps merge Stephen's change now, and address any changes in a
> followup? I'd like to get it merged into the selinux tree so I can
> merge the changes into the Android tree.

That's fine with me too.

>
> -- Nick
>
> On Tue, Nov 15, 2016 at 9:10 AM, William Roberts
> <bill.c.robe...@gmail.com> wrote:
> > For bit setting in constant time, one could always clear the bit(s) and
or
> > in what you want. I think that logic might be applicable here. I could
take
> > a stab at looking at it today, if no one has anything better by tomorrow
> > well just merge yours as is. Does that sound reasonable?
> >
> >
> > On Nov 15, 2016 06:18, "Stephen Smalley" <s...@tycho.nsa.gov> wrote:
> >>
> >> On 11/14/2016 02:41 PM, Roberts, William C wrote:
> >> >
> >> >
> >> >> -Original Message-
> >> >> From: Selinux [mailto:selinux-boun...@tycho.nsa.gov] On Behalf Of
> >> >> Roberts,
> >> >> William C
> >> >> Sent: Monday, November 14, 2016 10:44 AM
> >> >> To: Stephen Smalley <s...@tycho.nsa.gov>; selinux@tycho.nsa.gov
> >> >> Subject: RE: [PATCH v2] libsepol: fix checkpolicy dontaudit compiler
> >> >> bug
> >> >>
> >> >>
> >> >>
> >> >>> -Original Message-
> >> >>> From: Selinux [mailto:selinux-boun...@tycho.nsa.gov] On Behalf Of
> >> >>> Stephen Smalley
> >> >>> Sent: Monday, November 14, 2016 9:48 AM
> >> >>> To: selinux@tycho.nsa.gov
> >> >>> Cc: Stephen Smalley <s...@tycho.nsa.gov>
> >> >>> Subject: [PATCH v2] libsepol: fix checkpolicy dontaudit compiler
bug
> >> >>>
> >> >>> The combining logic for dontaudit rules was wrong, causing a
dontaudit
> >> >>> A B:C *; rule to be clobbered by a dontaudit A B:C p; rule.
> >> >>>
> >> >>> Reported-by: Nick Kralevich <n...@google.com>
> >> >>> Signed-off-by: Stephen Smalley <s...@tycho.nsa.gov>
> >> >>> ---
> >> >>>  libsepol/src/expand.c | 16 
> >> >>>  1 file changed, 12 insertions(+), 4 deletions(-)
> >> >>>
> >> >>> diff --git a/libsepol/src/expand.c b/libsepol/src/expand.c index
> >> >>> 004a029..d7adbf8
> >> >>> 100644
> >> >>> --- a/libsepol/src/expand.c
> >> >>> +++ b/libsepol/src/expand.c
> >> >>> @@ -1604,7 +1604,8 @@ static int expand_range_trans(expand_state_t
*
> >> >>> state, static avtab_ptr_t find_avtab_node(sepol_handle_t * handle,
> >> >>>avtab_t * avtab, avtab_key_t * key,
> >> >>>cond_av_list_t ** cond,
> >> >>> -  av_extended_perms_t *xperms)
> >> >>> +  av_extended_perms_t *xperms,
> >> >>> +  char *alloced)
> >> >>>  {
> >> >>> avtab_ptr_t node;
> >> >>> avtab_datum_t avdatum;
> >> >>> @@ -1658,6 +1659,11 @@ static avtab_ptr_t
> >> >>> find_avtab_node(sepol_handle_t * handle,
> >> >>> nl->next = *cond;
> >> >>> *cond = nl;
> >> >>> }
> >> >>> +   if (alloced)
> >> >>> +   *alloced = 1;
> >> >>> +   } else {
> >> >>> +   if (alloced)
> >> >>> +   *alloced = 0;
> >> >>> }
> >> >>>
> >> >>> return node;
> >> >>> @@ -1750,7 +1756,7 @@ static int
expand_terule_helper(sepol_handle_t *
> >> >>> handle,
> >> >>> return EXPAND_RULE_CONFLICT;
> >> >>> }
> >> >>>
> >> >>> -   node = find_avtab_node(handle, avtab, , cond,
NULL);
> >> >>> +   node = find_avtab_node(handle, avtab, , cond,
NULL,
> >> >>> NULL);
> >> >>> if (!node)
> >> >>> return -1;
> >> >>> if (enabled) {
> >> >>> 

Re: [PATCH v2] libsepol: fix checkpolicy dontaudit compiler bug

[William Roberts]

For bit setting in constant time, one could always clear the bit(s) and or
in what you want. I think that logic might be applicable here. I could take
a stab at looking at it today, if no one has anything better by tomorrow
well just merge yours as is. Does that sound reasonable?

On Nov 15, 2016 06:18, "Stephen Smalley" <s...@tycho.nsa.gov> wrote:

> On 11/14/2016 02:41 PM, Roberts, William C wrote:
> >
> >
> >> -Original Message-
> >> From: Selinux [mailto:selinux-boun...@tycho.nsa.gov] On Behalf Of
> Roberts,
> >> William C
> >> Sent: Monday, November 14, 2016 10:44 AM
> >> To: Stephen Smalley <s...@tycho.nsa.gov>; selinux@tycho.nsa.gov
> >> Subject: RE: [PATCH v2] libsepol: fix checkpolicy dontaudit compiler bug
> >>
> >>
> >>
> >>> -Original Message-
> >>> From: Selinux [mailto:selinux-boun...@tycho.nsa.gov] On Behalf Of
> >>> Stephen Smalley
> >>> Sent: Monday, November 14, 2016 9:48 AM
> >>> To: selinux@tycho.nsa.gov
> >>> Cc: Stephen Smalley <s...@tycho.nsa.gov>
> >>> Subject: [PATCH v2] libsepol: fix checkpolicy dontaudit compiler bug
> >>>
> >>> The combining logic for dontaudit rules was wrong, causing a dontaudit
> >>> A B:C *; rule to be clobbered by a dontaudit A B:C p; rule.
> >>>
> >>> Reported-by: Nick Kralevich <n...@google.com>
> >>> Signed-off-by: Stephen Smalley <s...@tycho.nsa.gov>
> >>> ---
> >>>  libsepol/src/expand.c | 16 
> >>>  1 file changed, 12 insertions(+), 4 deletions(-)
> >>>
> >>> diff --git a/libsepol/src/expand.c b/libsepol/src/expand.c index
> >>> 004a029..d7adbf8
> >>> 100644
> >>> --- a/libsepol/src/expand.c
> >>> +++ b/libsepol/src/expand.c
> >>> @@ -1604,7 +1604,8 @@ static int expand_range_trans(expand_state_t *
> >>> state, static avtab_ptr_t find_avtab_node(sepol_handle_t * handle,
> >>>avtab_t * avtab, avtab_key_t * key,
> >>>cond_av_list_t ** cond,
> >>> -  av_extended_perms_t *xperms)
> >>> +  av_extended_perms_t *xperms,
> >>> +  char *alloced)
> >>>  {
> >>> avtab_ptr_t node;
> >>> avtab_datum_t avdatum;
> >>> @@ -1658,6 +1659,11 @@ static avtab_ptr_t
> >>> find_avtab_node(sepol_handle_t * handle,
> >>> nl->next = *cond;
> >>> *cond = nl;
> >>> }
> >>> +   if (alloced)
> >>> +   *alloced = 1;
> >>> +   } else {
> >>> +   if (alloced)
> >>> +   *alloced = 0;
> >>> }
> >>>
> >>> return node;
> >>> @@ -1750,7 +1756,7 @@ static int expand_terule_helper(sepol_handle_t *
> >>> handle,
> >>> return EXPAND_RULE_CONFLICT;
> >>> }
> >>>
> >>> -   node = find_avtab_node(handle, avtab, , cond, NULL);
> >>> +   node = find_avtab_node(handle, avtab, , cond, NULL,
> >>> NULL);
> >>> if (!node)
> >>> return -1;
> >>> if (enabled) {
> >>> @@ -1790,6 +1796,7 @@ static int expand_avrule_helper(sepol_handle_t *
> >>> handle,
> >>> class_perm_node_t *cur;
> >>> uint32_t spec = 0;
> >>> unsigned int i;
> >>> +   char alloced;
> >>>
> >>> if (specified & AVRULE_ALLOWED) {
> >>> spec = AVTAB_ALLOWED;
> >>> @@ -1824,7 +1831,8 @@ static int expand_avrule_helper(sepol_handle_t *
> >>> handle,
> >>> avkey.target_class = cur->tclass;
> >>> avkey.specified = spec;
> >>>
> >>> -   node = find_avtab_node(handle, avtab, , cond,
> >>> extended_perms);
> >>> +   node = find_avtab_node(handle, avtab, , cond,
> >>> +  extended_perms, );
> >>> if (!node)
> >>> return EXPAND_RULE_ERROR;
> >>> if (enabled) {
> >>> @@ -1850,7 +1858,7 @@ static int expand_avrule_helper(sepol_handle_t *
> >>

Re: [PATCH v2] libsepol: fix checkpolicy dontaudit compiler bug

[Stephen Smalley]

On 11/14/2016 06:58 PM, Nick Kralevich wrote:
> On Mon, Nov 14, 2016 at 9:48 AM, Stephen Smalley  wrote:
>> The combining logic for dontaudit rules was wrong, causing
>> a dontaudit A B:C *; rule to be clobbered by a dontaudit A B:C p;
>> rule.
>>
>> Reported-by: Nick Kralevich 
>> Signed-off-by: Stephen Smalley 
>> ---
>>  libsepol/src/expand.c | 16 
>>  1 file changed, 12 insertions(+), 4 deletions(-)
>>
>> diff --git a/libsepol/src/expand.c b/libsepol/src/expand.c
>> index 004a029..d7adbf8 100644
>> --- a/libsepol/src/expand.c
>> +++ b/libsepol/src/expand.c
>> @@ -1604,7 +1604,8 @@ static int expand_range_trans(expand_state_t * state,
>>  static avtab_ptr_t find_avtab_node(sepol_handle_t * handle,
>>avtab_t * avtab, avtab_key_t * key,
>>cond_av_list_t ** cond,
>> -  av_extended_perms_t *xperms)
>> +  av_extended_perms_t *xperms,
>> +  char *alloced)
>>  {
>> avtab_ptr_t node;
> 
> For robustness, it would be safer to ensure that alloced was always
> assigned to. This variable may end up unassigned on certain error
> conditions. It's not a bug today, since the caller always performs a
> check on the return value prior to using this variable, but it could
> be a use of an unassigned variable in a future version of this code.
> 
> Also, "bool" would be a better type for alloced, rather than using a 
> "char"

Originally did that but it broke - requires a separate patch to rename
the field named "bool" in include/sepol/policydb/conditional.h and all
users.  There was no bool type in C when we first wrote the security
server code (for Flask).


___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.

Re: [PATCH v2] libsepol: fix checkpolicy dontaudit compiler bug

[Stephen Smalley]

On 11/14/2016 02:41 PM, Roberts, William C wrote:
> 
> 
>> -Original Message-
>> From: Selinux [mailto:selinux-boun...@tycho.nsa.gov] On Behalf Of Roberts,
>> William C
>> Sent: Monday, November 14, 2016 10:44 AM
>> To: Stephen Smalley <s...@tycho.nsa.gov>; selinux@tycho.nsa.gov
>> Subject: RE: [PATCH v2] libsepol: fix checkpolicy dontaudit compiler bug
>>
>>
>>
>>> -Original Message-
>>> From: Selinux [mailto:selinux-boun...@tycho.nsa.gov] On Behalf Of
>>> Stephen Smalley
>>> Sent: Monday, November 14, 2016 9:48 AM
>>> To: selinux@tycho.nsa.gov
>>> Cc: Stephen Smalley <s...@tycho.nsa.gov>
>>> Subject: [PATCH v2] libsepol: fix checkpolicy dontaudit compiler bug
>>>
>>> The combining logic for dontaudit rules was wrong, causing a dontaudit
>>> A B:C *; rule to be clobbered by a dontaudit A B:C p; rule.
>>>
>>> Reported-by: Nick Kralevich <n...@google.com>
>>> Signed-off-by: Stephen Smalley <s...@tycho.nsa.gov>
>>> ---
>>>  libsepol/src/expand.c | 16 
>>>  1 file changed, 12 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/libsepol/src/expand.c b/libsepol/src/expand.c index
>>> 004a029..d7adbf8
>>> 100644
>>> --- a/libsepol/src/expand.c
>>> +++ b/libsepol/src/expand.c
>>> @@ -1604,7 +1604,8 @@ static int expand_range_trans(expand_state_t *
>>> state, static avtab_ptr_t find_avtab_node(sepol_handle_t * handle,
>>>avtab_t * avtab, avtab_key_t * key,
>>>cond_av_list_t ** cond,
>>> -  av_extended_perms_t *xperms)
>>> +  av_extended_perms_t *xperms,
>>> +  char *alloced)
>>>  {
>>> avtab_ptr_t node;
>>> avtab_datum_t avdatum;
>>> @@ -1658,6 +1659,11 @@ static avtab_ptr_t
>>> find_avtab_node(sepol_handle_t * handle,
>>> nl->next = *cond;
>>> *cond = nl;
>>> }
>>> +   if (alloced)
>>> +   *alloced = 1;
>>> +   } else {
>>> +   if (alloced)
>>> +   *alloced = 0;
>>> }
>>>
>>> return node;
>>> @@ -1750,7 +1756,7 @@ static int expand_terule_helper(sepol_handle_t *
>>> handle,
>>> return EXPAND_RULE_CONFLICT;
>>> }
>>>
>>> -   node = find_avtab_node(handle, avtab, , cond, NULL);
>>> +   node = find_avtab_node(handle, avtab, , cond, NULL,
>>> NULL);
>>> if (!node)
>>> return -1;
>>> if (enabled) {
>>> @@ -1790,6 +1796,7 @@ static int expand_avrule_helper(sepol_handle_t *
>>> handle,
>>> class_perm_node_t *cur;
>>> uint32_t spec = 0;
>>> unsigned int i;
>>> +   char alloced;
>>>
>>> if (specified & AVRULE_ALLOWED) {
>>> spec = AVTAB_ALLOWED;
>>> @@ -1824,7 +1831,8 @@ static int expand_avrule_helper(sepol_handle_t *
>>> handle,
>>> avkey.target_class = cur->tclass;
>>> avkey.specified = spec;
>>>
>>> -   node = find_avtab_node(handle, avtab, , cond,
>>> extended_perms);
>>> +   node = find_avtab_node(handle, avtab, , cond,
>>> +  extended_perms, );
>>> if (!node)
>>> return EXPAND_RULE_ERROR;
>>> if (enabled) {
>>> @@ -1850,7 +1858,7 @@ static int expand_avrule_helper(sepol_handle_t *
>>> handle,
>>>  */
>>> avdatump->data &= cur->data;
>>> } else if (specified & AVRULE_DONTAUDIT) {
>>> -   if (avdatump->data)
>>> +   if (!alloced)
>>> avdatump->data &= ~cur->data;
>>> else
>>> avdatump->data = ~cur->data;
>>
>> This seems awkward to me. If the insertion created a new empty node why
>> wouldn't !avdump->data be true (note the addition of the not operator)?
> 
> I misstated that a bit, but the !avdump->data was the else case. I am really
> saying why didn't this work before? In my mind, we don't care if its 

RE: [PATCH v2] libsepol: fix checkpolicy dontaudit compiler bug

[Roberts, William C]

> -Original Message-
> From: Selinux [mailto:selinux-boun...@tycho.nsa.gov] On Behalf Of Roberts,
> William C
> Sent: Monday, November 14, 2016 10:44 AM
> To: Stephen Smalley <s...@tycho.nsa.gov>; selinux@tycho.nsa.gov
> Subject: RE: [PATCH v2] libsepol: fix checkpolicy dontaudit compiler bug
> 
> 
> 
> > -Original Message-
> > From: Selinux [mailto:selinux-boun...@tycho.nsa.gov] On Behalf Of
> > Stephen Smalley
> > Sent: Monday, November 14, 2016 9:48 AM
> > To: selinux@tycho.nsa.gov
> > Cc: Stephen Smalley <s...@tycho.nsa.gov>
> > Subject: [PATCH v2] libsepol: fix checkpolicy dontaudit compiler bug
> >
> > The combining logic for dontaudit rules was wrong, causing a dontaudit
> > A B:C *; rule to be clobbered by a dontaudit A B:C p; rule.
> >
> > Reported-by: Nick Kralevich <n...@google.com>
> > Signed-off-by: Stephen Smalley <s...@tycho.nsa.gov>
> > ---
> >  libsepol/src/expand.c | 16 
> >  1 file changed, 12 insertions(+), 4 deletions(-)
> >
> > diff --git a/libsepol/src/expand.c b/libsepol/src/expand.c index
> > 004a029..d7adbf8
> > 100644
> > --- a/libsepol/src/expand.c
> > +++ b/libsepol/src/expand.c
> > @@ -1604,7 +1604,8 @@ static int expand_range_trans(expand_state_t *
> > state, static avtab_ptr_t find_avtab_node(sepol_handle_t * handle,
> >avtab_t * avtab, avtab_key_t * key,
> >cond_av_list_t ** cond,
> > -  av_extended_perms_t *xperms)
> > +  av_extended_perms_t *xperms,
> > +  char *alloced)
> >  {
> > avtab_ptr_t node;
> > avtab_datum_t avdatum;
> > @@ -1658,6 +1659,11 @@ static avtab_ptr_t
> > find_avtab_node(sepol_handle_t * handle,
> > nl->next = *cond;
> > *cond = nl;
> > }
> > +   if (alloced)
> > +   *alloced = 1;
> > +   } else {
> > +   if (alloced)
> > +   *alloced = 0;
> > }
> >
> > return node;
> > @@ -1750,7 +1756,7 @@ static int expand_terule_helper(sepol_handle_t *
> > handle,
> > return EXPAND_RULE_CONFLICT;
> > }
> >
> > -   node = find_avtab_node(handle, avtab, , cond, NULL);
> > +   node = find_avtab_node(handle, avtab, , cond, NULL,
> > NULL);
> > if (!node)
> > return -1;
> > if (enabled) {
> > @@ -1790,6 +1796,7 @@ static int expand_avrule_helper(sepol_handle_t *
> > handle,
> > class_perm_node_t *cur;
> > uint32_t spec = 0;
> > unsigned int i;
> > +   char alloced;
> >
> > if (specified & AVRULE_ALLOWED) {
> > spec = AVTAB_ALLOWED;
> > @@ -1824,7 +1831,8 @@ static int expand_avrule_helper(sepol_handle_t *
> > handle,
> > avkey.target_class = cur->tclass;
> > avkey.specified = spec;
> >
> > -   node = find_avtab_node(handle, avtab, , cond,
> > extended_perms);
> > +   node = find_avtab_node(handle, avtab, , cond,
> > +  extended_perms, );
> > if (!node)
> > return EXPAND_RULE_ERROR;
> > if (enabled) {
> > @@ -1850,7 +1858,7 @@ static int expand_avrule_helper(sepol_handle_t *
> > handle,
> >  */
> > avdatump->data &= cur->data;
> > } else if (specified & AVRULE_DONTAUDIT) {
> > -   if (avdatump->data)
> > +   if (!alloced)
> > avdatump->data &= ~cur->data;
> > else
> > avdatump->data = ~cur->data;
> 
> This seems awkward to me. If the insertion created a new empty node why
> wouldn't !avdump->data be true (note the addition of the not operator)?

I misstated that a bit, but the !avdump->data was the else case. I am really
saying why didn't this work before? In my mind, we don't care if its allocated
we care if it's set or not.

> 
> Or perhaps a mechanism to actual set the data on allocation, this way the 
> logic is
> Just &=.
> 
> > --
> > 2.7.4
> >
> > ___
> > Selinux mailing list
> > Selinux@tycho.nsa.gov
> > To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
> > To get help, send an email containing "help" to 
> > selinux-requ...@tycho.nsa.gov.
> 
> ___
> Selinux mailing list
> Selinux@tycho.nsa.gov
> To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
> To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.

___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.

RE: [PATCH v2] libsepol: fix checkpolicy dontaudit compiler bug

[Roberts, William C]

> -Original Message-
> From: Selinux [mailto:selinux-boun...@tycho.nsa.gov] On Behalf Of Stephen
> Smalley
> Sent: Monday, November 14, 2016 9:48 AM
> To: selinux@tycho.nsa.gov
> Cc: Stephen Smalley 
> Subject: [PATCH v2] libsepol: fix checkpolicy dontaudit compiler bug
> 
> The combining logic for dontaudit rules was wrong, causing a dontaudit A B:C 
> *;
> rule to be clobbered by a dontaudit A B:C p; rule.
> 
> Reported-by: Nick Kralevich 
> Signed-off-by: Stephen Smalley 
> ---
>  libsepol/src/expand.c | 16 
>  1 file changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/libsepol/src/expand.c b/libsepol/src/expand.c index 
> 004a029..d7adbf8
> 100644
> --- a/libsepol/src/expand.c
> +++ b/libsepol/src/expand.c
> @@ -1604,7 +1604,8 @@ static int expand_range_trans(expand_state_t * state,
> static avtab_ptr_t find_avtab_node(sepol_handle_t * handle,
>  avtab_t * avtab, avtab_key_t * key,
>  cond_av_list_t ** cond,
> -av_extended_perms_t *xperms)
> +av_extended_perms_t *xperms,
> +char *alloced)
>  {
>   avtab_ptr_t node;
>   avtab_datum_t avdatum;
> @@ -1658,6 +1659,11 @@ static avtab_ptr_t find_avtab_node(sepol_handle_t *
> handle,
>   nl->next = *cond;
>   *cond = nl;
>   }
> + if (alloced)
> + *alloced = 1;
> + } else {
> + if (alloced)
> + *alloced = 0;
>   }
> 
>   return node;
> @@ -1750,7 +1756,7 @@ static int expand_terule_helper(sepol_handle_t *
> handle,
>   return EXPAND_RULE_CONFLICT;
>   }
> 
> - node = find_avtab_node(handle, avtab, , cond, NULL);
> + node = find_avtab_node(handle, avtab, , cond, NULL,
> NULL);
>   if (!node)
>   return -1;
>   if (enabled) {
> @@ -1790,6 +1796,7 @@ static int expand_avrule_helper(sepol_handle_t *
> handle,
>   class_perm_node_t *cur;
>   uint32_t spec = 0;
>   unsigned int i;
> + char alloced;
> 
>   if (specified & AVRULE_ALLOWED) {
>   spec = AVTAB_ALLOWED;
> @@ -1824,7 +1831,8 @@ static int expand_avrule_helper(sepol_handle_t *
> handle,
>   avkey.target_class = cur->tclass;
>   avkey.specified = spec;
> 
> - node = find_avtab_node(handle, avtab, , cond,
> extended_perms);
> + node = find_avtab_node(handle, avtab, , cond,
> +extended_perms, );
>   if (!node)
>   return EXPAND_RULE_ERROR;
>   if (enabled) {
> @@ -1850,7 +1858,7 @@ static int expand_avrule_helper(sepol_handle_t *
> handle,
>*/
>   avdatump->data &= cur->data;
>   } else if (specified & AVRULE_DONTAUDIT) {
> - if (avdatump->data)
> + if (!alloced)
>   avdatump->data &= ~cur->data;
>   else
>   avdatump->data = ~cur->data;

This seems awkward to me. If the insertion created a new empty node
why wouldn't !avdump->data be true (note the addition of the not operator)?

Or perhaps a mechanism to actual set the data on allocation, this way the logic 
is
Just &=.

> --
> 2.7.4
> 
> ___
> Selinux mailing list
> Selinux@tycho.nsa.gov
> To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
> To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.

___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.