Re: [PATCH 8/9] SELinux: Replace custom hash with generic lookup3 in symtab

[Ondrej Mosnacek <omosnace@redhat.com>]

Hi,

On Wed, Apr 8, 2020 at 8:24 PM <siarhei.liakh@concurrent-rt.com> wrote:
>
> From: Siarhei Liakh <siarhei.liakh@concurrent-rt.com>
>
> This patch replaces local copy of custom hash function with existing
> implementation of lookup3 from the standard Linux library. This change
> allows to reduce the amount of custom code with has to be maintained, while
> potentially improving overall performance of the hash table in question.
>
> Signed-off-by: Siarhei Liakh <siarhei.liakh@concurrent-rt.com>
> ---
> Please CC me directly in all replies.
>
>  security/selinux/ss/symtab.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>
> diff --git a/security/selinux/ss/symtab.c b/security/selinux/ss/symtab.c
> index dc2ce94165d3..8d189d7683d1 100644
> --- a/security/selinux/ss/symtab.c
> +++ b/security/selinux/ss/symtab.c
> @@ -9,6 +9,16 @@
>  #include <linux/errno.h>
>  #include "symtab.h"
>
> +#ifdef CONFIG_SECURITY_SELINUX_ADVANCED_HASHING
> +#include <linux/jhash.h>
> +
> +static unsigned int symhash(struct hashtab *h, const void *key)
> +{
> +       return jhash(key, strlen((const char *) key), 0) & (h->size - 1);

Did you consider using full_name_hash() here instead? It is used in
other places (mainly filesystem code) to hash strings. I wonder how it
compares to jhash both in terms of speed and in terms of randomness of
the resulting hash. It would be nice if you could do some benchmarks
and provide some numbers to support the choice.

> +}
> +
> +#else /* #ifdef CONFIG_SECURITY_SELINUX_ADVANCED_HASHING */
> +
>  static unsigned int symhash(struct hashtab *h, const void *key)
>  {
>         const char *p, *keyp;
> @@ -23,6 +33,8 @@ static unsigned int symhash(struct hashtab *h, const void *key)
>         return val & (h->size - 1);
>  }
>
> +#endif /* #else #ifdef CONFIG_SECURITY_SELINUX_ADVANCED_HASHING */
> +
>  static int symcmp(struct hashtab *h, const void *key1, const void *key2)
>  {
>         const char *keyp1, *keyp2;
> --
> 2.17.1
>

--
Ondrej Mosnacek <omosnace at redhat dot com>
Software Engineer, Security Technologies
Red Hat, Inc.