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

On Fri, Jul 24, 2026 at 1:11 PM Stephen Smalley
<[email protected]> wrote:
> On Fri, Jul 24, 2026 at 1:07 PM Bryam Vargas <[email protected]> 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/[email protected]/
> >
> > 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