On Thu, Apr 16, 2020 at 3:41 AM Paul Moore <paul@xxxxxxxxxxxxxx> wrote:
> On Tue, Apr 7, 2020 at 2:29 PM Ondrej Mosnacek <omosnace@xxxxxxxxxx> wrote:
> >
> > Currently, they are stored in a linked list, which adds significant
> > overhead to security_transition_sid(). On Fedora, with 428 role
> > transitions in policy, converting this list to a hash table cuts down
> > its run time by about 50%. This was measured by running 'stress-ng --msg
> > 1 --msg-ops 100000' under perf with and without this patch.
> >
> > Signed-off-by: Ondrej Mosnacek <omosnace@xxxxxxxxxx>
> > ---
> >
> > v2:
> > - fix typo scontext->tcontext in security_compute_sid()
> > - suggest a better command for testing in the commit msg
> >
> > security/selinux/ss/policydb.c | 138 ++++++++++++++++++++++-----------
> > security/selinux/ss/policydb.h | 8 +-
> > security/selinux/ss/services.c | 21 +++--
> > 3 files changed, 107 insertions(+), 60 deletions(-)
>
> ...
>
> > diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c
> > index 70ecdc78efbd..4f0cfffd008d 100644
> > --- a/security/selinux/ss/policydb.c
> > +++ b/security/selinux/ss/policydb.c
> > @@ -458,6 +465,30 @@ static int rangetr_cmp(struct hashtab *h, const void *k1, const void *k2)
> > return v;
> > }
> >
> > +static u32 role_trans_hash(struct hashtab *h, const void *k)
> > +{
> > + const struct role_trans_key *key = k;
> > +
> > + return (key->role + (key->type << 3) + (key->tclass << 5)) &
> > + (h->size - 1);
> > +}
> > +
> > +static int role_trans_cmp(struct hashtab *h, const void *k1, const void *k2)
> > +{
> > + const struct role_trans_key *key1 = k1, *key2 = k2;
> > + int v;
> > +
> > + v = key1->role - key2->role;
> > + if (v)
> > + return v;
> > +
> > + v = key1->type - key2->type;
> > + if (v)
> > + return v;
> > +
> > + return key1->tclass - key2->tclass;
>
> Why just a simple boolean statement?
>
> return key1->role != key2->role || \
> key1->type != key2->type || \
> key1->tclass != key2->tclass;
Because hashtab sorts the entries in each bucket, so it needs a proper
sort function. Other such functions (rangetr_cmp(), filenametr_cmp())
do a similar thing.
--
Ondrej Mosnacek <omosnace at redhat dot com>
Software Engineer, Security Technologies
Red Hat, Inc.
