[PATCH] selinux: reject a permission value exceeding the class permission count

[PATCH] selinux: reject a permission value exceeding the class permission count

[Bryam Vargas via B4 Relay]

From: Bryam Vargas <hexlabsecurity@proton.me>

perm_read() bounds a permission value by SEL_VEC_MAX but never by the
owning class or common's nprim, which is taken verbatim from the policy
image.  security_get_permissions() then writes perms[value - 1] into an
nprim-sized kcalloc() array, so a class declaring fewer permissions than
its largest permission value drives an out-of-bounds heap write.  The
top-level symbol tables are validated this way; the nested per-class
permission table is not.

Reject a permission whose value exceeds nprim, which is already set when
perm_read() runs.  Well-formed policies are unaffected.

Fixes: 55fcf09b3fe4 ("selinux: add support for querying object classes and permissions from the running policy")
Cc: stable@vger.kernel.org
Signed-off-by: Bryam Vargas <hexlabsecurity@proton.me>
---
Reproducer and A/B verification (below the --- so git am drops it):

A binary policy whose "process" class carries permissions.nprim = 1 while
keeping its 31 permissions (values 1..31) is accepted by the parser.  On
load, security_get_permissions() allocates a one-entry array and writes
perms[value - 1] for each permission -- a heap slab-out-of-bounds write.

