On Mon, Jun 19, 2023 at 11:10 PM Paul Moore <paul@xxxxxxxxxxxxxx> wrote:
>
> On Sat, Jun 17, 2023 at 5:30 AM Ondrej Mosnacek <omosnace@xxxxxxxxxx> wrote:
> > 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.
>
> :)
>
> Since we've got a couple of weeks (-rc7 + merge window), why not go
> ahead and do the respin to fix up those small things and simplify the
> policycap accessor (see the other patch I posted) - does that sound
> reasonable?
Sure, will do.
--
Ondrej Mosnacek
Senior Software Engineer, Linux Security - SELinux kernel
Red Hat, Inc.
