Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20230706133004.19064-1-cgzones@googlemail.com>
In-Reply-To: <20230706133004.19064-1-cgzones@googlemail.com>
From:   Paul Moore <paul@paul-moore.com>
Date:   Mon, 17 Jul 2023 16:46:10 -0400
Message-ID: <CAHC9VhS=DzORJHPH+ObJJ8HpsnBqcDw0_PzzP1=TNL9gvaPgsg@mail.gmail.com>
Subject: Re: [RFC PATCH] selinux: implement avtab_search() via avtab_search_node()
To:     =?UTF-8?Q?Christian_G=C3=B6ttsche?= <cgzones@googlemail.com>
Cc:     selinux@vger.kernel.org,
        Stephen Smalley <stephen.smalley.work@gmail.com>,
        Eric Paris <eparis@parisplace.org>,
        linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Thu, Jul 6, 2023 at 9:30=E2=80=AFAM Christian G=C3=B6ttsche
<cgzones@googlemail.com> wrote:
>
> Deduplicate avtab_search() by using the identical implementation from
> avtab_search_node().
>
> Signed-off-by: Christian G=C3=B6ttsche <cgzones@googlemail.com>
> ---
>  security/selinux/ss/avtab.c | 32 --------------------------------
>  security/selinux/ss/avtab.h | 11 ++++++++++-
>  2 files changed, 10 insertions(+), 33 deletions(-)

I only see three avtab_search() callers, and only one that actually
cares about a return value other than NULL/non-NULL.  With that in
mind, how about we remove avtab_search() entirely and update
security_compute_sid() to use avtab_search_node()?  After all, it
already uses it for the conditional rules lookup ... not sure why it
doesn't use it everywhere?

> diff --git a/security/selinux/ss/avtab.c b/security/selinux/ss/avtab.c
> index 6766edc0fe68..33a54fbd989b 100644
> --- a/security/selinux/ss/avtab.c
> +++ b/security/selinux/ss/avtab.c
> @@ -180,38 +180,6 @@ struct avtab_node *avtab_insert_nonunique(struct avt=
ab *h,
>         return avtab_insert_node(h, hvalue, prev, key, datum);
>  }
>
> -struct avtab_datum *avtab_search(struct avtab *h, const struct avtab_key=
 *key)
> -{
> -       int hvalue;
> -       struct avtab_node *cur;
> -       u16 specified =3D key->specified & ~(AVTAB_ENABLED|AVTAB_ENABLED_=
OLD);
> -
> -       if (!h || !h->nslot)
> -               return NULL;
> -
> -       hvalue =3D avtab_hash(key, h->mask);
> -       for (cur =3D h->htable[hvalue]; cur;
> -            cur =3D cur->next) {
> -               if (key->source_type =3D=3D cur->key.source_type &&
> -                   key->target_type =3D=3D cur->key.target_type &&
> -                   key->target_class =3D=3D cur->key.target_class &&
> -                   (specified & cur->key.specified))
> -                       return &cur->datum;
> -
> -               if (key->source_type < cur->key.source_type)
> -                       break;
> -               if (key->source_type =3D=3D cur->key.source_type &&
> -                   key->target_type < cur->key.target_type)
> -                       break;
> -               if (key->source_type =3D=3D cur->key.source_type &&
> -                   key->target_type =3D=3D cur->key.target_type &&
> -                   key->target_class < cur->key.target_class)
> -                       break;
> -       }
> -
> -       return NULL;
> -}
> -
>  /* This search function returns a node pointer, and can be used in
>   * conjunction with avtab_search_next_node()
>   */
> diff --git a/security/selinux/ss/avtab.h b/security/selinux/ss/avtab.h
> index d6742fd9c560..16238c7bcbba 100644
> --- a/security/selinux/ss/avtab.h
> +++ b/security/selinux/ss/avtab.h
> @@ -90,7 +90,6 @@ struct avtab {
>  void avtab_init(struct avtab *h);
>  int avtab_alloc(struct avtab *, u32);
>  int avtab_alloc_dup(struct avtab *new, const struct avtab *orig);
> -struct avtab_datum *avtab_search(struct avtab *h, const struct avtab_key=
 *k);
>  void avtab_destroy(struct avtab *h);
>  void avtab_hash_eval(struct avtab *h, const char *tag);
>
> @@ -110,6 +109,16 @@ struct avtab_node *avtab_insert_nonunique(struct avt=
ab *h,
>
>  struct avtab_node *avtab_search_node(struct avtab *h,
>                                      const struct avtab_key *key);
> +static inline struct avtab_datum *avtab_search(struct avtab *h,
> +                                              const struct avtab_key *ke=
y)
> +{
> +       struct avtab_node *cur =3D avtab_search_node(h, key);
> +
> +       if (cur)
> +               return &cur->datum;
> +
> +       return NULL;
> +}
>
>  struct avtab_node *avtab_search_node_next(struct avtab_node *node, int s=
pecified);
>
> --
> 2.40.1

--=20
paul-moore.com