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

On Tue, Jul 28, 2026 at 1:42=E2=80=AFPM Bryam Vargas <hexlabsecurity@proton=
.me> wrote:
> On Tue, 28 Jul 2026 10:43:39 -0400, Stephen Smalley wrote:
> > Sashiko again made some interesting observations but not about your pat=
ch per se,
> > https://lore.kernel.org/selinux/[email protected]=
.org/T/#u
>
> The first one reproduces. security_get_permissions() sizes the array by t=
he
> class's permissions.nprim and fills it by value, so any index no perm dat=
um
> claims stays NULL, and sel_make_perm_files() passes every entry to
> sel_attach_file() -> d_alloc_name() -> hashlen_string(). One u32 in the p=
olicy
> image is enough: on the policy I tested, class process has nel =3D=3D npr=
im =3D=3D 31
> and inherits no common, so raising its nprim to 39 leaves slots 31..38
> unwritten.
>
>   BUG: kernel NULL pointer dereference, address: 0000000000000000
>   RIP: 0010:hashlen_string+0xa/0xc0
>   Call Trace:
>    d_alloc_name+0x61/0xa0
>    sel_make_policy_nodes+0xb35/0x1310
>    sel_write_load+0x2a5/0x490
>
> load_unaligned_zeropad() doesn't absorb it: ex_handler_zeropad() rounds t=
he
> address down and bails unless fault_addr =3D=3D addr + sizeof(long), so a=
 NULL
> base takes the ordinary fault.
>
> It doesn't stop at the loading task. sel_make_policy_nodes() runs under
> selinux_state.policy_mutex and the only unlock is the out_unlock label th=
e
> oops never reaches, so every later load blocks in D state; SIGKILL doesn'=
t
> clear it and only a reboot recovers. Under panic_on_oops it is a panic at
> step one.
>
> The second observation is real as stated: get_permissions_callback() does
> overwrite perms[value - 1], and the cleanup loop walks i < nperms freeing
> whichever pointer survived, so the first kstrdup() leaks. What makes it w=
orth
> more than its severity is that it also defeats the obvious fix for the fi=
rst
> one. Requiring the counts to add up -- nel plus the inherited nprim equal=
s
> permissions.nprim -- doesn't close the gap, because symtab_insert() hashe=
s
> and compares the name only, so two perms may carry the same value: the co=
unt
> still matches and one slot is still never written. The property that hold=
s is
> that the values present are exactly {1..nprim}, onto and injective, where=
 the
> two patches on the list establish only "into".
>
> The classes side has the same shape: get_classes_callback() also writes a=
t
> value - 1, and sel_make_classes() passes classes[i] to sel_make_dir(). By
> inspection; I have not reproduced that one.
>
> Policy crafter and the full log on request.

Thanks for looking into this, Bryam.  Are you interested in working on
a patchset to fix these issues?  We could always use the help :)

--=20
paul-moore.com