* Re: [PATCH v3 10/14] selinux: validate symbols
2025-05-11 17:30 ` [PATCH v3 10/14] selinux: validate symbols Christian Göttsche
@ 2025-05-14 19:06 ` Stephen Smalley
2026-05-06 23:43 ` Paul Moore
1 sibling, 0 replies; 3+ messages in thread
From: Stephen Smalley @ 2025-05-14 19:06 UTC (permalink / raw)
To: cgzones
Cc: selinux, Paul Moore, Ondrej Mosnacek, linux-kernel,
Nathan Chancellor, Nick Desaulniers, Bill Wendling, Justin Stitt,
llvm
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
>
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH v3 10/14] selinux: validate symbols
2025-05-11 17:30 ` [PATCH v3 10/14] selinux: validate symbols Christian Göttsche
2025-05-14 19:06 ` Stephen Smalley
@ 2026-05-06 23:43 ` Paul Moore
1 sibling, 0 replies; 3+ messages in thread
From: Paul Moore @ 2026-05-06 23:43 UTC (permalink / raw)
To: Christian Göttsche, selinux
Cc: Christian Göttsche, Stephen Smalley, Ondrej Mosnacek,
linux-kernel, Nathan Chancellor, Nick Desaulniers, Bill Wendling,
Justin Stitt, llvm
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
^ permalink raw reply [flat|nested] 3+ messages in thread