On Wed, Jan 29, 2020 at 5:42 PM Stephen Smalley <sds@xxxxxxxxxxxxx> wrote: > 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. > > This is a first step toward enabling future evolution of > initial SIDs. Further changes are required to both userspace > and the kernel to fully address > https://github.com/SELinuxProject/selinux-kernel/issues/12 > but this takes a small step toward that end. > > Fully decoupling the policy and kernel initial SID values will > require introducing a mapping between them and dyhamically Nit: s/dyhamically/dynamically/ > mapping them at load time. > > Signed-off-by: Stephen Smalley <sds@xxxxxxxxxxxxx> > --- > v2 avoids loading all unused initial SID contexts into the sidtab, > not just ones beyond SECINITSID_NUM. It also drops the unnecessary > check for an undefined context because all contexts in the OCON_ISID > list were already validated at load time via context_read_and_validate(). > > 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(-) <snip> > diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c > index 2aa7f2e1a8e7..768a9d4e0b86 100644 > --- a/security/selinux/ss/policydb.c > +++ b/security/selinux/ss/policydb.c > @@ -865,29 +865,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 null was assigned a context.\n"); > sidtab_destroy(s); > goto out; > } Your sentence "Stop treating it as an error if a policy defines additional initial SIDs unknown to the kernel." and the removed check for > SECINITSID_NUM suggest that you intend to not treat this condition as an error, but sidtab_set_initial() called bellow will reject such SID with -ENIVAL. Or am I misreading it and you just wanted to remove the duplicate check? > + > + /* 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; > } <snip> -- Ondrej Mosnacek <omosnace at redhat dot com> Software Engineer, Security Technologies Red Hat, Inc.
