On Fri, Jun 16, 2023 at 10:43 PM Paul Moore <paul@xxxxxxxxxxxxxx> wrote:
>
> On Mon, Jun 12, 2023 at 5:01 AM Ondrej Mosnacek <omosnace@xxxxxxxxxx> wrote:
...
> > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> > index 99ded60a6b911..dd410ceb178cb 100644
> > --- a/security/selinux/hooks.c
> > +++ b/security/selinux/hooks.c
> > @@ -2264,6 +2264,18 @@ static int selinux_bprm_creds_for_exec(struct linux_binprm *bprm)
> > new_tsec->keycreate_sid = 0;
> > new_tsec->sockcreate_sid = 0;
> >
> > + /*
> > + * Before policy is loaded, label any task outside kernel space
> > + * as SECINITSID_INIT, so that any userspace tasks surviving from
> > + * early boot end up with a label different from SECINITSID_KERNEL
> > + * (if the policy chooses to set SECINITSID_INIT != SECINITSID_KERNEL).
> > + */
> > + if (!selinux_initialized()) {
> > + new_tsec->sid = SECINITSID_INIT;
> > + new_tsec->exec_sid = 0; /* just in case */
>
> Style nit, I don't like placing trailing comments on the same line as
> code. Don't respin this patch just for this, but remember this for
> future submissions.
Ack.
> > + return 0;
> > + }
> > +
> > if (old_tsec->exec_sid) {
> > new_tsec->sid = old_tsec->exec_sid;
> > /* Reset exec SID on execve. */
>
> ...
>
> > diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c
> > index 97c0074f9312a..240e0fb1d57f9 100644
> > --- a/security/selinux/ss/policydb.c
> > +++ b/security/selinux/ss/policydb.c
> > @@ -863,6 +863,8 @@ void policydb_destroy(struct policydb *p)
> > int policydb_load_isids(struct policydb *p, struct sidtab *s)
> > {
> > struct ocontext *head, *c;
> > + bool secsid_init_supported = ebitmap_get_bit(&p->policycaps,
> > + POLICYDB_CAP_USERSPACE_INITIAL_CONTEXT);
>
> This is another "please don't respin for this", but if you have to
> respin for any reason can you change the variable name to
> "isid_init_supported" or something similar? The "secsid" portion of
> the name looks wrong to me.
It was supposed to be "secinitsid_init_supported" but I botched it :)
Though that name is very long, so if I were to change it, I would go
with your suggestion.
--
Ondrej Mosnacek
Senior Software Engineer, Linux Security - SELinux kernel
Red Hat, Inc.
