Re: [PATCH 7/9] SELinux: Expose filename_tr hash table sizing via Kconfig

From: Siarhei Liakh <siarhei.liakh@concurrent-rt.com>
To: Ondrej Mosnacek <omosnace@redhat.com>
Cc: SElinux list <selinux@vger.kernel.org>,
	colin.king@canonical.com, Eric Paris <eparis@parisplace.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Jeff Vander Stoep <jeffv@google.com>,
	Paul Moore <paul@paul-moore.com>,
	Stephen Smalley <stephen.smalley.work@gmail.com>,
	Thomas Gleixner <tglx@linutronix.de>
Subject: Re: [PATCH 7/9] SELinux: Expose filename_tr hash table sizing via Kconfig
Date: Tue, 14 Apr 2020 09:39:34 -0400	[thread overview]
Message-ID: <20200414133931.GB10584@concurrent-rt.com> (raw)
In-Reply-To: <CAFqZXNt4+O6Ys-5Xb8mrXyvSsVt6NanuHxkq0oN7BPok-ecvOQ@mail.gmail.com>

The 04/14/2020 12:54, Ondrej Mosnacek wrote:
> Hi Siarhei,
> 
> On Wed, Apr 8, 2020 at 8:24 PM <siarhei.liakh@concurrent-rt.com> wrote:
> >
> > From: Siarhei Liakh <siarhei.liakh@concurrent-rt.com>
> >
> > This change exposes previously hardcoded filename_tr sizing via Kconfig,
> > which provides a more convenient tuning mechanism for downstream distributions.
> > Default sizing is not affected.
> >
> > Signed-off-by: Siarhei Liakh <siarhei.liakh@concurrent-rt.com>
> > ---
> > Please CC me directly in all replies.
> >
> >  security/selinux/Kconfig       | 10 ++++++++++
> >  security/selinux/ss/policydb.c |  3 ++-
> >  2 files changed, 12 insertions(+), 1 deletion(-)
> >
> > diff --git a/security/selinux/Kconfig b/security/selinux/Kconfig
> > index b7ced53ffd76..23ec741b1ce6 100644
> > --- a/security/selinux/Kconfig
> > +++ b/security/selinux/Kconfig
> > @@ -123,6 +123,16 @@ config SECURITY_SELINUX_AVTAB_HASH_BITS
> >           footprint at price of hash table lookup efficiency. One bucket
> >           per 10 to 100 rules is reasonable.
> >
> > +config SECURITY_SELINUX_PDB_FILE_TR_HASH_BITS
> > +       int "Number of slots (buckets) for File Transitions hash table, expressed as number of bits (i.e. 2^n)"
> > +       depends on SECURITY_SELINUX
> > +       range 1 32
> > +       default "11"
> > +       help
> > +         This is a power of 2 representing the number of slots (buckets)
> > +         used for File Transitions hash table. Smaller value reduces memory
> > +         footprint at price of hash table lookup efficiency.
> > +
> >  config SECURITY_SELINUX_CHECKREQPROT_VALUE
> >         int "NSA SELinux checkreqprot default value"
> >         depends on SECURITY_SELINUX
> > diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c
> > index 0d03036ca20d..f2d809dffb25 100644
> > --- a/security/selinux/ss/policydb.c
> > +++ b/security/selinux/ss/policydb.c
> > @@ -496,7 +496,8 @@ static int policydb_init(struct policydb *p)
> >         cond_policydb_init(p);
> >
> >         p->filename_trans = hashtab_create(filenametr_hash, filenametr_cmp,
> > -                                          (1 << 11));
> > +                          (1 << CONFIG_SECURITY_SELINUX_PDB_FILE_TR_HASH_BITS));
> > +
> >         if (!p->filename_trans)
> >                 return -ENOMEM;
> >
> > --
> > 2.17.1
> 
> Note that this patch in particular won't be needed after (if) [1] gets
> merged. Then for all policies built by new userspace the number of
> elements will be known before the hashtab creation and it will be
> passed to hashtab_create() directly (as is already done for the other
> hashtabs). The hard-coded size will only be used in the
> backwards-compat code path (when a policy built by an older userspace
> is loaded) and thus won't be worth tuning any more.
> 
> [1] https://patchwork.kernel.org/patch/11462503/

This is excellent news!
Unfortunately, my clients tend to have really long product life cycles, meaning
that even with new kernels they will still have old userspace tools. I guess
I'll just keep these patches in our local tree...

Thank you!

-- 
Siarhei Liakh
Concurrent Real-Time