On Wed, Mar 30, 2022 at 11:09 AM Joshua Brindle
<joshua.brindle@xxxxxxxxxxxxxxx> wrote:
>
> On Wed, Mar 30, 2022 at 10:51 AM James Carter <jwcart2@xxxxxxxxx> wrote:
> >
> > On Wed, Mar 30, 2022 at 9:55 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.
> >
> > Many of the calls to mallocarray() should be replaced by calloc()
> > because the array is initialized to zero right after the mallocarray()
> > call. I guess all of the calls can be replaced if you don't mind
> > having the memory set to zero and then immediately setting the array
> > to different values.
> >
> > I will merge this patch and send another patch replacing mallocarray()
> > where appropriate.
> >
>
> Hrm, I meant something like:
>
> diff --git a/libsepol/src/private.h b/libsepol/src/private.h
> index a8cc1472..9a51fb34 100644
> --- a/libsepol/src/private.h
> +++ b/libsepol/src/private.h
> @@ -90,7 +90,7 @@ static inline void* mallocarray(size_t nmemb, size_t size) {
> return NULL;
> }
>
> - return malloc(nmemb * size);
> + return calloc(nmemb, size);
> }
>
> #ifndef HAVE_REALLOCARRAY
>
>
> Since mallocarray has additional length checking logic that you lose
> if you just switch from mallocarray to calloc.
Does it? The man page for calloc says that it will return an error if
nmemb * size will overflow.
Jim
