On Wed, Jun 9, 2021 at 10:39 AM Petr Lautrbach <plautrba@xxxxxxxxxx> wrote:
>
> liwugang <liwugang@xxxxxxx> writes:
>
> > In the define_te_avtab_ioctl function:
> > 1. the parameter avrule_template has been copied to
> > new elements which added to avrule list through
> > the function avrule_cpy, so it should free avrule_template.
> > 2. And for rangelist, it does not free the allocated memory.
> >
> > The memory leak can by found by using memory sanitizer:
> > =================================================================
> > ==20021==ERROR: LeakSanitizer: detected memory leaks
> >
> > Direct leak of 10336 byte(s) in 76 object(s) allocated from:
> > #0 0x7f8f96d9cb50 in __interceptor_malloc (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xdeb50)
> > #1 0x55c2e9447fb3 in define_te_avtab_xperms_helper /mnt/sources/tools/selinux/checkpolicy/policy_define.c:2046
> > #2 0x55c2e944a6ca in define_te_avtab_extended_perms /mnt/sources/tools/selinux/checkpolicy/policy_define.c:2479
> > #3 0x55c2e943126b in yyparse /mnt/sources/tools/selinux/checkpolicy/policy_parse.y:494
> > #4 0x55c2e9440221 in read_source_policy /mnt/sources/tools/selinux/checkpolicy/parse_util.c:64
> > #5 0x55c2e945a3df in main /mnt/sources/tools/selinux/checkpolicy/checkpolicy.c:619
> > #6 0x7f8f968eeb96 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21b96)
> >
> > Direct leak of 240 byte(s) in 15 object(s) allocated from:
> > #0 0x7f8f96d9cb50 in __interceptor_malloc (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xdeb50)
> > #1 0x55c2e9446cd9 in avrule_sort_ioctls /mnt/sources/tools/selinux/checkpolicy/policy_define.c:1846
> > #2 0x55c2e9447d8f in avrule_ioctl_ranges /mnt/sources/tools/selinux/checkpolicy/policy_define.c:2020
> > #3 0x55c2e944a0de in define_te_avtab_ioctl /mnt/sources/tools/selinux/checkpolicy/policy_define.c:2409
> > #4 0x55c2e944a744 in define_te_avtab_extended_perms /mnt/sources/tools/selinux/checkpolicy/policy_define.c:2485
> > #5 0x55c2e94312bf in yyparse /mnt/sources/tools/selinux/checkpolicy/policy_parse.y:503
> > #6 0x55c2e9440221 in read_source_policy /mnt/sources/tools/selinux/checkpolicy/parse_util.c:64
> > #7 0x55c2e945a3df in main /mnt/sources/tools/selinux/checkpolicy/checkpolicy.c:619
> > #8 0x7f8f968eeb96 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21b96)
> >
> > Tested-By: Christian Gttsche <cgzones@xxxxxxxxxxxxxx>
> > Signed-off-by: liwugang <liwugang@xxxxxxx>
> > ---
> > checkpolicy/policy_define.c | 10 +++++++++-
> > 1 file changed, 9 insertions(+), 1 deletion(-)
> >
> > diff --git a/checkpolicy/policy_define.c b/checkpolicy/policy_define.c
> > index 16234f31..52105d3a 100644
> > --- a/checkpolicy/policy_define.c
> > +++ b/checkpolicy/policy_define.c
> > @@ -2400,7 +2400,7 @@ int avrule_cpy(avrule_t *dest, avrule_t *src)
> > int define_te_avtab_ioctl(avrule_t *avrule_template)
> > {
> > avrule_t *avrule;
> > - struct av_ioctl_range_list *rangelist;
> > + struct av_ioctl_range_list *rangelist, *r;
> > av_extended_perms_t *complete_driver, *partial_driver, *xperms;
> > unsigned int i;
> >
> > @@ -2458,6 +2458,12 @@ done:
> > if (partial_driver)
> > free(partial_driver);
> >
> > + while (rangelist != NULL) {
> > + r = rangelist;
> > + rangelist = rangelist->next;
> > + free(r);
> > + }
> > +
> > return 0;
> > }
> >
> > @@ -2484,6 +2490,8 @@ int define_te_avtab_extended_perms(int which)
> > free(id);
> > if (define_te_avtab_ioctl(avrule_template))
> > return -1;
> > + avrule_destroy(avrule_template);
> > + free(avrule_template);
>
> I still think that avrule_template should be free'd before return -1. If
> I'm wrong please explain
>
It looks to me like avrule_destroy() and free() needs to be called
after define_te_avtab_ioctl() regardless of whether or not there was
an error.
Jim
>
>
>
>
> > } else {
> > yyerror("only ioctl extended permissions are supported");
> > free(id);
> > --
> > 2.30.2
>
