On Mon, Mar 2, 2020 at 3:57 PM Stephen Smalley <stephen.smalley.work@xxxxxxxxx> wrote: > On Mon, Mar 2, 2020 at 9:50 AM Stephen Smalley > <stephen.smalley.work@xxxxxxxxx> wrote: > > > > On Fri, Feb 28, 2020 at 1:08 PM Stephen Smalley > > <stephen.smalley.work@xxxxxxxxx> wrote: > > > > > > On Thu, Feb 27, 2020 at 11:03 AM Ondrej Mosnacek <omosnace@xxxxxxxxxx> wrote: > > > > > > > > This series contains two small changes (these don't seem to affect > > > > performance measurably, but are nonetheless logical) and a patch that > > > > changes how the policy optimization "type_map" helper structure is > > > > represented, which speeds up the whole process. > > > > > > > > Ondrej Mosnacek (3): > > > > libsepol: skip unnecessary check in build_type_map() > > > > libsepol: optimize inner loop in build_type_map() > > > > libsepol: speed up policy optimization > > > > > > Not a comment on the patches themselves, but this made me wonder if > > > the optimization support is actually tested by our travis > > > configuration. > > > Doesn't appear to be (e.g. no usage of -O/--optimize or semanage.conf > > > with optimize-policy true). Yeah, there is currently no test for this. I have something hackish that I used locally - I'll try to convert it to something more usable an automated and integrate it into the repo. > > > > Adding optimize-policy = true to /etc/selinux/semanage.conf and > > running semodule -BN before and after these patches yields different > > binary kernel policy files (policy.32). > > Is that expected? > > Here is one example difference between the policies, along with what > was present in the original unoptimized policy: > $ sesearch -A -s guest_t -t guest_t -c context -p contains policy.32.unoptimized > allow guest_t guest_t:context contains; > allow guest_usertype guest_usertype:context contains; > > $ sesearch -A -s guest_t -t guest_t -c context -p contains > policy.32.optimizedbefore > allow guest_t guest_t:context contains; > > $ sesearch -A -s guest_t -t guest_t -c context -p contains > policy.32.optimizedafter > allow guest_usertype guest_usertype:context contains; > > Seems like the code prior to these changes yielded a more optimal > policy since guest_usertype only has a single type in it. Hm... this is probably a consequence of the second patch. Types are no longer considered a superset of an attribute containing a single type, so the single-type rule gets removed instead of the attribute one... But even before it picked the first rule only by chance (it was first in order). I would say that picking a single-type rule over an attribute rule in this case is outside of the scope of the algorithm. Shouldn't the compiler automatically expand each attribute that has less than 5 types in it? I recall seeing something in the code that did this. I think this was in the CIL part of libsepol, so maybe it applies only when compiling from CIL? -- Ondrej Mosnacek <omosnace at redhat dot com> Software Engineer, Security Technologies Red Hat, Inc.
