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.
