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

On Tue, 28 Jul 2026 10:43:39 -0400, Stephen Smalley wrote:
> Sashiko again made some interesting observations but not about your patch per se,
> https://lore.kernel.org/selinux/[email protected]/T/#u

The first one reproduces. security_get_permissions() sizes the array by the
class's permissions.nprim and fills it by value, so any index no perm datum
claims stays NULL, and sel_make_perm_files() passes every entry to
sel_attach_file() -> d_alloc_name() -> hashlen_string(). One u32 in the policy
image is enough: on the policy I tested, class process has nel == nprim == 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 the
address down and bails unless fault_addr == 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 the
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 worth
more than its severity is that it also defeats the obvious fix for the first
one. Requiring the counts to add up -- nel plus the inherited nprim equals
permissions.nprim -- doesn't close the gap, because symtab_insert() hashes
and compares the name only, so two perms may carry the same value: the count
still matches and one slot is still never written. The property that holds 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 at
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 the ack.

Bryam