diff mbox series

[v2] libselinux/utils: drop reachable assert in sefcontext_compile

Message ID 20241203155448.48237-1-cgoettsche@seltendoof.de (mailing list archive)
State New
Delegated to: Petr Lautrbach
Headers show
Series [v2] libselinux/utils: drop reachable assert in sefcontext_compile | expand

Commit Message

Christian Göttsche Dec. 3, 2024, 3:54 p.m. UTC 
From: Christian Göttsche <cgzones@googlemail.com>

The two asserts following qsort(3) where useful during development to
ensure the comparison function and the corresponding pointer handling
were correct.  They however do not take into account an empty file
context definition file containing no definitions and thus `stab->nel`
being NULL.  Drop the two asserts.

Also return early to not depend on whether calloc(3) called with a size
of zero returns NULL or a special value.

Reported-by: Petr Lautrbach <lautrbach@redhat.com>
Closes: https://lore.kernel.org/selinux/87jzchqck5.fsf@redhat.com/
Fixes: 92306daf ("libselinux: rework selabel_file(5) database")
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
v2: fix condition from not zero to equal to zero
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
 libselinux/utils/sefcontext_compile.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Comments

Petr Lautrbach Dec. 3, 2024, 4:21 p.m. UTC | #1 
Christian Göttsche <cgoettsche@seltendoof.de> writes:

> From: Christian Göttsche <cgzones@googlemail.com>
>
> The two asserts following qsort(3) where useful during development to
> ensure the comparison function and the corresponding pointer handling
> were correct.  They however do not take into account an empty file
> context definition file containing no definitions and thus `stab->nel`
> being NULL.  Drop the two asserts.
>
> Also return early to not depend on whether calloc(3) called with a size
> of zero returns NULL or a special value.
>
> Reported-by: Petr Lautrbach <lautrbach@redhat.com>
> Closes: https://lore.kernel.org/selinux/87jzchqck5.fsf@redhat.com/
> Fixes: 92306daf ("libselinux: rework selabel_file(5) database")
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>

Works for me. Thanks!

Tested-by: Petr Lautrbach <lautrbach@redhat.com>

