2023-07-06 13:38:17

by Christian Göttsche

[permalink] [raw]
Subject: [RFC PATCH] selinux: implement avtab_search() via avtab_search_node()

Deduplicate avtab_search() by using the identical implementation from
avtab_search_node().

Signed-off-by: Christian Göttsche <[email protected]>
---
security/selinux/ss/avtab.c | 32 --------------------------------
security/selinux/ss/avtab.h | 11 ++++++++++-
2 files changed, 10 insertions(+), 33 deletions(-)

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 avtab *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 = key->specified & ~(AVTAB_ENABLED|AVTAB_ENABLED_OLD);
-
- if (!h || !h->nslot)
- return NULL;
-
- hvalue = avtab_hash(key, h->mask);
- for (cur = h->htable[hvalue]; cur;
- cur = cur->next) {
- if (key->source_type == cur->key.source_type &&
- key->target_type == cur->key.target_type &&
- key->target_class == cur->key.target_class &&
- (specified & cur->key.specified))
- return &cur->datum;
-
- if (key->source_type < cur->key.source_type)
- break;
- if (key->source_type == cur->key.source_type &&
- key->target_type < cur->key.target_type)
- break;
- if (key->source_type == cur->key.source_type &&
- key->target_type == 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 avtab *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 *key)
+{
+ struct avtab_node *cur = avtab_search_node(h, key);
+
+ if (cur)
+ return &cur->datum;
+
+ return NULL;
+}

struct avtab_node *avtab_search_node_next(struct avtab_node *node, int specified);

--
2.40.1



2023-07-17 21:20:23

by Paul Moore

[permalink] [raw]
Subject: Re: [RFC PATCH] selinux: implement avtab_search() via avtab_search_node()

On Thu, Jul 6, 2023 at 9:30 AM Christian Göttsche
<[email protected]> wrote:
>
> Deduplicate avtab_search() by using the identical implementation from
> avtab_search_node().
>
> Signed-off-by: Christian Göttsche <[email protected]>
> ---
> 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 avtab *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 = key->specified & ~(AVTAB_ENABLED|AVTAB_ENABLED_OLD);
> -
> - if (!h || !h->nslot)
> - return NULL;
> -
> - hvalue = avtab_hash(key, h->mask);
> - for (cur = h->htable[hvalue]; cur;
> - cur = cur->next) {
> - if (key->source_type == cur->key.source_type &&
> - key->target_type == cur->key.target_type &&
> - key->target_class == cur->key.target_class &&
> - (specified & cur->key.specified))
> - return &cur->datum;
> -
> - if (key->source_type < cur->key.source_type)
> - break;
> - if (key->source_type == cur->key.source_type &&
> - key->target_type < cur->key.target_type)
> - break;
> - if (key->source_type == cur->key.source_type &&
> - key->target_type == 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 avtab *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 *key)
> +{
> + struct avtab_node *cur = avtab_search_node(h, key);
> +
> + if (cur)
> + return &cur->datum;
> +
> + return NULL;
> +}
>
> struct avtab_node *avtab_search_node_next(struct avtab_node *node, int specified);
>
> --
> 2.40.1

--
paul-moore.com