Re: [PATCH v4] selinux: remove unused initial SIDs and improve handling

On Thu, Oct 30, 2025 at 12:37 PM Stephen Smalley
<[email protected]> wrote:
>
> On Mon, Feb 24, 2020 at 8:55 PM Paul Moore <[email protected]> wrote:
> >
> > FWD'ing this to the mailing list while Stephen is having problems posting.
> >
> > ---------- Forwarded message ---------
> > From: Stephen Smalley <[email protected]>
> > Date: Mon, Feb 24, 2020 at 11:09 AM
> > Subject: [PATCH v4] selinux: remove unused initial SIDs and improve handling
> > To: <[email protected]>
> > Cc: <[email protected]>, <[email protected]>, <[email protected]>,
> > Stephen Smalley <[email protected]>
> >
> >
> > Remove initial SIDs that have never been used or are no longer used by
> > the kernel from its string table, which is also used to generate the
> > SECINITSID_* symbols referenced in code.  Update the code to
> > gracefully handle the fact that these can now be NULL. Stop treating
> > it as an error if a policy defines additional initial SIDs unknown to
> > the kernel.  Do not load unused initial SID contexts into the sidtab.
> > Fix the incorrect usage of the name from the ocontext in error
> > messages when loading initial SIDs since these are not presently
> > written to the kernel policy and are therefore always NULL.
> >
> > After this change, it is possible to safely reclaim and reuse some of
> > the unused initial SIDs without compatibility issues.  Specifically,
> > unused initial SIDs that were being assigned the same context as the
> > unlabeled initial SID in policies can be reclaimed and reused for
> > another purpose, with existing policies still treating them as having
> > the unlabeled context and future policies having the option of mapping
> > them to a more specific context.  For example, this could have been
> > used when the infiniband labeling support was introduced to define
> > initial SIDs for the default pkey and endport SIDs similar to the
> > handling of port/netif/node SIDs rather than always using
> > SECINITSID_UNLABELED as the default.
> >
> > The set of safely reclaimable unused initial SIDs across all known
> > policies is igmp_packet (13), icmp_socket (14), tcp_socket (15), kmod
> > (24), policy (25), and scmp_packet (26); these initial SIDs were
> > assigned the same context as unlabeled in all known policies including
> > mls.  If only considering non-mls policies (i.e. assuming that mls
> > users always upgrade policy with their kernels), the set of safely
> > reclaimable unused initial SIDs further includes file_labels (6), init
> > (7), sysctl_modprobe (16), and sysctl_fs (18) through sysctl_dev (23).
> >
> > Adding new initial SIDs beyond SECINITSID_NUM to policy unfortunately
> > became a fatal error in commit 24ed7fdae669 ("selinux: use separate
> > table for initial SID lookup") and even before that it could cause
> > problems on a policy reload (collision between the new initial SID and
> > one allocated at runtime) ever since commit 42596eafdd75 ("selinux:
> > load the initial SIDs upon every policy load") so we cannot safely
> > start adding new initial SIDs to policies beyond SECINITSID_NUM (27)
> > until such a time as all such kernels do not need to be supported and
> > only those that include this commit are relevant. That is not a big
> > deal since we haven't added a new initial SID since 2004 (v2.6.7) and
> > we have plenty of unused ones we can reclaim if we truly need one.
> >
> > If we want to avoid the wasted storage in initial_sid_to_string[]
> > and/or sidtab->isids[] for the unused initial SIDs, we could introduce
> > an indirection between the kernel initial SID values and the policy
> > initial SID values and just map the policy SID values in the ocontexts
> > to the kernel values during policy_load_isids(). Originally I thought
> > we'd do this by preserving the initial SID names in the kernel policy
> > and creating a mapping at load time like we do for the security
> > classes and permissions but that would require a new kernel policy
> > format version and associated changes to libsepol/checkpolicy and I'm
> > not sure it is justified. Simpler approach is just to create a fixed
> > mapping table in the kernel from the existing fixed policy values to
> > the kernel values. Less flexible but probably sufficient.
> >
> > A separate selinux userspace change was applied in
> > https://github.com/SELinuxProject/selinux/commit/8677ce5e8f592950ae6f14cea1b68a20ddc1ac25
> > to enable removal of most of the unused initial SID contexts from
> > policies, but there is no dependency between that change and this one.
> > That change permits removing all of the unused initial SID contexts
> > from policy except for the fs and sysctl SID contexts.  The initial
> > SID declarations themselves would remain in policy to preserve the
> > values of subsequent ones but the contexts can be dropped.  If/when
> > the kernel decides to reuse one of them, future policies can change
> > the name and start assigning a context again without breaking
> > compatibility.
> >
> > Here is how I would envision staging changes to the initial SIDs in a
> > compatible manner after this commit is applied:
> >
> > 1. At any time after this commit is applied, the kernel could choose
> > to reclaim one of the safely reclaimable unused initial SIDs listed
> > above for a new purpose (i.e. replace its NULL entry in the
> > initial_sid_to_string[] table with a new name and start using the
> > newly generated SECINITSID_name symbol in code), and refpolicy could
> > at that time rename its declaration of that initial SID to reflect its
> > new purpose and start assigning it a context going
> > forward. Existing/old policies would map the reclaimed initial SID to
> > the unlabeled context, so that would be the initial default behavior
> > until policies are updated. This doesn't depend on the selinux
> > userspace change; it will work with existing policies and userspace.
> >
> > 2. In 6 months or so we'll have another SELinux userspace release that
> > will include the libsepol/checkpolicy support for omitting unused
> > initial SID contexts.
> >
> > 3. At any time after that release, refpolicy can make that release its
> > minimum build requirement and drop the sid context statements (but not
> > the sid declarations) for all of the unused initial SIDs except for
> > fs and sysctl, which must remain for compatibility on policy
> > reload with old kernels and for compatibility with kernels that were
> > still using SECINITSID_SYSCTL (< 2.6.39). This doesn't depend on this
> > kernel commit; it will work with previous kernels as well.
>
> This never happened AFAICT and I'm not sure if it was even tried.
> Might need/want to retain the sid context statement for the init SID
> too since it was re-activated by a later patch by Ondrej.

I posted a refpolicy patch to remove the unused initial SID contexts
that can be safely removed for any kernel, see:
https://lore.kernel.org/selinux-refpolicy/[email protected]/

This requires libsepol >= 3.1 but should work on any kernel even
without this kernel patch (which was merged for v5.7).

>
> > 4. After N years for some value of N, refpolicy decides that it no
> > longer cares about policy reload compatibility for kernels that
> > predate this kernel commit, and refpolicy drops the fs and sysctl
> > SID contexts from policy too (but retains the declarations).
>
> Checking to see if N == 5 here ;)
>
> > 5. After M years for some value of M, the kernel decides that it no
> > longer cares about compatibility with refpolicies that predate step 4
> > (dropping the fs and sysctl SIDs), and those two SIDs also become
> > safely reclaimable.  This step is optional and need not ever occur unless
> > we decide that the need to reclaim those two SIDs outweighs the
> > compatibility cost.
> >
> > 6. After O years for some value of O, refpolicy decides that it no
> > longer cares about policy load (not just reload) compatibility for
> > kernels that predate this kernel commit, and both kernel and refpolicy
> > can then start adding and using new initial SIDs beyond 27. This does
> > not depend on the previous change (step 5) and can occur independent
> > of it.
> >
> > Fixes: https://github.com/SELinuxProject/selinux-kernel/issues/12
> > Signed-off-by: Stephen Smalley <[email protected]>
> > ---
> > v4 fixes the commit hashes that I cut-and-pasted from the GH issue
> > comments to be the proper length and added the one-line descriptions.
> > Oddly checkpatch.pl didn't catch that originally.
> >
> >  scripts/selinux/genheaders/genheaders.c       | 11 +++-
> >  .../selinux/include/initial_sid_to_string.h   | 57 +++++++++----------
> >  security/selinux/selinuxfs.c                  |  6 +-
> >  security/selinux/ss/policydb.c                | 25 ++++----
> >  security/selinux/ss/services.c                | 26 ++++-----
> >  5 files changed, 66 insertions(+), 59 deletions(-)
> >
> > diff --git a/scripts/selinux/genheaders/genheaders.c
> > b/scripts/selinux/genheaders/genheaders.c
> > index 544ca126a8a8..f355b3e0e968 100644
> > --- a/scripts/selinux/genheaders/genheaders.c
> > +++ b/scripts/selinux/genheaders/genheaders.c
> > @@ -67,8 +67,12 @@ int main(int argc, char *argv[])
> >         }
> >
> >         isids_len = sizeof(initial_sid_to_string) / sizeof (char *);
> > -       for (i = 1; i < isids_len; i++)
> > -               initial_sid_to_string[i] = stoupperx(initial_sid_to_string[i]);
> > +       for (i = 1; i < isids_len; i++) {
> > +               const char *s = initial_sid_to_string[i];
> > +
> > +               if (s)
> > +                       initial_sid_to_string[i] = stoupperx(s);
> > +       }
> >
> >         fprintf(fout, "/* This file is automatically generated.  Do
> > not edit. */\n");
> >         fprintf(fout, "#ifndef _SELINUX_FLASK_H_\n#define
> > _SELINUX_FLASK_H_\n\n");
> > @@ -82,7 +86,8 @@ int main(int argc, char *argv[])
> >
> >         for (i = 1; i < isids_len; i++) {
> >                 const char *s = initial_sid_to_string[i];
> > -               fprintf(fout, "#define SECINITSID_%-39s %2d\n", s, i);
> > +               if (s)
> > +                       fprintf(fout, "#define SECINITSID_%-39s %2d\n", s, i);
> >         }
> >         fprintf(fout, "\n#define SECINITSID_NUM %d\n", i-1);
> >         fprintf(fout, "\nstatic inline bool
> > security_is_socket_class(u16 kern_tclass)\n");
> > diff --git a/security/selinux/include/initial_sid_to_string.h
> > b/security/selinux/include/initial_sid_to_string.h
> > index 4f93f697f71c..5d332aeb8b6c 100644
> > --- a/security/selinux/include/initial_sid_to_string.h
> > +++ b/security/selinux/include/initial_sid_to_string.h
> > @@ -1,34 +1,33 @@
> >  /* SPDX-License-Identifier: GPL-2.0 */
> > -/* This file is automatically generated.  Do not edit. */
> >  static const char *initial_sid_to_string[] =
> >  {
> > -    "null",
> > -    "kernel",
> > -    "security",
> > -    "unlabeled",
> > -    "fs",
> > -    "file",
> > -    "file_labels",
> > -    "init",
> > -    "any_socket",
> > -    "port",
> > -    "netif",
> > -    "netmsg",
> > -    "node",
> > -    "igmp_packet",
> > -    "icmp_socket",
> > -    "tcp_socket",
> > -    "sysctl_modprobe",
> > -    "sysctl",
> > -    "sysctl_fs",
> > -    "sysctl_kernel",
> > -    "sysctl_net",
> > -    "sysctl_net_unix",
> > -    "sysctl_vm",
> > -    "sysctl_dev",
> > -    "kmod",
> > -    "policy",
> > -    "scmp_packet",
> > -    "devnull",
> > +       NULL,
> > +       "kernel",
> > +       "security",
> > +       "unlabeled",
> > +       NULL,
> > +       "file",
> > +       NULL,
> > +       NULL,
> > +       "any_socket",
> > +       "port",
> > +       "netif",
> > +       "netmsg",
> > +       "node",
> > +       NULL,
> > +       NULL,
> > +       NULL,
> > +       NULL,
> > +       NULL,
> > +       NULL,
> > +       NULL,
> > +       NULL,
> > +       NULL,
> > +       NULL,
> > +       NULL,
> > +       NULL,
> > +       NULL,
> > +       NULL,
> > +       "devnull",
> >  };
> >
> > diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c
> > index 533ab170ad52..4781314c2510 100644
> > --- a/security/selinux/selinuxfs.c
> > +++ b/security/selinux/selinuxfs.c
> > @@ -1701,7 +1701,11 @@ static int sel_make_initcon_files(struct dentry *dir)
> >         for (i = 1; i <= SECINITSID_NUM; i++) {
> >                 struct inode *inode;
> >                 struct dentry *dentry;
> > -               dentry = d_alloc_name(dir, security_get_initial_sid_context(i));
> > +               const char *s = security_get_initial_sid_context(i);
> > +
> > +               if (!s)
> > +                       continue;
> > +               dentry = d_alloc_name(dir, s);
> >                 if (!dentry)
> >                         return -ENOMEM;
> >
> > diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c
> > index 32b3a8acf96f..406fb02d80ae 100644
> > --- a/security/selinux/ss/policydb.c
> > +++ b/security/selinux/ss/policydb.c
> > @@ -867,29 +867,28 @@ int policydb_load_isids(struct policydb *p,
> > struct sidtab *s)
> >
> >         head = p->ocontexts[OCON_ISID];
> >         for (c = head; c; c = c->next) {
> > -               rc = -EINVAL;
> > -               if (!c->context[0].user) {
> > -                       pr_err("SELinux:  SID %s was never defined.\n",
> > -                               c->u.name);
> > -                       sidtab_destroy(s);
> > -                       goto out;
> > -               }
> > -               if (c->sid[0] == SECSID_NULL || c->sid[0] > SECINITSID_NUM) {
> > -                       pr_err("SELinux:  Initial SID %s out of range.\n",
> > -                               c->u.name);
> > +               u32 sid = c->sid[0];
> > +               const char *name = security_get_initial_sid_context(sid);
> > +
> > +               if (sid == SECSID_NULL) {
> > +                       pr_err("SELinux:  SID 0 was assigned a context.\n");
> >                         sidtab_destroy(s);
> >                         goto out;
> >                 }
> > +
> > +               /* Ignore initial SIDs unused by this kernel. */
> > +               if (!name)
> > +                       continue;
> > +
> >                 rc = context_add_hash(p, &c->context[0]);
> >                 if (rc) {
> >                         sidtab_destroy(s);
> >                         goto out;
> >                 }
> > -
> > -               rc = sidtab_set_initial(s, c->sid[0], &c->context[0]);
> > +               rc = sidtab_set_initial(s, sid, &c->context[0]);
> >                 if (rc) {
> >                         pr_err("SELinux:  unable to load initial SID %s.\n",
> > -                               c->u.name);
> > +                              name);
> >                         sidtab_destroy(s);
> >                         goto out;
> >                 }
> > diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
> > index f90e6550eec8..8ad34fd031d1 100644
> > --- a/security/selinux/ss/services.c
> > +++ b/security/selinux/ss/services.c
> > @@ -1322,23 +1322,22 @@ static int security_sid_to_context_core(struct
> > selinux_state *state,
> >         if (!selinux_initialized(state)) {
> >                 if (sid <= SECINITSID_NUM) {
> >                         char *scontextp;
> > +                       const char *s = initial_sid_to_string[sid];
> >
> > -                       *scontext_len = strlen(initial_sid_to_string[sid]) + 1;
> > +                       if (!s)
> > +                               return -EINVAL;
> > +                       *scontext_len = strlen(s) + 1;
> >                         if (!scontext)
> > -                               goto out;
> > -                       scontextp = kmemdup(initial_sid_to_string[sid],
> > -                                           *scontext_len, GFP_ATOMIC);
> > -                       if (!scontextp) {
> > -                               rc = -ENOMEM;
> > -                               goto out;
> > -                       }
> > +                               return 0;
> > +                       scontextp = kmemdup(s, *scontext_len, GFP_ATOMIC);
> > +                       if (!scontextp)
> > +                               return -ENOMEM;
> >                         *scontext = scontextp;
> > -                       goto out;
> > +                       return 0;
> >                 }
> >                 pr_err("SELinux: %s:  called before initial "
> >                        "load_policy on unknown SID %d\n", __func__, sid);
> > -               rc = -EINVAL;
> > -               goto out;
> > +               return -EINVAL;
> >         }
> >         read_lock(&state->ss->policy_rwlock);
> >         policydb = &state->ss->policydb;
> > @@ -1362,7 +1361,6 @@ static int security_sid_to_context_core(struct
> > selinux_state *state,
> >
> >  out_unlock:
> >         read_unlock(&state->ss->policy_rwlock);
> > -out:
> >         return rc;
> >
> >  }
> > @@ -1552,7 +1550,9 @@ static int security_context_to_sid_core(struct
> > selinux_state *state,
> >                 int i;
> >
> >                 for (i = 1; i < SECINITSID_NUM; i++) {
> > -                       if (!strcmp(initial_sid_to_string[i], scontext2)) {
> > +                       const char *s = initial_sid_to_string[i];
> > +
> > +                       if (s && !strcmp(s, scontext2)) {
> >                                 *sid = i;
> >                                 goto out;
> >                         }
> > --
> > 2.24.1
> >
> >
> >
> > --
> > paul moore
> > www.paul-moore.com