> ---
> v2: fix condition from not zero to equal to zero
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> ---
>  libselinux/utils/sefcontext_compile.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/libselinux/utils/sefcontext_compile.c b/libselinux/utils/sefcontext_compile.c
> index 23d31274..e5da51ea 100644
> --- a/libselinux/utils/sefcontext_compile.c
> +++ b/libselinux/utils/sefcontext_compile.c
> @@ -188,6 +188,9 @@ static int write_sidtab(FILE *bin_file, const struct sidtab *stab)
>  	if (len != 1)
>  		return -1;
>  
> +	if (stab->nel == 0)
> +		return 0;
> +
>  	/* sort entries by id */
>  	sids = calloc(stab->nel, sizeof(*sids));
>  	if (!sids)
> @@ -203,8 +206,6 @@ static int write_sidtab(FILE *bin_file, const struct sidtab *stab)
>  	}
>  	assert(index == stab->nel);
>  	qsort(sids, stab->nel, sizeof(struct security_id), security_id_compare);
> -	assert(sids[0].id == 1);
> -	assert(sids[stab->nel - 1].id == stab->nel);
>  
>  	/* write raw contexts sorted by id */
>  	for (uint32_t i = 0; i < stab->nel; i++) {
> -- 
> 2.45.2
James Carter Dec. 3, 2024, 10:01 p.m. UTC | #2 
On Tue, Dec 3, 2024 at 11:24 AM Petr Lautrbach <lautrbach@redhat.com> wrote:
>
> Christian Göttsche <cgoettsche@seltendoof.de> writes:
>
> > From: Christian Göttsche <cgzones@googlemail.com>
> >
> > The two asserts following qsort(3) where useful during development to
> > ensure the comparison function and the corresponding pointer handling
> > were correct.  They however do not take into account an empty file
> > context definition file containing no definitions and thus `stab->nel`
> > being NULL.  Drop the two asserts.
> >
> > Also return early to not depend on whether calloc(3) called with a size
> > of zero returns NULL or a special value.
> >
> > Reported-by: Petr Lautrbach <lautrbach@redhat.com>
> > Closes: https://lore.kernel.org/selinux/87jzchqck5.fsf@redhat.com/
> > Fixes: 92306daf ("libselinux: rework selabel_file(5) database")
> > Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
>
> Works for me. Thanks!
>
> Tested-by: Petr Lautrbach <lautrbach@redhat.com>
>

Acked-by: James Carter <jwcart2@gmail.com>

> > ---
> > v2: fix condition from not zero to equal to zero
> > Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> > ---
> >  libselinux/utils/sefcontext_compile.c | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/libselinux/utils/sefcontext_compile.c b/libselinux/utils/sefcontext_compile.c
> > index 23d31274..e5da51ea 100644
> > --- a/libselinux/utils/sefcontext_compile.c
> > +++ b/libselinux/utils/sefcontext_compile.c
> > @@ -188,6 +188,9 @@ static int write_sidtab(FILE *bin_file, const struct sidtab *stab)
> >       if (len != 1)
> >               return -1;
> >
> > +     if (stab->nel == 0)
> > +             return 0;
> > +
> >       /* sort entries by id */
> >       sids = calloc(stab->nel, sizeof(*sids));
> >       if (!sids)
> > @@ -203,8 +206,6 @@ static int write_sidtab(FILE *bin_file, const struct sidtab *stab)
> >       }
> >       assert(index == stab->nel);
> >       qsort(sids, stab->nel, sizeof(struct security_id), security_id_compare);
> > -     assert(sids[0].id == 1);
> > -     assert(sids[stab->nel - 1].id == stab->nel);
> >
> >       /* write raw contexts sorted by id */
> >       for (uint32_t i = 0; i < stab->nel; i++) {
> > --
> > 2.45.2
>
>
James Carter Dec. 4, 2024, 2:32 p.m. UTC | #3 
On Tue, Dec 3, 2024 at 5:01 PM James Carter <jwcart2@gmail.com> wrote:
>
> On Tue, Dec 3, 2024 at 11:24 AM Petr Lautrbach <lautrbach@redhat.com> wrote:
> >
> > Christian Göttsche <cgoettsche@seltendoof.de> writes:
> >
> > > From: Christian Göttsche <cgzones@googlemail.com>
> > >
> > > The two asserts following qsort(3) where useful during development to
> > > ensure the comparison function and the corresponding pointer handling
> > > were correct.  They however do not take into account an empty file
> > > context definition file containing no definitions and thus `stab->nel`
> > > being NULL.  Drop the two asserts.
> > >
> > > Also return early to not depend on whether calloc(3) called with a size
> > > of zero returns NULL or a special value.
> > >
> > > Reported-by: Petr Lautrbach <lautrbach@redhat.com>
> > > Closes: https://lore.kernel.org/selinux/87jzchqck5.fsf@redhat.com/
> > > Fixes: 92306daf ("libselinux: rework selabel_file(5) database")
> > > Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> >
> > Works for me. Thanks!
> >
> > Tested-by: Petr Lautrbach <lautrbach@redhat.com>
> >
>
> Acked-by: James Carter <jwcart2@gmail.com>
>

Merged.
Thanks,
Jim

> > > ---
> > > v2: fix condition from not zero to equal to zero
> > > Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> > > ---
> > >  libselinux/utils/sefcontext_compile.c | 5 +++--
> > >  1 file changed, 3 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/libselinux/utils/sefcontext_compile.c b/libselinux/utils/sefcontext_compile.c
> > > index 23d31274..e5da51ea 100644
> > > --- a/libselinux/utils/sefcontext_compile.c
> > > +++ b/libselinux/utils/sefcontext_compile.c
> > > @@ -188,6 +188,9 @@ static int write_sidtab(FILE *bin_file, const struct sidtab *stab)
> > >       if (len != 1)
> > >               return -1;
> > >
> > > +     if (stab->nel == 0)
> > > +             return 0;
> > > +
> > >       /* sort entries by id */
> > >       sids = calloc(stab->nel, sizeof(*sids));
> > >       if (!sids)
> > > @@ -203,8 +206,6 @@ static int write_sidtab(FILE *bin_file, const struct sidtab *stab)
> > >       }
> > >       assert(index == stab->nel);
> > >       qsort(sids, stab->nel, sizeof(struct security_id), security_id_compare);
> > > -     assert(sids[0].id == 1);
> > > -     assert(sids[stab->nel - 1].id == stab->nel);
> > >
> > >       /* write raw contexts sorted by id */
> > >       for (uint32_t i = 0; i < stab->nel; i++) {
> > > --
> > > 2.45.2
> >
> >
diff mbox series

Patch

diff --git a/libselinux/utils/sefcontext_compile.c b/libselinux/utils/sefcontext_compile.c
index 23d31274..e5da51ea 100644
--- a/libselinux/utils/sefcontext_compile.c
+++ b/libselinux/utils/sefcontext_compile.c
@@ -188,6 +188,9 @@  static int write_sidtab(FILE *bin_file, const struct sidtab *stab)
 	if (len != 1)
 		return -1;
 
+	if (stab->nel == 0)
+		return 0;
+
 	/* sort entries by id */
 	sids = calloc(stab->nel, sizeof(*sids));
 	if (!sids)
@@ -203,8 +206,6 @@  static int write_sidtab(FILE *bin_file, const struct sidtab *stab)
 	}
 	assert(index == stab->nel);
 	qsort(sids, stab->nel, sizeof(struct security_id), security_id_compare);
-	assert(sids[0].id == 1);
-	assert(sids[stab->nel - 1].id == stab->nel);
 
 	/* write raw contexts sorted by id */
 	for (uint32_t i = 0; i < stab->nel; i++) {