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

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 N=
ULL 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 secu=
rity_get_classes() ---
commit 8c3d50aa472cc225281a590fe76e7aab43766da5
Author: Bryam Vargas <[email protected]>

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/service=
s.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 *p=
olicy,
> =20
>  	rc =3D hashtab_map(&policydb->p_classes.table, get_classes_callback,
>  			 *classes);
> -	if (rc) {
> -		u32 i;
> +	if (rc)
> +		goto err;
> =20
> -		for (i =3D 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 =3D 0; i < *nclasses; i++) {
> +		if (!(*classes)[i]) {
> +			rc =3D -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] =3D policydb->bool_val_to_struct[i]->state;
    ...
}

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

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

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

cond_evaluate_expr() {
    ...
        s[sp] =3D 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 =3D 0; i < *nclasses; i++)
> +		kfree((*classes)[i]);
> +	kfree(*classes);
> +	return rc;
>  }

--=20
Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260731-b4-disp-d3=
[email protected]?part=3D3