Re: [PATCH] libsepol: Use calloc when initializing bool_val_to_struct array

[William Roberts <bill.c.roberts@xxxxxxxxx>]

On Wed, Mar 30, 2022 at 10:46 AM Joshua Brindle
<joshua.brindle@xxxxxxxxxxxxxxx> wrote:
>
> On Mon, Mar 14, 2022 at 2:24 PM James Carter <jwcart2@xxxxxxxxx> wrote:
> >
> > Use calloc() instead of mallocarray() so that everything is
> > initialized to zero to prevent the use of unitialized memory when
> > validating malformed binary policies.
> >
> > Found by oss-fuzz (#45493)
> >
> > Signed-off-by: James Carter <jwcart2@xxxxxxxxx>
> > ---
> >  libsepol/src/conditional.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/libsepol/src/conditional.c b/libsepol/src/conditional.c
> > index f78b38a2..a620451d 100644
> > --- a/libsepol/src/conditional.c
> > +++ b/libsepol/src/conditional.c
> > @@ -522,7 +522,7 @@ int cond_init_bool_indexes(policydb_t * p)
> >         if (p->bool_val_to_struct)
> >                 free(p->bool_val_to_struct);
> >         p->bool_val_to_struct = (cond_bool_datum_t **)
> > -           mallocarray(p->p_bools.nprim, sizeof(cond_bool_datum_t *));
> > +           calloc(p->p_bools.nprim, sizeof(cond_bool_datum_t *));
> >         if (!p->bool_val_to_struct)
> >                 return -1;
> >         return 0;
> > --
> > 2.34.1
>
> Why not change the mallocarray macro to use calloc? I see a number of
> mallocarray calls that should be audited if this approach is taken.

+1, it also gets rid of initialization code like this (note the loop
setting to NULL):
int sepol_sidtab_init(sidtab_t * s)
{
        int i;

        s->htable = mallocarray(SIDTAB_SIZE, sizeof(sidtab_ptr_t));
        if (!s->htable)
                return -ENOMEM;
        for (i = 0; i < SIDTAB_SIZE; i++)
                s->htable[i] = (sidtab_ptr_t) NULL;
        s->nel = 0;
        s->next_sid = 1;
        s->shutdown = 0;
        INIT_SIDTAB_LOCK(s);
        return 0;
}