specifying -O no-expr-simplify results in cache miss
Affects | Status | Importance | Assigned to | Milestone | |
---|---|---|---|---|---|
| AppArmor |
Undecided
|
Jaroslavas Karmazinas | ||
| apparmor (Ubuntu) |
Undecided
|
Jamie Strandboge | ||
| Disco |
Undecided
|
Jamie Strandboge | ||
| Eoan |
Undecided
|
Jamie Strandboge |
Bug Description
[Impact]
* AppArmor 2.13 unconditionally invalidates its cache when parser options are specified. To decrease compile times for ARM systems, -O no-expr-simplify has been used in Ubuntu for click and snap policy for many years, but was temporarily disabled during the disco development release with the 2.13 upload so caching properly worked everywhere. Now that a simple upstream workaround is available (and already in eoan), we'd like to apply the upstream patch and re-enable -O no-expr-simplify.
* A condition of the AppArmor 2.13 feature freeze exception was to fix this bug in SRU to re-enable -O no-expr-simplify. This will help compile times for all architectures with default and typical AppArmor policy (ie, a mixture of (distro) system and snap policy), but especially ARM systems with snaps where the improvement could be in terms of minutes saved.
* Specifically, the upstream patch workaround to no longer unconditionally skip reading the cache when parser options are specified. It also re-enables an existing quilt patch to update /etc/apparmor/
[Test Case]
# setup
$ mkdir -p /tmp/aa/cache /tmp/aa/profiles
$ cp /etc/apparmor.
# no options, no cache, expect a miss and to write
$ /sbin/apparmor_
Cache: added primary location '/tmp/aa/cache'
Cache miss: /tmp/aa/
Wrote cache: /tmp/aa/
# no options, cache, expect a hit
$ /sbin/apparmor_
Cache: added primary location '/tmp/aa/cache'
Cache hit: /tmp/aa/
# reset
$ rm -rf /tmp/aa/cache/*
# options, no cache, expect a miss and to write
$ /sbin/apparmor_
Cache: added primary location '/tmp/aa/cache'
Cache miss: /tmp/aa/
Wrote cache: /tmp/aa/
# options, cache, expect a hit
$ /sbin/apparmor_
Cache: added primary location '/tmp/aa/cache'
Cache miss: /tmp/aa/
Wrote cache: /tmp/aa/
Same thing happens if omitting -O no-expr-simplify but add Optimize=
[Regression Potential]
The regression potential is considered low since the patch is simple and easily verifiable. If something went wrong, it would be around cache invalidation which the above test case will demonstrate it works correctly.
# Original description
With 2.13.2 and the most recent testsuite patches from the 2.13 branch, I find that the cache works correctly when no options are specified. Eg
# setup
$ mkdir -p /tmp/aa/cache /tmp/aa/profiles
$ cp /etc/apparmor.
# no options, no cache, expect a miss and to write
$ /sbin/apparmor_
Cache: added primary location '/tmp/aa/cache'
Cache miss: /tmp/aa/
Wrote cache: /tmp/aa/
# no options, cache, expect a hit
$ /sbin/apparmor_
Cache: added primary location '/tmp/aa/cache'
Cache hit: /tmp/aa/
# reset
$ rm -rf /tmp/aa/cache/*
# options, no cache, expect a miss and to write
$ /sbin/apparmor_
Cache: added primary location '/tmp/aa/cache'
Cache miss: /tmp/aa/
Wrote cache: /tmp/aa/
# options, cache, expect a hit
$ /sbin/apparmor_
Cache: added primary location '/tmp/aa/cache'
Cache miss: /tmp/aa/
Wrote cache: /tmp/aa/
Same thing happens if omitting -O no-expr-simplify but add Optimize=
Jamie Strandboge (jdstrand) wrote : | #1 |
Jamie Strandboge (jdstrand) wrote : | #2 |
I can say that this was also not introduced by these latest commits:
5704fba8d935e8e
9374f419a0169e1
68eb3be2ae87775
01aec04bd645744
b502110dcf629a7
description: | updated |
tags: | added: aa-parser |
Jamie Strandboge (jdstrand) wrote : | #3 |
Adding an Ubuntu 19.04 task in anticipation of the 2.13.2 upload.
description: | updated |
Jamie Strandboge (jdstrand) wrote : | #4 |
Upstream patch is here: https:/
Changed in apparmor: | |
status: | New → In Progress |
Changed in apparmor (Ubuntu): | |
status: | New → Triaged |
Changed in apparmor (Ubuntu Eoan): | |
assignee: | nobody → Jamie Strandboge (jdstrand) |
Changed in apparmor (Ubuntu Disco): | |
assignee: | nobody → Jamie Strandboge (jdstrand) |
Changed in apparmor (Ubuntu Eoan): | |
status: | Triaged → In Progress |
Changed in apparmor (Ubuntu Disco): | |
status: | New → Triaged |
Changed in apparmor (Ubuntu Eoan): | |
status: | In Progress → Fix Committed |
Changed in apparmor: | |
status: | In Progress → Fix Released |
Changed in apparmor (Ubuntu Disco): | |
status: | Triaged → In Progress |
Changed in apparmor: | |
assignee: | nobody → John Johansen (jjohansen) |
Launchpad Janitor (janitor) wrote : | #5 |
This bug was fixed in the package apparmor - 2.13.2-9ubuntu7
---------------
apparmor (2.13.2-9ubuntu7) eoan; urgency=medium
* lp1820068.patch: don't skip read cache when options are set (LP: #1820068)
* reenable ubuntu/
-- Jamie Strandboge <email address hidden> Thu, 06 Jun 2019 21:46:34 +0000
Changed in apparmor (Ubuntu Eoan): | |
status: | Fix Committed → Fix Released |
description: | updated |
description: | updated |
Hello Jamie, or anyone else affected,
Accepted apparmor into disco-proposed. The package will build now and be available at https:/
Please help us by testing this new package. See https:/
If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested and change the tag from verification-
Further information regarding the verification process can be found at https:/
N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.
Changed in apparmor (Ubuntu Disco): | |
status: | In Progress → Fix Committed |
tags: | added: verification-needed verification-needed-disco |
Jamie Strandboge (jdstrand) wrote : | #7 |
FYI, I tested this and 2.13.2-9ubuntu6.1 fixes this bug.
I also executed https:/
Lastly, I wanted to double check the performance impact of no-expr-simplify on amd64 especially as it pertains to cloud and found it was slightly faster for default cloud, lxd and server policy.
(It was slower on desktop, as expected and in the expected way due to the somewhat pathological evince profile. This has been the historical acceptable tradeoff to help ARM devices (note that because no-expr-simplify is faster with snap policy, with a handful of additional snaps, desktop breaks even then eventually gets faster too).
tags: |
added: verification-done verification-done-disco removed: verification-needed verification-needed-disco |
Launchpad Janitor (janitor) wrote : | #8 |
This bug was fixed in the package apparmor - 2.13.2-9ubuntu6.1
---------------
apparmor (2.13.2-9ubuntu6.1) disco-proposed; urgency=medium
* lp1820068.patch: don't skip read cache when options are set (LP: #1820068)
* reenable ubuntu/
-- Jamie Strandboge <email address hidden> Thu, 06 Jun 2019 21:04:34 +0000
Changed in apparmor (Ubuntu Disco): | |
status: | Fix Committed → Fix Released |
The verification of the Stable Release Update for apparmor has completed successfully and the package has now been released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.
Changed in apparmor: | |
assignee: | John Johansen (jjohansen) → Jaroslavas Karmazinas (cheops) |
I verified this does not affect Ubuntu 18.04 which uses 2.12.