apparmor_parser -O no-expr-simplify problematic

[Michael Vogt]

There was a recent issue with a core refresh that caused breakage. Upon further investigation it turns out that the apparmor_parser uses an substantial of memory.

Upon some more investigation it turns out that that -O no-expr-simplify makes both time to compile and memory usage increase 10x. Tested with 22.04 but I see the same ballpark results with 16.04:

$ /usr/bin/time --verbose apparmor_parser -S 2.59/profiles/snap.screenly-client.command-executor > /dev/null
    Command being timed: "apparmor_parser -S 2.59/profiles/snap.screenly-client.command-executor"
    User time (seconds): 4.32
    Maximum resident set size (kbytes): 117392

$ /usr/bin/time --verbose apparmor_parser -O no-expr-simplify -S 2.59/profiles/snap.screenly-client.command-executor > /dev/null
    Command being timed: "apparmor_parser -O no-expr-simplify -S 2.59/profiles/snap.screenly-client.command-executor"
    User time (seconds): 40.64
    Maximum resident set size (kbytes): 1015816

Profile is attached.

It seems like we seriously need to consider dropping "-O no-expr-simplify".

For context:
https://bugs.launchpad.net/ubuntu-rtm/+source/apparmor/+bug/1383858
is why it was added in the first place

And some recent work to make things faster:
https://gitlab.com/apparmor/apparmor/-/merge_requests/711

[Michael Vogt (mvo)]

Just another data point, it seems some expressions are quite pathological, e.g. removing

