* [PATCH] libsepol: drop broken warning on duplicate filename transitions
@ 2020-05-13 21:15 Stephen Smalley
2020-05-14 8:19 ` Ondrej Mosnacek
0 siblings, 1 reply; 5+ messages in thread
From: Stephen Smalley @ 2020-05-13 21:15 UTC (permalink / raw)
To: selinux; +Cc: Stephen Smalley
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]);
free(ft);
free(name);
free(otype);
--
2.23.3
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] libsepol: drop broken warning on duplicate filename transitions
2020-05-13 21:15 [PATCH] libsepol: drop broken warning on duplicate filename transitions Stephen Smalley
@ 2020-05-14 8:19 ` Ondrej Mosnacek
2020-05-14 12:02 ` Stephen Smalley
0 siblings, 1 reply; 5+ messages in thread
From: Ondrej Mosnacek @ 2020-05-14 8:19 UTC (permalink / raw)
To: Stephen Smalley; +Cc: SElinux list
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
> free(ft);
> free(name);
> free(otype);
> --
> 2.23.3
>
--
Ondrej Mosnacek <omosnace at redhat dot com>
Software Engineer, Security Technologies
Red Hat, Inc.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] libsepol: drop broken warning on duplicate filename transitions
2020-05-14 8:19 ` Ondrej Mosnacek
@ 2020-05-14 12:02 ` Stephen Smalley
2020-05-14 12:19 ` Ondrej Mosnacek
0 siblings, 1 reply; 5+ messages in thread
From: Stephen Smalley @ 2020-05-14 12:02 UTC (permalink / raw)
To: Ondrej Mosnacek; +Cc: SElinux list
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).
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] libsepol: drop broken warning on duplicate filename transitions
2020-05-14 12:02 ` Stephen Smalley
@ 2020-05-14 12:19 ` Ondrej Mosnacek
2020-05-15 13:37 ` Petr Lautrbach
0 siblings, 1 reply; 5+ messages in thread
From: Ondrej Mosnacek @ 2020-05-14 12:19 UTC (permalink / raw)
To: Stephen Smalley; +Cc: SElinux list
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>
--
Ondrej Mosnacek <omosnace at redhat dot com>
Software Engineer, Security Technologies
Red Hat, Inc.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] libsepol: drop broken warning on duplicate filename transitions
2020-05-14 12:19 ` Ondrej Mosnacek
@ 2020-05-15 13:37 ` Petr Lautrbach
0 siblings, 0 replies; 5+ messages in thread
From: Petr Lautrbach @ 2020-05-15 13:37 UTC (permalink / raw)
To: SElinux list; +Cc: Stephen Smalley, Ondrej Mosnacek
[-- Attachment #1: Type: text/plain, Size: 3208 bytes --]
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.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-05-15 13:38 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-13 21:15 [PATCH] libsepol: drop broken warning on duplicate filename transitions Stephen Smalley
2020-05-14 8:19 ` Ondrej Mosnacek
2020-05-14 12:02 ` Stephen Smalley
2020-05-14 12:19 ` Ondrej Mosnacek
2020-05-15 13:37 ` Petr Lautrbach
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.