[PATCH v3 10/14] selinux: validate symbols

[PATCH v3 10/14] selinux: validate symbols

[Christian Göttsche]

From: Christian Göttsche <cgzones@googlemail.com>

Some symbol tables need to be validated after indexing, since during
indexing their referenced entries might not yet have been indexed.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
 security/selinux/ss/policydb.c | 94 ++++++++++++++++++++++++++++++++++
 1 file changed, 94 insertions(+)

diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c
index f8d6e993ce89..4559c8918134 100644
--- a/security/selinux/ss/policydb.c
+++ b/security/selinux/ss/policydb.c
@@ -673,6 +673,84 @@ static int (*const index_f[SYM_NUM])(void *key, void *datum, void *datap) = {
 };
 /* clang-format on */
 
+static int role_validate(void *key, void *datum, void *datap)
+{
+	const struct policydb *p = datap;
+	const struct role_datum *role = datum;
+	struct ebitmap_node *node;
+	u32 i;
+
+	ebitmap_for_each_positive_bit(&role->dominates, node, i) {
+		if (!policydb_role_isvalid(p, i))
+			goto bad;
+	}
+
+	ebitmap_for_each_positive_bit(&role->types, node, i) {
+		if (!policydb_type_isvalid(p, i + 1))
+			goto bad;
+	}
+
+	return 0;
+
+bad:
+	pr_err("SELinux:  invalid role %s\n", sym_name(p, SYM_ROLES, role->value - 1));
+	return -EINVAL;
+}
+
+static int user_validate(void *key, void *datum, void *datap)
+{
+	const struct policydb *p = datap;
+	const struct user_datum *usrdatum = datum;
+	struct ebitmap_node *node;
+	u32 i;
+
+	ebitmap_for_each_positive_bit(&usrdatum->roles, node, i) {
+		if (!policydb_role_isvalid(p, i))
+			goto bad;
+	}
+
+	if (!mls_range_isvalid(p, &usrdatum->range))
+		goto bad;
+
+	if (!mls_level_isvalid(p, &usrdatum->dfltlevel))
+		goto bad;
+
+	return 0;
+
+bad:
+	pr_err("SELinux:  invalid user %s\n", sym_name(p, SYM_USERS, usrdatum->value - 1));
+	return -EINVAL;
+}
+
+static int sens_validate(void *key, void *datum, void *datap)
+{
+	const struct policydb *p = datap;
+	const struct level_datum *levdatum = datum;
+
+	if (!mls_level_isvalid(p, &levdatum->level))
+		goto bad;
+
+	return 0;
+
+bad:
+	pr_err("SELinux:  invalid sensitivity\n");
+	return -EINVAL;
+}
+
+
+/* clang-format off */
+static int (*const validate_f[SYM_NUM])(void *key, void *datum, void *datap) = {
+	NULL, /* Everything validated in common_read() and common_index() */
+	NULL, /* Everything validated in class_read() and class_index() */
+	role_validate,
+	NULL, /* Everything validated in type_read(), type_index() and type_bounds_sanity_check() */
+	user_validate,
+	NULL, /* Everything validated in cond_read_bool() and cond_index_bool() */
+	sens_validate,
+	NULL, /* Everything validated in cat_read() and cat_index() */
+};
+/* clang-format on */
+
 #ifdef CONFIG_SECURITY_SELINUX_DEBUG
 static void hash_eval(struct hashtab *h, const char *hash_name,
 		      const char *hash_details)
@@ -765,6 +843,16 @@ static int policydb_index(struct policydb *p)
 		if (rc)
 			goto out;
 	}
+
+	for (i = 0; i < SYM_NUM; i++) {
+		if (!validate_f[i])
+			continue;
+
+		rc = hashtab_map(&p->symtab[i].table, validate_f[i], p);
+		if (rc)
+			goto out;
+	}
+
 	rc = 0;
 out:
 	return rc;
@@ -1087,6 +1175,12 @@ static int context_read_and_validate(struct context *c, struct policydb *p,
 			pr_err("SELinux: error reading MLS range of context\n");
 			goto out;
 		}
+
+		rc = -EINVAL;
+		if (!mls_range_isvalid(p, &c->range)) {
+			pr_warn("SELinux: invalid range in security context\n");
+			goto out;
+		}
 	}
 
 	rc = -EINVAL;
-- 
2.49.0

Re: [PATCH v3 10/14] selinux: validate symbols

[Stephen Smalley]

On Sun, May 11, 2025 at 1:31 PM Christian Göttsche
<cgoettsche@seltendoof.de> wrote:
>
> From: Christian Göttsche <cgzones@googlemail.com>
>
> Some symbol tables need to be validated after indexing, since during
> indexing their referenced entries might not yet have been indexed.
>
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>

Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com>

NB role dominance is already gone in checkpolicy and never supported by secilc.
At some point likely should drop corresponding kernel and libsepol support too.

