Re: [PATCH 3/4] selinux: reject an unclaimed class value in security_get_classes()

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] The boolean symbol table can be sparse, leading to unconditional NULL pointer dereferences in `security_get_bools()`, `security_set_bools()`, and `cond_evaluate_expr()`.
--

--- Patch [3]: [PATCH 3/4] selinux: reject an unclaimed class value in security_get_classes() ---
commit 8c3d50aa472cc225281a590fe76e7aab43766da5
Author: Bryam Vargas <hexlabsecurity@proton.me>

selinux: reject an unclaimed class value in security_get_classes()

This commit updates security_get_classes() to fail the lookup if a class
value went unclaimed. This prevents a NULL pointer dereference in
sel_make_classes() when the policy defines a sparse class symbol table.

> diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
> index 90e81186cb2ee..7afce975436e9 100644
> --- a/security/selinux/ss/services.c
> +++ b/security/selinux/ss/services.c
[ ... ]
> @@ -3316,16 +3317,29 @@ int security_get_classes(struct selinux_policy *policy,
>  
>  	rc = hashtab_map(&policydb->p_classes.table, get_classes_callback,
>  			 *classes);
> -	if (rc) {
> -		u32 i;
> +	if (rc)
> +		goto err;
>  
> -		for (i = 0; i < *nclasses; i++)
> -			kfree((*classes)[i]);
> -		kfree(*classes);
> +	/*
> +	 * The class symtab may be sparse, which policydb_class_isvalid() exists
> +	 * to absorb; the callback fills this array by value, so an unclaimed
> +	 * one leaves a NULL that sel_make_classes() hands to sel_make_dir().
> +	 */
> +	for (i = 0; i < *nclasses; i++) {
> +		if (!(*classes)[i]) {
> +			rc = -EINVAL;
> +			goto err;
> +		}
>  	}

[Severity: High]
This is a pre-existing issue, but since this patch handles sparse class
symbol tables, does the boolean symbol table have a similar vulnerability?

In security/selinux/ss/services.c:security_get_bools(), there is an
unconditional dereference of bool_val_to_struct:

security_get_bools() {
    ...
        (*values)[i] = policydb->bool_val_to_struct[i]->state;
    ...
}

Similarly in security/selinux/ss/services.c:security_set_bools():

security_set_bools() {
    ...
        int old_state = newpolicy->policydb.bool_val_to_struct[i]->state;
    ...
}

And in security/selinux/ss/conditional.c:cond_evaluate_expr():

cond_evaluate_expr() {
    ...
        s[sp] = p->bool_val_to_struct[node->boolean - 1]->state;
    ...
}

Can a malformed policy with a sparse boolean symbol table cause a NULL
pointer dereference in these locations?

>  out:
>  	return rc;
> +
> +err:
> +	for (i = 0; i < *nclasses; i++)
> +		kfree((*classes)[i]);
> +	kfree(*classes);
> +	return rc;
>  }

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