On Sat, Jul 27, 2019 at 12:27 AM Paul Moore <paul@xxxxxxxxxxxxxx> wrote:
> On Thu, Jul 25, 2019 at 6:52 AM Ondrej Mosnacek <omosnace@xxxxxxxxxx> wrote:
> > Since roles_init() adds some entries to the role hash table, we need to
> > destroy also its keys/values on error, otherwise we get a memory leak in
> > the error path.
> >
> > Reported-by: syzbot+fee3a14d4cdf92646287@xxxxxxxxxxxxxxxxxxxxxxxxx
> > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> > Signed-off-by: Ondrej Mosnacek <omosnace@xxxxxxxxxx>
> > ---
> > security/selinux/ss/policydb.c | 6 +++++-
> > 1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c
> > index daecdfb15a9c..38d0083204f1 100644
> > --- a/security/selinux/ss/policydb.c
> > +++ b/security/selinux/ss/policydb.c
> > @@ -274,6 +274,8 @@ static int rangetr_cmp(struct hashtab *h, const void *k1, const void *k2)
> > return v;
> > }
> >
> > +static int (*destroy_f[SYM_NUM]) (void *key, void *datum, void *datap);
>
> I'm prefer not to use forward declarations if they can be avoided, and
> in this particular case it looks like we can move the *_destroy
> functions up just below policydb_lookup_compat() and avoid the forward
> declaration while keeping some sanity to the layout of the file.
>
> Yes, the patch does become much larger (378 lines changed in the test
> patch I just did), but I think the end result is cleaner.
OK, that'ts reasonable. I'll post an updated v2.
>
> > /*
> > * Initialize a policy database structure.
> > */
> > @@ -321,8 +323,10 @@ static int policydb_init(struct policydb *p)
> > out:
> > hashtab_destroy(p->filename_trans);
> > hashtab_destroy(p->range_tr);
> > - for (i = 0; i < SYM_NUM; i++)
> > + for (i = 0; i < SYM_NUM; i++) {
> > + hashtab_map(p->symtab[i].table, destroy_f[i], NULL);
> > hashtab_destroy(p->symtab[i].table);
> > + }
> > return rc;
> > }
> >
> > --
> > 2.21.0
> >
>
>
> --
> paul moore
> www.paul-moore.com
--
Ondrej Mosnacek <omosnace at redhat dot com>
Software Engineer, Security Technologies
Red Hat, Inc.
