[apparmor] [PATCH 06/11] Ensure that all cases that resolve to nonmatching state are hashed the same

John Johansen john.johansen at canonical.com
Tue Nov 9 00:20:55 GMT 2010


On 11/08/2010 01:56 PM, Steve Beattie wrote:
> On Mon, Oct 18, 2010 at 05:20:38PM -0700, John Johansen wrote:
>> When hashing Nodes ensure that cases.otherwise == NULL is treated the same
>> as pointing to the nonmatching state.  Having this mix shouldn't currently
>> exist but adding the extra check makes the code more robust.
>> ---
>>  parser/libapparmor_re/regexp.y |    6 +++---
>>  1 files changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/parser/libapparmor_re/regexp.y b/parser/libapparmor_re/regexp.y
>> index 6de7ae6..4c5d801 100644
>> --- a/parser/libapparmor_re/regexp.y
>> +++ b/parser/libapparmor_re/regexp.y
>> @@ -1640,14 +1640,14 @@ void DFA::remove_unreachable(dfaflags_t flags)
>>  bool DFA::same_mappings(map <State *, Partition *> &partition_map, State *s1,
>>  			State *s2)
>>  {
>> -	if (s1->cases.otherwise) {
>> -		if (!s2->cases.otherwise)
>> +	if (s1->cases.otherwise && s1->cases.otherwise != nonmatching) {
>> +		if (!s2->cases.otherwise && s2->cases.otherwise != nonmatching)
> 
> Is the logic here in the second if-statement correct? Shouldn't it be
> 
>     if (!s2->cases.otherwise || s2->cases.otherwise == nonmatching)
> 
> as you've already verified that s1->cases.otherwise is not NULL and not
> a nonmatching state? Or am I missing something?
> 
nope, you are right I messed that test up
>>  			return false;
>>  		Partition *p1 = partition_map.find(s1->cases.otherwise)->second;
>>  		Partition *p2 = partition_map.find(s2->cases.otherwise)->second;
>>  		if (p1 != p2)
>>  			return false;
>> -	} else if (s2->cases.otherwise) {
>> +	} else if (s2->cases.otherwise && s1->cases.otherwise != nonmatching) {
I also messed up this one

>>  		return false;
>>  	}
>>  
> 
the new diff

--- a/parser/libapparmor_re/regexp.y
+++ b/parser/libapparmor_re/regexp.y
@@ -1639,14 +1639,14 @@
 bool DFA::same_mappings(map <State *, Partition *> &partition_map, State *s1,
 			State *s2)
 {
-	if (s1->cases.otherwise) {
-		if (!s2->cases.otherwise)
+	if (s1->cases.otherwise && s1->cases.otherwise != nonmatching) {
+		if (!s2->cases.otherwise || s2->cases.otherwise == nonmatching)
 			return false;
 		Partition *p1 = partition_map.find(s1->cases.otherwise)->second;
 		Partition *p2 = partition_map.find(s2->cases.otherwise)->second;
 		if (p1 != p2)
 			return false;
-	} else if (s2->cases.otherwise) {
+	} else if (s2->cases.otherwise && s2->cases.otherwise != nonmatching) {
 		return false;
 	}
 



More information about the AppArmor mailing list