Re: [PATCH] selinux: reject a permission value exceeding the class permission count
Stephen Smalley <[email protected]> Fri, 24 Jul 2026 11:48:47 -0400
| Newsgroups | org.kernel.vger.selinux,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <CAEjxPJ4_chAbtJa4obgN+QSjMnCwk9XQ-4bnyOFxgFoFxTsV9A@mail.gmail.com> |
On Fri, Jul 24, 2026 at 10:47 AM Bryam Vargas via B4 Relay <[email protected]> wrote: > > From: Bryam Vargas <[email protected]> > > 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: [email protected] > Signed-off-by: Bryam Vargas <[email protected]> Acked-by: Stephen Smalley <[email protected]> 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 <[email protected]> > >