/sys/devices/{,*pcie-controller/,platform/{soc,scb}/*.pcie/}pci[0-9a-f]*/**/config r,
/sys/devices/{,*pcie-controller/,platform/{soc,scb}/*.pcie/}pci[0-9a-f]*/**/revision r,
/sys/devices/{,*pcie-controller/,platform/{soc,scb}/*.pcie/}pci[0-9a-f]*/**/resource r,
/sys/devices/{,*pcie-controller/,platform/{soc,scb}/*.pcie/}pci[0-9a-f]*/**/irq r,
/sys/devices/{,*pcie-controller/,platform/{soc,scb}/*.pcie/}pci[0-9a-f]*/**/boot_vga r,
/sys/devices/{,*pcie-controller/,platform/{soc,scb}/*.pcie/}pci[0-9a-f]*/**/{,subsystem_}class r,
/sys/devices/{,*pcie-controller/,platform/{soc,scb}/*.pcie/}pci[0-9a-f]*/**/{,subsystem_}device r,
/sys/devices/{,*pcie-controller/,platform/{soc,scb}/*.pcie/}pci[0-9a-f]*/**/{,subsystem_}vendor r,
/sys/devices/**/drm{,_dp_aux_dev}/** r,

makes the profile generation go down from 44s -> 10s so it seems some specific lines are most likly causing this issue.

[John Johansen (jjohansen)]

Yes, this is to be expected. The dfa build algorithm can have exponential state explosive. Expr simplification is a technique to help avoid/mitigate that from happening. There is no reason that expr simplification shouldn't be done.

In the past Jamie had disabled it for a couple of reasons.

1. for very simple profiles it isn't needed, and causes compilation to slow down a little, vs. not having it. (this was on click, with phones slow processor).

2. expr simplification could in its own rights in the past could be pathalogical as it makes multiple passes, working on simplifying expressions to deal with this explosion issue. In these cases, while it would reduce memory overhead of the compile it would slow down the compile significantly.

Case 2 was taken care of but putting a hard cap on the number of passes simplification will do, in upstream commit

2809060be parser: limit the number of passes expr tree simplification does (MR: https://gitlab.com/apparmor/apparmor/merge_requests/246)

This was found to achieve the majority of simplification gains without multiple passes where as few as a single change was made. And there is of course MR 711 that mvo has already brought up. There is some other work that will further improve expr simplification when it lands, so we should see further performance improvements in the future.

[Michael Vogt (mvo)]

I did a quick experiment in our "interfaces-many-snap-provided" test to see what impact the options have. I ran spread with:

$ spread -shell-after -v qemu:ubuntu-22.04-64:tests/main/interfaces-many-snap-provided

And inside spread (on 22.04) compiled the profiles with/without no-expr-simplify:

With "no-expr-simplify"
# /usr/bin/time -f '%E real, %M max-mem' apparmor_parser -O no-expr-simplify -S /var/lib/snapd/apparmor/profiles/snap.{test-snapd-policy-app-consumer,test-snapd-policy-app-provider-classic}.* >/dev/null
0:22.07 real, 22908 max-mem

Without "no-expr-simplify"
# /usr/bin/time -f '%E real, %M max-mem' apparmor_parser -S /var/lib/snapd/apparmor/profiles/snap.{test-snapd-policy-app-consumer,test-snapd-policy-app-provider-classic}.* >/dev/null
0:49.45 real, 13720 max-mem

Which seems to indicate that there is a trade-off here and moving away from "no-expr-simplify" will make the installs/refreshes slower but also reduces memory usage. It also seems like our normal tests do not catch the memory explosion that we saw here.

[Michael Vogt (mvo)]

I created https://github.com/snapcore/snapd/pull/12919 to demonstrate that a combination of certain profiles lead to very large memory usage (1.6G in my test on 22.04) and that omitting it results in more reasonable memory usage (380Mb).

And I created https://github.com/snapcore/snapd/pull/12919 that changes the default to stop using "-O no-expr-simplify".

[John Johansen (jjohansen)]

sigh, that performance drop is more than I was expecting. I do have a set of click/snap profiles I use for testing. Can you send me your set, as it obviously is better for this

I will take a peek at the expr simplification series. Its not done, but it has several patch in it that are, and we might be able to apply those and get a bit of a boost without waiting on the completed set.

[Philip Meulengracht (the-meulengracht)]

On the encouragement from mvo, I made a small tool that can optimize a generated snapd apparmor profile. By using the profile from this bug, I can see almost 50% improvement in cpu time and memory time. It was just a small side-project while I was working.

https://github.com/Meulengracht/aa-preprocess

Profile used (https://launchpadlibrarian.net/674087996/snap.screenly-client.command-executor)

Before running the tool

User time (seconds): 6.73
Maximum resident set size (kbytes): 294408

After running the tool
Optimized profile here (https://paste.ubuntu.com/p/GCt6j4zrzW/)

User time (seconds): 3.56
Maximum resident set size (kbytes): 167712

Both times are run with "apparmor_parser -O no-expr-simplify". The tool is not that sophisticated and simply consolidates lines that match each other in permissions and wildcards to reduce the number of lines in the apparmor profile. It also only runs on lines starting with /sys/devices currently, but could be extended to others. The resulting profile is not extensively tested either, so take this with a grain of salt as well.

Maybe it's something that can be considered somewhere to increase performance?

[Alex Murray (alexmurray)]

FWIW this aa-preprocess tool sounds like it would be good to run via a github action so that it can suggest changes to the existing profiles which would then be made by hand.

[John Johansen (jjohansen)]

so I think this is largely because the apparmor version snap is using is not running rule deduplication on mount rules.

[Ondrej Kubik (ondrak)]

@Alex Murray <email address hidden> github action is good idea to
optimise on the interface level and PR I open is trying to do that (though
I have done it manually)
But this has limitation as this optimisation can be done only per interface.
Preprocessing the full profile has the potential to optimise
cross-interfaces when multiple interfaces could define the same expression.
But one can argue that apparmor_parser should have this as the first step
before even parsing the profile, dummy dedupe and simplification of the
profile before building the tree.
it seems a lot cheaper as pre-processing step

On Tue, 4 Jul 2023 at 04:25, John Johansen <email address hidden>
wrote:

> so I think this is largely because the apparmor version snap is using is
> not running rule deduplication on mount rules.
>
> --
> You received this bug notification because you are subscribed to the bug
> report.
> https://bugs.launchpad.net/bugs/2025030
>
> Title:
> apparmor_parser -O no-expr-simplify problematic
>
> Status in snapd:
> In Progress
>
> Bug description:
> There was a recent issue with a core refresh that caused breakage.
> Upon further investigation it turns out that the apparmor_parser uses
> an substantial of memory.
>
> Upon some more investigation it turns out that that -O no-expr-
> simplify makes both time to compile and memory usage increase 10x.
> Tested with 22.04 but I see the same ballpark results with 16.04:
>
> $ /usr/bin/time --verbose apparmor_parser -S
> 2.59/profiles/snap.screenly-client.command-executor > /dev/null
> Command being timed: "apparmor_parser -S
> 2.59/profiles/snap.screenly-client.command-executor"
> User time (seconds): 4.32
> Maximum resident set size (kbytes): 117392
>
> $ /usr/bin/time --verbose apparmor_parser -O no-expr-simplify -S
> 2.59/profiles/snap.screenly-client.command-executor > /dev/null
> Command being timed: "apparmor_parser -O no-expr-simplify -S
> 2.59/profiles/snap.screenly-client.command-executor"
> User time (seconds): 40.64
> Maximum resident set size (kbytes): 1015816
>
> Profile is attached.
>
>
> It seems like we seriously need to consider dropping "-O
> no-expr-simplify".
>
> For context:
> https://bugs.launchpad.net/ubuntu-rtm/+source/apparmor/+bug/1383858
> is why it was added in the first place
>
> And some recent work to make things faster:
> https://gitlab.com/apparmor/apparmor/-/merge_requests/711
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/snapd/+bug/2025030/+subscriptions
>
>