On Thu, May 14, 2020 at 02:19:05PM +0200, Ondrej Mosnacek wrote:
> On Thu, May 14, 2020 at 2:03 PM Stephen Smalley
> <stephen.smalley.work@gmail.com> wrote:
> > On Thu, May 14, 2020 at 4:19 AM Ondrej Mosnacek <omosnace@redhat.com> wrote:
> > >
> > > On Wed, May 13, 2020 at 11:16 PM Stephen Smalley
> > > <stephen.smalley.work@gmail.com> wrote:
> > > > As per the issue below, libsepol segfaults on loading old kernel policies
> > > > that contain duplicate filename transition rules.  The segfault is due to
> > > > the fact that the val_to_name arrays have not yet been populated at this
> > > > point in the policydb_read() processing.  Since this warning apparently
> > > > never worked since it was first introduced, drop it and just silently
> > > > discard the duplicate like the kernel does.  I was not able to produce a
> > > > policy with such duplicates using the current policy toolchain, either
> > > > via CIL or via binary modules with manual semodule_link/expand.
> > > >
> > > > Fixes: https://github.com/SELinuxProject/selinux/issues/239
> > > > Fixes: 8fdb2255215a1f14 ("libsepol,checkpolicy: convert rangetrans and filenametrans to hashtabs")
> > > > Signed-off-by: Stephen Smalley <stephen.smalley.work@gmail.com>
> > > > ---
> > > >  libsepol/src/policydb.c | 9 +--------
> > > >  1 file changed, 1 insertion(+), 8 deletions(-)
> > > >
> > > > diff --git a/libsepol/src/policydb.c b/libsepol/src/policydb.c
> > > > index 5b289a52..3992ea56 100644
> > > > --- a/libsepol/src/policydb.c
> > > > +++ b/libsepol/src/policydb.c
> > > > @@ -2655,15 +2655,8 @@ int filename_trans_read(policydb_t *p, struct policy_file *fp)
> > > >                          * Some old policies were wrongly generated with
> > > >                          * duplicate filename transition rules.  For backward
> > > >                          * compatibility, do not reject such policies, just
> > > > -                        * issue a warning and ignore the duplicate.
> > > > +                        * ignore the duplicate.
> > > >                          */
> > > > -                       WARN(fp->handle,
> > > > -                            "Duplicate name-based type_transition %s %s:%s \"%s\":  %s, ignoring",
> > > > -                            p->p_type_val_to_name[ft->stype - 1],
> > > > -                            p->p_type_val_to_name[ft->ttype - 1],
> > > > -                            p->p_class_val_to_name[ft->tclass - 1],
> > > > -                            ft->name,
> > > > -                            p->p_type_val_to_name[otype->otype - 1]);
> > >
> > > Not sure if it's the same situation, but should we also do something
> > > about a similar pattern in checkpolicy/policy_define.c?
> > >
> > > https://github.com/SELinuxProject/selinux/blob/63bf6afe5ed20e1d62f966de65882dc327fb2915/checkpolicy/policy_define.c#L3408
> >
> > No, in that case we are compiling source policy and we want to warn on
> > it to encourage removal of duplicates (and we have populated the
> > val_to_name arrays there so the warning works).
> 
> Ok, makes sense. In that case:
> 
> Acked-by: Ondrej Mosnacek <omosnace@redhat.com>
> 

Applied.