Expression simplification can get into an infinite loop due to eps
pairs hiding behind and alternation that can't be caught by
normalize_eps() (which exists in the first place to stop a similar
loop).
The loop in question happens in AltNode::normalize when a subtree has
the following structure.
1. elseif (child[dir]->is_type(ALT_NODE)) rotate_node too
alt
/\
/ \
/ \
eps alt
/\
/ \
/ \
alt eps
/\
/ \
/ \
eps eps
2. if (normalize_eps(dir)) results in
alt
/\
/ \
/ \
alt eps
/\
/ \
/ \
alt eps
/\
/ \
/ \
eps eps
3. elseif (child[dir]->is_type(ALT_NODE)) rotate_node too
alt
/\
/ \
/ \
alt alt
/\ /\
/ \ / \
/ \ / \
eps eps eps eps
4. elseif (child[dir]->is_type(ALT_NODE)) rotate_node too
alt
/\
/ \
/ \
eps alt
/\
/ \
/ \
eps alt
/\
/ \
/ \
eps eps
5. if (normalize_eps(dir)) results in
alt
/\
/ \
/ \
alt eps
/\
/ \
/ \
eps alt
/\
/ \
/ \
eps eps
6. elseif (child[dir]->is_type(ALT_NODE)) rotate_node too
alt
/\
/ \
/ \
eps alt
/\
/ \
/ \
alt eps
/\
/ \
/ \
eps eps
back to beginning of cycle
Fix this by detecting the creation of an eps_pair in rotate_node(),
that pair can be immediately eliminated by simplifying the tree in that
step.
In the above cycle the pair creation is caught at step 3 resulting
in
3. elseif (child[dir]->is_type(ALT_NODE)) rotate_node too
alt
/\
/ \
/ \
alt eps
/\
/ \
/ \
eps eps
4. elseif (child[dir]->is_type(ALT_NODE)) rotate_node too
alt
/\
/ \
/ \
eps alt
/\
/ \
/ \
eps eps
which gets reduced to
alt
/\
/ \
/ \
eps eps
breaking the normalization loop. The degenerate alt node will be caught
in turn when its parent is dealt with.
This needs to be backported to all releases
Closes: https://gitlab.com/apparmor/apparmor/-/issues/398
Fixes: 846cee506 ("Split out parsing and expression trees from regexp.y")
Reported-by: Christian Boltz <apparmor@cboltz.de>
Signed-off-by: John Johansen <john.johansen@canonical.com>
Closes#398
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1252
Approved-by: Georgia Garcia <georgia.garcia@canonical.com>
Merged-by: John Johansen <john@jjmx.net>
(cherry picked from commit a6691ca53e)
Signed-off-by: John Johansen <john.johansen@canonical.com>
This is patch tries to reduce the number of dynamic_cast<>s needed
during normalization by pushing the operations of normalize_tree()
into the expr-tree classes themselves rather than perform it as
an external function. This eliminates the need for dynamic_cast<>
checks on the current object under inspection and reduces the number
of checks needing to be performed on child Nodes as well.
In non-strict benchmarking, doing the dynamic_cast<> reduction
for just the tree normalization operation resulted in a ~10-15%
improvement in overall time on a couple of different hosts (amd64,
armel), as measured against apparmor_parser -Q. Valgrind's callgrind
tool indicated a reduction in the number of calls to dynamic_cast<>
on the tst/simple_tests/vars/dbus_vars_9.sd test profile from ~19
million calls to ~12 million.
In comparisons with dumped expr trees over both the entire
tst/simple_tests/ tree and from 1000 randomly generated profiles via
stress.rb, the generated trees were identical.
Patch history:
v1: initial version of patch
v2: update patch to take into account the infinite loop fix in
trunk rev 1975 and refresh against current code.
v3: no change
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
There are some rare occassions, when lots of alternations are used that
tree simplification can result in an expression of
(E | (E | E)) or (E . (E . E)) where E is the epsnode
both of these expressions will lead to an inifinite loop in normalize_tree
as the epsnode test
if ((&epsnode == t->child[dir]) &&
(&epsnode != t->child[!dir]) &&
dynamic_cast<TwoChildNode *>(t)) {
and the tree node rotation test
} else if ((dynamic_cast<AltNode *>(t) &&
dynamic_cast<AltNode *>(t->child[dir])) ||
(dynamic_cast<CatNode *>(t) &&
dynamic_cast<CatNode *>(t->child[dir]))) {
end up undoing each others work, ie.
eps flip rotate
(E | (E | E)) --------> ((E | E) | E) -------> (E | (E | E))
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-By: Steve Beattie <sbeattie@ubuntu.com>
Split hfa into hfa and compressed_hfa files. The hfa portion focuses on
creating an manipulating hfas, while compressed_hfa is used for creating
compressed hfas that can be used/reused at run time with much less memory
usage than the full blown hfa.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-By: Steve Beattie <sbeattie@ubuntu.com>
Start of splitting regexp.y into logical components instead of the mess
it is today. Split out the expr-tree and parsing components from regexp.y
int expr-tree.x and parse.y and since regexp.y no longer does parsing
rename it to hfa.cc
Some code cleanups snuck their way into this patch and since I am to
lazy to redo it, I have left them in.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-By: Steve Beattie <sbeattie@ubuntu.com>