Tested on a KASAN kernel, loading via /sys/fs/selinux/load:

  Build A (without this patch): loading the malformed policy ->
    BUG: KASAN: slab-out-of-bounds in get_permissions_callback
    Write of size 8 ... security_get_permissions()
    (followed by a GPF once an adjacent object's pointer is overwritten)

  Build B (with this patch): the same policy is rejected at parse time
    (-EINVAL); no KASAN report.

  Control (without this patch): a well-formed policy (nprim = 31) loads
    cleanly, no KASAN report -- the fault is specific to nprim < value.

Reachable by a process with CAP_MAC_ADMIN writing a crafted policy to
/sys/fs/selinux/load; SELinux policy load is not namespaced.
---
 security/selinux/ss/policydb.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c
index ead504a639e3..6973b68d9782 100644
--- a/security/selinux/ss/policydb.c
+++ b/security/selinux/ss/policydb.c
@@ -1175,6 +1175,9 @@ static int perm_read(struct policydb *p, struct symtab *s, struct policy_file *f
 	rc = -EINVAL;
 	if (perdatum->value < 1 || perdatum->value > SEL_VEC_MAX)
 		goto bad;
+	/* the value indexes an nprim-sized array in security_get_permissions() */
+	if (perdatum->value > s->nprim)
+		goto bad;
 
 	rc = str_read(&key, GFP_KERNEL, fp, len);
 	if (rc)

---
base-commit: 48a5a7ab8d6ab7090564339e039c421f315de912
change-id: 20260724-b4-disp-ec8ac9f6-576c3f177a04

Best regards,
--  
Bryam Vargas <hexlabsecurity@proton.me>

Re: [PATCH] selinux: reject a permission value exceeding the class permission count

[Stephen Smalley]

On Fri, Jul 24, 2026 at 10:47 AM Bryam Vargas via B4 Relay
<devnull+hexlabsecurity.proton.me@kernel.org> wrote:
>
> From: Bryam Vargas <hexlabsecurity@proton.me>
>
> perm_read() bounds a permission value by SEL_VEC_MAX but never by the
> owning class or common's nprim, which is taken verbatim from the policy
> image.  security_get_permissions() then writes perms[value - 1] into an
> nprim-sized kcalloc() array, so a class declaring fewer permissions than
> its largest permission value drives an out-of-bounds heap write.  The
> top-level symbol tables are validated this way; the nested per-class
> permission table is not.
>
> Reject a permission whose value exceeds nprim, which is already set when
> perm_read() runs.  Well-formed policies are unaffected.
>
> Fixes: 55fcf09b3fe4 ("selinux: add support for querying object classes and permissions from the running policy")
> Cc: stable@vger.kernel.org
> Signed-off-by: Bryam Vargas <hexlabsecurity@proton.me>
> ---
> Reproducer and A/B verification (below the --- so git am drops it):
>
> A binary policy whose "process" class carries permissions.nprim = 1 while
> keeping its 31 permissions (values 1..31) is accepted by the parser.  On
> load, security_get_permissions() allocates a one-entry array and writes
> perms[value - 1] for each permission -- a heap slab-out-of-bounds write.
>
> Tested on a KASAN kernel, loading via /sys/fs/selinux/load:
>
>   Build A (without this patch): loading the malformed policy ->
>     BUG: KASAN: slab-out-of-bounds in get_permissions_callback
>     Write of size 8 ... security_get_permissions()
>     (followed by a GPF once an adjacent object's pointer is overwritten)
>
>   Build B (with this patch): the same policy is rejected at parse time
>     (-EINVAL); no KASAN report.
>
>   Control (without this patch): a well-formed policy (nprim = 31) loads
>     cleanly, no KASAN report -- the fault is specific to nprim < value.
>
> Reachable by a process with CAP_MAC_ADMIN writing a crafted policy to
> /sys/fs/selinux/load; SELinux policy load is not namespaced.

Thanks for the report and patch - one point of clarification to your
statement above:
SELinux doesn't check CAP_MAC_ADMIN for policy load since SELinux predates
CAP_MAC_ADMIN and defines and checks its own permission checks (in this case,
security:load_policy) for its operations. You need to be able to open
/sys/fs/selinux/load
with write access so that's typically uid-0 or CAP_DAC_OVERRIDE along with the
SELinux load_policy permission check.

> ---
>  security/selinux/ss/policydb.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c
> index ead504a639e3..6973b68d9782 100644
> --- a/security/selinux/ss/policydb.c
> +++ b/security/selinux/ss/policydb.c
> @@ -1175,6 +1175,9 @@ static int perm_read(struct policydb *p, struct symtab *s, struct policy_file *f
>         rc = -EINVAL;
>         if (perdatum->value < 1 || perdatum->value > SEL_VEC_MAX)
>                 goto bad;
> +       /* the value indexes an nprim-sized array in security_get_permissions() */
> +       if (perdatum->value > s->nprim)
> +               goto bad;
>
>         rc = str_read(&key, GFP_KERNEL, fp, len);
>         if (rc)
>
> ---
> base-commit: 48a5a7ab8d6ab7090564339e039c421f315de912
> change-id: 20260724-b4-disp-ec8ac9f6-576c3f177a04
>
> Best regards,
> --
> Bryam Vargas <hexlabsecurity@proton.me>
>
>

Re: [PATCH] selinux: reject a permission value exceeding the class permission count

[Stephen Smalley]

On Fri, Jul 24, 2026 at 10:47 AM Bryam Vargas via B4 Relay
<devnull+hexlabsecurity.proton.me@kernel.org> wrote:
>
> From: Bryam Vargas <hexlabsecurity@proton.me>
>
> perm_read() bounds a permission value by SEL_VEC_MAX but never by the
> owning class or common's nprim, which is taken verbatim from the policy
> image.  security_get_permissions() then writes perms[value - 1] into an
> nprim-sized kcalloc() array, so a class declaring fewer permissions than
> its largest permission value drives an out-of-bounds heap write.  The
> top-level symbol tables are validated this way; the nested per-class
> permission table is not.
>
> Reject a permission whose value exceeds nprim, which is already set when
> perm_read() runs.  Well-formed policies are unaffected.
>
> Fixes: 55fcf09b3fe4 ("selinux: add support for querying object classes and permissions from the running policy")
> Cc: stable@vger.kernel.org
> Signed-off-by: Bryam Vargas <hexlabsecurity@proton.me>

Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com>
but also see:
https://lore.kernel.org/selinux/CAEjxPJ7YRgtOXrbPYar0qXAq=wNbOTTMS-EsU=9HKR1NQ3qA1g@mail.gmail.com/T/#me98ec932b314ae94242ed3af7ab74445d273cc7d

> ---
> Reproducer and A/B verification (below the --- so git am drops it):
>
> A binary policy whose "process" class carries permissions.nprim = 1 while
> keeping its 31 permissions (values 1..31) is accepted by the parser.  On
> load, security_get_permissions() allocates a one-entry array and writes
> perms[value - 1] for each permission -- a heap slab-out-of-bounds write.
>
> Tested on a KASAN kernel, loading via /sys/fs/selinux/load:
>
>   Build A (without this patch): loading the malformed policy ->
>     BUG: KASAN: slab-out-of-bounds in get_permissions_callback
>     Write of size 8 ... security_get_permissions()
>     (followed by a GPF once an adjacent object's pointer is overwritten)
>
>   Build B (with this patch): the same policy is rejected at parse time
>     (-EINVAL); no KASAN report.
>
>   Control (without this patch): a well-formed policy (nprim = 31) loads
>     cleanly, no KASAN report -- the fault is specific to nprim < value.
>
> Reachable by a process with CAP_MAC_ADMIN writing a crafted policy to
> /sys/fs/selinux/load; SELinux policy load is not namespaced.
> ---
>  security/selinux/ss/policydb.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c
> index ead504a639e3..6973b68d9782 100644
> --- a/security/selinux/ss/policydb.c
> +++ b/security/selinux/ss/policydb.c
> @@ -1175,6 +1175,9 @@ static int perm_read(struct policydb *p, struct symtab *s, struct policy_file *f
>         rc = -EINVAL;
>         if (perdatum->value < 1 || perdatum->value > SEL_VEC_MAX)
>                 goto bad;
> +       /* the value indexes an nprim-sized array in security_get_permissions() */
> +       if (perdatum->value > s->nprim)
> +               goto bad;
>
>         rc = str_read(&key, GFP_KERNEL, fp, len);
>         if (rc)
>
> ---
> base-commit: 48a5a7ab8d6ab7090564339e039c421f315de912
> change-id: 20260724-b4-disp-ec8ac9f6-576c3f177a04
>
> Best regards,
> --
> Bryam Vargas <hexlabsecurity@proton.me>
>
>

Re: [PATCH] selinux: reject a permission value exceeding the class permission count

[Stephen Smalley]

On Fri, Jul 24, 2026 at 11:48 AM Stephen Smalley
<stephen.smalley.work@gmail.com> wrote:
>
> On Fri, Jul 24, 2026 at 10:47 AM Bryam Vargas via B4 Relay
> <devnull+hexlabsecurity.proton.me@kernel.org> wrote:
> >
> > From: Bryam Vargas <hexlabsecurity@proton.me>
> >
> > perm_read() bounds a permission value by SEL_VEC_MAX but never by the
> > owning class or common's nprim, which is taken verbatim from the policy
> > image.  security_get_permissions() then writes perms[value - 1] into an
> > nprim-sized kcalloc() array, so a class declaring fewer permissions than
> > its largest permission value drives an out-of-bounds heap write.  The
> > top-level symbol tables are validated this way; the nested per-class
> > permission table is not.
> >
> > Reject a permission whose value exceeds nprim, which is already set when
> > perm_read() runs.  Well-formed policies are unaffected.
> >
> > Fixes: 55fcf09b3fe4 ("selinux: add support for querying object classes and permissions from the running policy")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Bryam Vargas <hexlabsecurity@proton.me>
>
> Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com>
> but also see:
> https://lore.kernel.org/selinux/CAEjxPJ7YRgtOXrbPYar0qXAq=wNbOTTMS-EsU=9HKR1NQ3qA1g@mail.gmail.com/T/#me98ec932b314ae94242ed3af7ab74445d273cc7d

Also wondering to what extent this may overlap with this series that
was never fully merged:
https://lore.kernel.org/selinux/20250511173055.406906-15-cgoettsche@seltendoof.de/

Re: [PATCH] selinux: reject a permission value exceeding the class permission count

[Bryam Vargas]

On Fri, 24 Jul 2026 11:56:11 -0400, Stephen Smalley wrote:
> Also wondering to what extent this may overlap with this series that
> was never fully merged:
> https://lore.kernel.org/selinux/20250511173055.406906-15-cgoettsche@seltendoof.de/

Little concrete overlap. I went through the v3 "harden against malformed
policies" series: patch 3 adds the value > SEL_VEC_MAX, nel, U16_MAX-class and
default-* enum bounds (my hunk sits on top of its perm_read value > SEL_VEC_MAX
check, already in mainline), and later patches bound the top-level symbol values
against the symtab counts. None of the 14 patches bounds a permission value
against the owning class/common nprim or touches security_get_permissions(), so
the OOB this patch closes isn't covered there. Happy to coordinate with Christian
(on Cc) so this and a revived series don't collide.

The review is right that this is incomplete. A class that inherits a common maps
the common's permissions into an array sized by the class's own nprim
(security_get_permissions() -> get_permissions_callback(), perms[value - 1]),
and nothing checks the class nprim covers the common; perm_read() doesn't catch
it because the common's values are bounded against the common's nprim. v2 adds a
class_read() check rejecting permissions.nprim < comdatum->permissions.nprim,
with the A/B for that trigger. The sparse-value NULL deref in
sel_make_perm_files() is separate -- I can fold it into the series or post it on
its own, whichever you prefer.

Correct on CAP_MAC_ADMIN too: the precondition is the load_policy permission
plus DAC write to /sys/fs/selinux/load, not the capability. That line was below
the --- so it never entered the commit message.

Thanks for the ack.

Bryam

Re: [PATCH] selinux: reject a permission value exceeding the class permission count

[Stephen Smalley]

On Fri, Jul 24, 2026 at 1:07 PM Bryam Vargas <hexlabsecurity@proton.me> wrote:
>
> On Fri, 24 Jul 2026 11:56:11 -0400, Stephen Smalley wrote:
> > Also wondering to what extent this may overlap with this series that
> > was never fully merged:
> > https://lore.kernel.org/selinux/20250511173055.406906-15-cgoettsche@seltendoof.de/
>
> Little concrete overlap. I went through the v3 "harden against malformed
> policies" series: patch 3 adds the value > SEL_VEC_MAX, nel, U16_MAX-class and
> default-* enum bounds (my hunk sits on top of its perm_read value > SEL_VEC_MAX
> check, already in mainline), and later patches bound the top-level symbol values
> against the symtab counts. None of the 14 patches bounds a permission value
> against the owning class/common nprim or touches security_get_permissions(), so
> the OOB this patch closes isn't covered there. Happy to coordinate with Christian
> (on Cc) so this and a revived series don't collide.
>
> The review is right that this is incomplete. A class that inherits a common maps
> the common's permissions into an array sized by the class's own nprim
> (security_get_permissions() -> get_permissions_callback(), perms[value - 1]),
> and nothing checks the class nprim covers the common; perm_read() doesn't catch
> it because the common's values are bounded against the common's nprim. v2 adds a
> class_read() check rejecting permissions.nprim < comdatum->permissions.nprim,
> with the A/B for that trigger. The sparse-value NULL deref in
> sel_make_perm_files() is separate -- I can fold it into the series or post it on
> its own, whichever you prefer.
>
> Correct on CAP_MAC_ADMIN too: the precondition is the load_policy permission
> plus DAC write to /sys/fs/selinux/load, not the capability. That line was below
> the --- so it never entered the commit message.
>
> Thanks for the ack.

Ok, thanks, will defer to Paul on whether to just take this patch or
wait on a larger one.

Re: [PATCH] selinux: reject a permission value exceeding the class permission count

[Paul Moore]

On Fri, Jul 24, 2026 at 1:11 PM Stephen Smalley
<stephen.smalley.work@gmail.com> wrote:
> On Fri, Jul 24, 2026 at 1:07 PM Bryam Vargas <hexlabsecurity@proton.me> wrote:
> >
> > On Fri, 24 Jul 2026 11:56:11 -0400, Stephen Smalley wrote:
> > > Also wondering to what extent this may overlap with this series that
> > > was never fully merged:
> > > https://lore.kernel.org/selinux/20250511173055.406906-15-cgoettsche@seltendoof.de/
> >
> > Little concrete overlap. I went through the v3 "harden against malformed
> > policies" series: patch 3 adds the value > SEL_VEC_MAX, nel, U16_MAX-class and
> > default-* enum bounds (my hunk sits on top of its perm_read value > SEL_VEC_MAX
> > check, already in mainline), and later patches bound the top-level symbol values
> > against the symtab counts. None of the 14 patches bounds a permission value
> > against the owning class/common nprim or touches security_get_permissions(), so
> > the OOB this patch closes isn't covered there. Happy to coordinate with Christian
> > (on Cc) so this and a revived series don't collide.
> >
> > The review is right that this is incomplete. A class that inherits a common maps
> > the common's permissions into an array sized by the class's own nprim
> > (security_get_permissions() -> get_permissions_callback(), perms[value - 1]),
> > and nothing checks the class nprim covers the common; perm_read() doesn't catch
> > it because the common's values are bounded against the common's nprim. v2 adds a
> > class_read() check rejecting permissions.nprim < comdatum->permissions.nprim,
> > with the A/B for that trigger. The sparse-value NULL deref in
> > sel_make_perm_files() is separate -- I can fold it into the series or post it on
> > its own, whichever you prefer.
> >
> > Correct on CAP_MAC_ADMIN too: the precondition is the load_policy permission
> > plus DAC write to /sys/fs/selinux/load, not the capability. That line was below
> > the --- so it never entered the commit message.
> >
> > Thanks for the ack.
>
> Ok, thanks, will defer to Paul on whether to just take this patch or
> wait on a larger one.

Considering the privileged nature of policy loading I don't think this
is a "OMG EVERYTHING IS ON FIRE! situation, so I'd rather fix this
properly.

Bryam, it sounds like you have a fix ready (or close to ready) to
post, would you mind posting that as two patches in one patchset so we
can take a look?  If we all decide it's better as one patch, I can
always squash the two when merging.

Thanks.

-- 
paul-moore.com

Re: [PATCH] selinux: reject a permission value exceeding the class permission count

[Paul Moore]

On Fri, Jul 24, 2026 at 11:56 AM Stephen Smalley
<stephen.smalley.work@gmail.com> wrote:
> On Fri, Jul 24, 2026 at 11:48 AM Stephen Smalley
> <stephen.smalley.work@gmail.com> wrote:
> >
> > On Fri, Jul 24, 2026 at 10:47 AM Bryam Vargas via B4 Relay
> > <devnull+hexlabsecurity.proton.me@kernel.org> wrote:
> > >
> > > From: Bryam Vargas <hexlabsecurity@proton.me>
> > >
> > > perm_read() bounds a permission value by SEL_VEC_MAX but never by the
> > > owning class or common's nprim, which is taken verbatim from the policy
> > > image.  security_get_permissions() then writes perms[value - 1] into an
> > > nprim-sized kcalloc() array, so a class declaring fewer permissions than
> > > its largest permission value drives an out-of-bounds heap write.  The
> > > top-level symbol tables are validated this way; the nested per-class
> > > permission table is not.
> > >
> > > Reject a permission whose value exceeds nprim, which is already set when
> > > perm_read() runs.  Well-formed policies are unaffected.
> > >
> > > Fixes: 55fcf09b3fe4 ("selinux: add support for querying object classes and permissions from the running policy")
> > > Cc: stable@vger.kernel.org
> > > Signed-off-by: Bryam Vargas <hexlabsecurity@proton.me>
> >
> > Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com>
> > but also see:
> > https://lore.kernel.org/selinux/CAEjxPJ7YRgtOXrbPYar0qXAq=wNbOTTMS-EsU=9HKR1NQ3qA1g@mail.gmail.com/T/#me98ec932b314ae94242ed3af7ab74445d273cc7d
>
> Also wondering to what extent this may overlap with this series that
> was never fully merged:
> https://lore.kernel.org/selinux/20250511173055.406906-15-cgoettsche@seltendoof.de/

FWIW, most of that patchset is has been merged, the individual patches
that haven't been merged were held back due to review comments.

-- 
paul-moore.com