> ---
>  security/selinux/ss/policydb.c | 94 ++++++++++++++++++++++++++++++++++
>  1 file changed, 94 insertions(+)
>
> diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c
> index f8d6e993ce89..4559c8918134 100644
> --- a/security/selinux/ss/policydb.c
> +++ b/security/selinux/ss/policydb.c
> @@ -673,6 +673,84 @@ static int (*const index_f[SYM_NUM])(void *key, void *datum, void *datap) = {
>  };
>  /* clang-format on */
>
> +static int role_validate(void *key, void *datum, void *datap)
> +{
> +       const struct policydb *p = datap;
> +       const struct role_datum *role = datum;
> +       struct ebitmap_node *node;
> +       u32 i;
> +
> +       ebitmap_for_each_positive_bit(&role->dominates, node, i) {
> +               if (!policydb_role_isvalid(p, i))
> +                       goto bad;
> +       }
> +
> +       ebitmap_for_each_positive_bit(&role->types, node, i) {
> +               if (!policydb_type_isvalid(p, i + 1))
> +                       goto bad;
> +       }
> +
> +       return 0;
> +
> +bad:
> +       pr_err("SELinux:  invalid role %s\n", sym_name(p, SYM_ROLES, role->value - 1));
> +       return -EINVAL;
> +}
> +
> +static int user_validate(void *key, void *datum, void *datap)
> +{
> +       const struct policydb *p = datap;
> +       const struct user_datum *usrdatum = datum;
> +       struct ebitmap_node *node;
> +       u32 i;
> +
> +       ebitmap_for_each_positive_bit(&usrdatum->roles, node, i) {
> +               if (!policydb_role_isvalid(p, i))
> +                       goto bad;
> +       }
> +
> +       if (!mls_range_isvalid(p, &usrdatum->range))
> +               goto bad;
> +
> +       if (!mls_level_isvalid(p, &usrdatum->dfltlevel))
> +               goto bad;
> +
> +       return 0;
> +
> +bad:
> +       pr_err("SELinux:  invalid user %s\n", sym_name(p, SYM_USERS, usrdatum->value - 1));
> +       return -EINVAL;
> +}
> +
> +static int sens_validate(void *key, void *datum, void *datap)
> +{
> +       const struct policydb *p = datap;
> +       const struct level_datum *levdatum = datum;
> +
> +       if (!mls_level_isvalid(p, &levdatum->level))
> +               goto bad;
> +
> +       return 0;
> +
> +bad:
> +       pr_err("SELinux:  invalid sensitivity\n");
> +       return -EINVAL;
> +}
> +
> +
> +/* clang-format off */
> +static int (*const validate_f[SYM_NUM])(void *key, void *datum, void *datap) = {
> +       NULL, /* Everything validated in common_read() and common_index() */
> +       NULL, /* Everything validated in class_read() and class_index() */
> +       role_validate,
> +       NULL, /* Everything validated in type_read(), type_index() and type_bounds_sanity_check() */
> +       user_validate,
> +       NULL, /* Everything validated in cond_read_bool() and cond_index_bool() */
> +       sens_validate,
> +       NULL, /* Everything validated in cat_read() and cat_index() */
> +};
> +/* clang-format on */
> +
>  #ifdef CONFIG_SECURITY_SELINUX_DEBUG
>  static void hash_eval(struct hashtab *h, const char *hash_name,
>                       const char *hash_details)
> @@ -765,6 +843,16 @@ static int policydb_index(struct policydb *p)
>                 if (rc)
>                         goto out;
>         }
> +
> +       for (i = 0; i < SYM_NUM; i++) {
> +               if (!validate_f[i])
> +                       continue;
> +
> +               rc = hashtab_map(&p->symtab[i].table, validate_f[i], p);
> +               if (rc)
> +                       goto out;
> +       }
> +
>         rc = 0;
>  out:
>         return rc;
> @@ -1087,6 +1175,12 @@ static int context_read_and_validate(struct context *c, struct policydb *p,
>                         pr_err("SELinux: error reading MLS range of context\n");
>                         goto out;
>                 }
> +
> +               rc = -EINVAL;
> +               if (!mls_range_isvalid(p, &c->range)) {
> +                       pr_warn("SELinux: invalid range in security context\n");
> +                       goto out;
> +               }
>         }
>
>         rc = -EINVAL;
> --
> 2.49.0
>

Re: [PATCH v3 10/14] selinux: validate symbols

[Paul Moore]

On May 11, 2025 =?UTF-8?q?Christian=20G=C3=B6ttsche?= <cgoettsche@seltendoof.de> wrote:
> 
> Some symbol tables need to be validated after indexing, since during
> indexing their referenced entries might not yet have been indexed.
> 
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com>
> ---
>  security/selinux/ss/policydb.c | 94 ++++++++++++++++++++++++++++++++++
>  1 file changed, 94 insertions(+)

...

> diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c
> index f8d6e993ce89..4559c8918134 100644
> --- a/security/selinux/ss/policydb.c
> +++ b/security/selinux/ss/policydb.c
> @@ -765,6 +843,16 @@ static int policydb_index(struct policydb *p)
>  		if (rc)
>  			goto out;
>  	}
> +
> +	for (i = 0; i < SYM_NUM; i++) {
> +		if (!validate_f[i])
> +			continue;
> +
> +		rc = hashtab_map(&p->symtab[i].table, validate_f[i], p);
> +		if (rc)
> +			goto out;
> +	}

Is there a reason why we need a second loop to do the validation?  Can we
simply do the validation in the indexing loop above this?

  for (i = 0; i < SYM_NUM; i++) {
    p->table[i] = kvcalloc(...);

    hashtab_map(p->table, index_f[i]);

    if (validate_f[i])
      hashtab_map(p->table, validate_f[i]);
  }

--
paul-moore.com