On Sun, Mar 14, 2021 at 4:22 PM Nicolas Iooss <nicolas.iooss@xxxxxxx> wrote: > > OSS-Fuzz found a memory leak when trying to compile the following > policy: > > (class CLASS (PERM ioctl)) > (classorder (CLASS)) > (sid SID) > (sidorder (SID)) > (user USER) > (role ROLE) > (type TYPE) > (category CAT) > (categoryorder (CAT)) > (sensitivity SENS) > (sensitivityorder (SENS)) > (sensitivitycategory SENS (CAT)) > (allow TYPE self (CLASS (PERM))) > (roletype ROLE TYPE) > (userrole USER ROLE) > (userlevel USER (SENS)) > (userrange USER ((SENS)(SENS (CAT)))) > (sidcontext SID (USER ROLE TYPE ((SENS)(SENS)))) > > (permissionx ioctl_test (ioctl CLASS (and (range 0x1600 0x19FF) (not (range 0x1750 0x175F))))) > (allowx TYPE TYPE ioctl_test) > > (boolean BOOLEAN false) > > (booleanif (not (xor (eq BOOLEAN BOOLEAN) > (and (not (xor (eq BOOLEAN BOOLEAN) > (and (not (xor (eq BOOLEAN BOOLEAN) > (and (not (xor (eq BOOLEAN BOOLEAN) > (and (not (xor (eq BOOLEAN BOOLEAN) > (and (not (xor (eq BOOLEAN BOOLEAN) > (and (not (xor (eq BOOLEAN BOOLEAN) > (and (not (xor (eq BOOLEAN BOOLEAN) > (and (not (xor (eq BOOLEAN BOOLEAN) > (and (not (xor (eq BOOLEAN BOOLEAN) > (and (not (xor (eq BOOLEAN BOOLEAN) > (and (not (xor (eq BOOLEAN BOOLEAN) > (and (not (xor (eq BOOLEAN BOOLEAN) > (and (not (xor (eq BOOLEAN BOOLEAN) > (and (not (xor (eq BOOLEAN BOOLEAN) > (and (not (xor (eq BOOLEAN BOOLEAN) > (and (not (xor (eq BOOLEAN BOOLEAN) > (and (not (xor (eq BOOLEAN BOOLEAN) > (and (not (xor (eq BOOLEAN BOOLEAN) > (and (not (xor (eq BOOLEAN BOOLEAN) > (and (not (xor (eq BOOLEAN BOOLEAN) > (and (not (xor (eq BOOLEAN BOOLEAN) > (and (not (xor (eq BOOLEAN BOOLEAN) > (and (not (xor (eq BOOLEAN BOOLEAN) > (and (not (xor (eq BOOLEAN BOOLEAN) > (and (not (xor (eq BOOLEAN BOOLEAN) > (and (not (xor (eq BOOLEAN BOOLEAN) > (and (not (xor (eq BOOLEAN BOOLEAN) > (and (not (xor (eq BOOLEAN BOOLEAN) > (and (not (xor (eq BOOLEAN BOOLEAN) > (and (not (xor (eq BOOLEAN BOOLEAN) > (and (not (xor (eq BOOLEAN BOOLEAN) > (and (not (xor (eq BOOLEAN BOOLEAN) > (and (not (xor (eq BOOLEAN BOOLEAN) > (and (not (xor (eq BOOLEAN BOOLEAN) > (and (not (xor (eq BOOLEAN BOOLEAN) > (and (not (xor (eq BOOLEAN BOOLEAN) > (and (eq BOOLEAN BOOLEAN) BOOLEAN ) ) ) > BOOLEAN ) ) ) > BOOLEAN ) ) ) > BOOLEAN ) ) ) > BOOLEAN ) ) ) > BOOLEAN ) ) ) > BOOLEAN ) ) ) > BOOLEAN ) ) ) > BOOLEAN ) ) ) > BOOLEAN ) ) ) > BOOLEAN ) ) ) > BOOLEAN ) ) ) > BOOLEAN ) ) ) > BOOLEAN ) ) ) > BOOLEAN ) ) ) > BOOLEAN ) ) ) > BOOLEAN ) ) ) > BOOLEAN ) ) ) > BOOLEAN ) ) ) > BOOLEAN ) ) ) > BOOLEAN ) ) ) > BOOLEAN ) ) ) > BOOLEAN ) ) ) > BOOLEAN ) ) ) > BOOLEAN ) ) ) > BOOLEAN ) ) ) > BOOLEAN ) ) ) > BOOLEAN ) ) ) > BOOLEAN ) ) ) > BOOLEAN ) ) ) > BOOLEAN ) ) ) > BOOLEAN ) ) ) > BOOLEAN ) ) ) > BOOLEAN ) ) ) > BOOLEAN ) ) ) > BOOLEAN ) ) ) > BOOLEAN ) ) ) > (true > (allow TYPE TYPE (CLASS (PERM))) > ) > ) > > When the CIL compiler reports "Conditional expression exceeded max > allowable depth" because of the loooooong expression in the booleanif > statement, cil_binary_create_allocated_pdb returns without freeing the > memory which was allocated to store the keys and values of hash table > avrulex_ioctl_table. > > Fix this by moving the freeing logic to a dedicated destructor function > and calling it in the exit block of cil_binary_create_allocated_pdb. > > Fixes: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=28618 > Signed-off-by: Nicolas Iooss <nicolas.iooss@xxxxxxx> Acked-by: James Carter <jwcart2@xxxxxxxxx> > --- > libsepol/cil/src/cil_binary.c | 18 ++++++++++-------- > 1 file changed, 10 insertions(+), 8 deletions(-) > > diff --git a/libsepol/cil/src/cil_binary.c b/libsepol/cil/src/cil_binary.c > index f80d84679f85..18532aad9801 100644 > --- a/libsepol/cil/src/cil_binary.c > +++ b/libsepol/cil/src/cil_binary.c > @@ -1668,14 +1668,6 @@ exit: > } > cil_list_destroy(&xperms_list, CIL_FALSE); > } > - > - // hashtab_t does not have a way to free keys or datum since it doesn't > - // know what they are. We won't need the keys/datum after this function, so > - // clean them up here. > - free(avtab_key); > - ebitmap_destroy(datum); > - free(datum); > - > return rc; > } > > @@ -1885,6 +1877,15 @@ exit: > return rc; > } > > +static int __cil_avrulex_ioctl_destroy(hashtab_key_t k, hashtab_datum_t datum, __attribute__((unused)) void *args) > +{ > + free(k); > + ebitmap_destroy(datum); > + free(datum); > + > + return SEPOL_OK; > +} > + > int __cil_cond_to_policydb_helper(struct cil_tree_node *node, __attribute__((unused)) uint32_t *finished, void *extra_args) > { > int rc; > @@ -5037,6 +5038,7 @@ int cil_binary_create_allocated_pdb(const struct cil_db *db, sepol_policydb_t *p > > exit: > hashtab_destroy(role_trans_table); > + hashtab_map(avrulex_ioctl_table, __cil_avrulex_ioctl_destroy, NULL); > hashtab_destroy(avrulex_ioctl_table); > free(type_value_to_cil); > free(class_value_to_cil); > -- > 2.30.2 >
