[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>

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

[Bryam Vargas]

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

[sashiko-bot]

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

Pre-existing issues:
- [High] The patch prevents an out-of-bounds write for class-specific permissions but fails to address the identical vulnerability when a class inherits permissions from a common. If a malicious policy declares a common with a large `nprim` and a class that inherits it but specifies a small `nprim`, the common permissions will bypass the patch's check and map to out-of-bounds indices in `security_get_permissions()`.
- [High] NULL pointer dereference in selinuxfs when a policy contains gaps in permission or class values. If a policy defines a sparse set of values (e.g., `nprim` is 3, but only values 1 and 3 are populated), `security_get_permissions()` leaves gaps in its allocated string array as NULL pointers. When `sel_make_perm_files()` iterates through these arrays, it passes the NULL pointers to `sel_attach_file()`, causing a kernel panic.
--

--- Patch [1]: [PATCH] selinux: reject a permission value exceeding the class permission count ---
commit eb44c136ba9338c4aa49d6ec88bea5f639bd151b
Author: Bryam Vargas <hexlabsecurity@proton.me>

selinux: reject a permission value exceeding the class permission count

This commit adds a bounds check in perm_read() to ensure a permission value
does not exceed the nprim value of its owning class or common. It attempts
to prevent an out-of-bounds heap write when security_get_permissions()
allocates an array sized by nprim and populates it.

> diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c
> index ead504a639e36..6973b68d9782a 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;

[Severity: High]
This is a pre-existing issue, but does this patch fully prevent the
out-of-bounds write if a class inherits permissions from a common?

While this check bounds the permission value against its immediate
symbol table's nprim, it seems a malicious policy could declare a
common with a large nprim and a class that inherits it with a small
nprim.

The parser sets the class's nprim without validating it against the
inherited common:

security/selinux/ss/policydb.c:class_read() {
    ...
    cladatum->permissions.nprim = le32_to_cpu(buf[3]);
    ...
}

Then security_get_permissions() allocates an array based solely on the
class's nprim:

security/selinux/ss/services.c:security_get_permissions() {
    ...
    *nperms = match->permissions.nprim;
    *perms = kcalloc(*nperms, sizeof(**perms), GFP_ATOMIC);
    ...
    if (match->comdatum) {
        rc = hashtab_map(&match->comdatum->permissions.table,
                         get_permissions_callback, *perms);
    ...
}

Since the common permissions bypass this patch's symtab-specific bounds
check and are mapped into the undersized perms array, could this still
lead to a slab-out-of-bounds write in get_permissions_callback()?

[Severity: High]
This is a pre-existing issue, but could this sparse array allocation
lead to a null pointer dereference later on?

If a policy defines a sparse set of values (for instance, if nprim is 3
but only values 1 and 3 are populated), get_permissions_callback() maps
the present permissions:

security/selinux/ss/services.c:get_permissions_callback() {
    ...
    perms[value] = kstrdup(name, GFP_ATOMIC);
    ...
}

This leaves intermediate unpopulated indices as NULL in the array. When
sel_make_perm_files() iterates through this array:

security/selinux/selinuxfs.c:sel_make_perm_files() {
    ...
    for (i = 0; i < nperms; i++) {
        rc = sel_attach_file(dir, perms[i], inode);
    ...
}

It passes the NULL pointer to sel_attach_file(), which passes it to
d_alloc_name(), which requires a valid string for hashlen_string().

Can this cause a kernel panic when a process loads such a crafted policy?

>  
>  	rc = str_read(&key, GFP_KERNEL, fp, len);
>  	if (rc)

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260724-b4-disp-ec8ac9f6-v1-1-d76d51a69b9d@proton.me?part=1

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.