Re: [PATCH v2 2/2] selinux: reject a class permission count below its inherited common

Stephen Smalley <[email protected]> Tue, 28 Jul 2026 10:43:39 -0400
Newsgroups org.kernel.vger.selinux,org.kernel.vger.linux-kernel
Message-ID <CAEjxPJ4X_p6Qpebe9cV=eruOVM2HXHMDWsWJpQA3QoBc+W1kXA@mail.gmail.com>
On Mon, Jul 27, 2026 at 9:30 PM Bryam Vargas via B4 Relay
<[email protected]> wrote:
>
> From: Bryam Vargas <[email protected]>
>
> security_get_permissions() maps an inherited common's permissions into
> an array sized by the class's own permissions.nprim, but class_read()
> takes that nprim verbatim from the policy image and never checks that it
> covers the common.  A class that inherits a common of N permissions while
> declaring a smaller nprim is accepted, and on load the common's
> permissions are written past the class-sized array -- an out-of-bounds
> heap write.
>
> Reject a class whose permission count is below its inherited common's.
> Well-formed policies, where the class count already includes the
> inherited permissions, are unaffected.
>
> Fixes: 55fcf09b3fe4 ("selinux: add support for querying object classes and permissions from the running policy")
> Cc: [email protected]
> Signed-off-by: Bryam Vargas <[email protected]>

Acked-by: Stephen Smalley <[email protected]>

Sashiko again made some interesting observations but not about your
patch per se,
https://lore.kernel.org/selinux/[email protected]/T/#u

> ---
>  security/selinux/ss/policydb.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>
> diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c
> index 6973b68d9782..c6f55f0b4cb5 100644
> --- a/security/selinux/ss/policydb.c
> +++ b/security/selinux/ss/policydb.c
> @@ -1422,6 +1422,18 @@ static int class_read(struct policydb *p, struct symtab *s, struct policy_file *
>                                cladatum->comkey);
>                         goto bad;
>                 }
> +
> +               /*
> +                * security_get_permissions() maps the common's permissions
> +                * into an array sized by this class's nprim, so a class must
> +                * declare at least as many as the common it inherits.
> +                */
> +               if (cladatum->permissions.nprim <
> +                   cladatum->comdatum->permissions.nprim) {
> +                       pr_err("SELinux:  class %s has fewer permissions than common %s\n",
> +                              key, cladatum->comkey);
> +                       goto bad;
> +               }
>         }
>         for (i = 0; i < nel; i++) {
>                 rc = perm_read(p, &cladatum->permissions, fp);
>
> --
> 2.55.0
>
>