[apparmor] [patch] Fix dfa minimization

John Johansen john.johansen at canonical.com
Thu Jan 9 05:06:08 UTC 2014


On 01/08/2014 02:50 PM, Steve Beattie wrote:
> On Sat, Jan 04, 2014 at 03:09:43AM -0800, John Johansen wrote:
>> And the patch (yes I died a little with this one, guess what code is now on
>> my hit list). And yes it is just removing casting from the one line and
>> changing it in the macro
>>
>> Signed-off-by: John Johansen <john.johansen at canonical.com>
>>
>> oh and nominated for the 2.8 series as well
> 
> Acked-by: Steve Beattie <steve at nxnw.org> (trunk and 2.8)
> 
> While this patch is fine, I think, applying it + the DFA minimization
> patch, while generating a subtly different binary blob, still results in
> a policy that the kernel rejects. So this might be part of the issue,
> but it's not the whole issue.
> 
> 
> 
so I must have dropped the deny or something in my testing so a missed
a couple of more bugs around this found. The issue is that
  audit deny dbus,

is being reduced to a dfa with a single none accepting state, and the
table generation doesn't handle this correctly, there are 2 issues
- the accept and accept2 tables are resized to 1 but the cfha format
  requires at least 2. 1 for the none accepting state and 1 for the
  start state.
  the kernel check that the accept tables == other state table sizes
  caught this and rejected it.
- the next/check table needs to be padded to the smallest base position
  used + 256 so no input can ever overflow the next/check table
  (next/check[base+c]).

  This is normally handled by inserting a transition which resizes
  the table. However in this case there where no transitions being
  inserted into the dfa. Resulting in a next/check table size of
  2, with a base pos of 0. Meaning the table needed to be padded
  to 256.

I have also include the original alignment casting patch

Signed-off-by: John Johansen <john.johansen at canonical.com>

=== modified file 'parser/libapparmor_re/chfa.cc'
--- parser/libapparmor_re/chfa.cc	2013-12-28 11:37:22 +0000
+++ parser/libapparmor_re/chfa.cc	2014-01-09 04:43:58 +0000
@@ -98,9 +98,9 @@
 	default_base.push_back(make_pair(dfa.nonmatching, 0));
 	num.insert(make_pair(dfa.nonmatching, num.size()));
 
-	accept.resize(dfa.states.size());
-	accept2.resize(dfa.states.size());
-	next_check.resize(optimal);
+	accept.resize(max(dfa.states.size(),2ul));
+	accept2.resize(max(dfa.states.size(),2ul));
+	next_check.resize(max(optimal,256ul));
 	free_list.resize(optimal);
 
 	accept[0] = 0;

=== modified file 'parser/parser_interface.c'
--- parser/parser_interface.c	2013-10-14 21:37:48 +0000
+++ parser/parser_interface.c	2014-01-09 01:28:22 +0000
@@ -363,7 +363,7 @@
 	return 1;
 }
 
-#define align64(X) (((size_t) (X) + (size_t) 7) & ~((size_t) 7))
+#define align64(X) (((X) + (typeof(X)) 7) & ~((typeof(X)) 7))
 inline int sd_write_aligned_blob(sd_serialize *p, void *b, int buf_size,
 				 const char *name)
 {
@@ -371,7 +371,7 @@
 	u32 tmp;
 	if (!sd_write_name(p, name))
 		return 0;
-	pad = align64(((long)(p->pos + 5) - (long)(p->buffer)) - ((long)(p->pos + 5) - (long)(p->buffer)));
+	pad = align64(p->pos + 5 - p->buffer) - (p->pos + 5 - p->buffer);
 	if (!sd_prepare_write(p, SD_BLOB, 4 + buf_size + pad))
 		return 0;
 	tmp = cpu_to_le32(buf_size + pad);





More information about the AppArmor mailing list