Re: [PATCH 4/4] selinux: require every boolean value to be defined

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing validation for sparse types and users in `policydb_context_isvalid()` allows a crafted SELinux policy to trigger kernel panics.
--

--- Patch [4]: [PATCH 4/4] selinux: require every boolean value to be defined ---
commit 44c8d8193cd1a1c62c2bebd114870d7131ceb5ab
Author: Bryam Vargas <hexlabsecurity@proton.me>

selinux: require every boolean value to be defined

This commit adds validation during SELinux policy loading to ensure every
declared boolean is defined. This prevents NULL pointer dereferences when
consumers iterate over the bool_val_to_struct array.

Cc: stable@vger.kernel.org

> diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c
> index d358200817bd9..d88713201be92 100644
> --- a/security/selinux/ss/policydb.c
> +++ b/security/selinux/ss/policydb.c

[ ... ]

> @@ -769,6 +770,24 @@ static int policydb_index(struct policydb *p)
>  		if (rc)
>  			goto out;
>  	}
> +
> +	/*
> +	 * A sparse class value is absorbed by policydb_class_isvalid() and
> +	 * its siblings, but no such predicate exists for booleans: every
> +	 * user of bool_val_to_struct[] walks it by index and dereferences
> +	 * each entry -- cond_evaluate_expr(), the two getters and
> +	 * security_set_bools() -- so an unclaimed one has no consumer that
> +	 * can tolerate it.
> +	 */
> +	for (v = 0; v < p->p_bools.nprim; v++) {
> +		if (!p->bool_val_to_struct[v]) {
> +			pr_err("SELinux:  boolean %u is declared but not defined\n",
> +			       v + 1);
> +			rc = -EINVAL;
> +			goto out;
> +		}
> +	}
> +

[Severity: High]
This is a pre-existing issue, but since this patch adds validation for
sparse booleans, is it necessary to add similar validation for sparse types
and users?

Context validation skips checking if the type is actually defined when
c->role == OBJECT_R_VAL:

security/selinux/ss/policydb.c:policydb_context_isvalid() {
    ...
    if (!c->type || c->type > p->p_types.nprim)
        return false;

    if (c->role != OBJECT_R_VAL) {
    ...
}

If a crafted policy assigns a sparse type to an object context, it is
accepted. Later, when the context is evaluated, does this trigger a kernel
panic when fetching the NULL struct?

security/selinux/ss/services.c:type_attribute_bounds_av() {
    ...
    target = policydb->type_val_to_struct[tcontext->type - 1];
    BUG_ON(!target);
    ...
}

It also looks like this could cause a NULL pointer dereference during string
conversion (such as in audit logs) because sym_name() returns NULL:

security/selinux/ss/services.c:context_struct_to_string() {
    ...
    *scontext_len += strlen(sym_name(p, SYM_TYPES, context->type - 1)) + 1;
    ...
}

Could a malicious container policy exploit this missing type validation to
crash the host system?

>  	rc = 0;
>  out:
>  	return rc;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260731-b4-disp-d32e997b-v1-0-0341d038ee47@proton.me?part=4