All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] checkpolicy: fix the leak memory when uses xperms
@ 2021-05-10 11:03 liwugang
  2021-05-22 15:41 ` Christian Göttsche
  2021-05-31  9:14 ` Petr Lautrbach
  0 siblings, 2 replies; 6+ messages in thread
From: liwugang @ 2021-05-10 11:03 UTC (permalink / raw)
  To: selinux; +Cc: liwugang

In the define_te_avtab_ioctl function:
1. the parameter avrule_template has been copies 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)

Signed-off-by: liwugang <liwugang@163.com>
---
 checkpolicy/policy_define.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/checkpolicy/policy_define.c b/checkpolicy/policy_define.c
index 16234f31..04064af2 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, *r2;
 	av_extended_perms_t *complete_driver, *partial_driver, *xperms;
 	unsigned int i;
 
@@ -2458,6 +2458,13 @@ done:
 	if (partial_driver)
 		free(partial_driver);
 
+	r = rangelist;
+	while (r != NULL) {
+		r2 = r;
+		r = r->next;
+		free(r2);
+	}
+
 	return 0;
 }
 
@@ -2484,6 +2491,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);
 	} else {
 		yyerror("only ioctl extended permissions are supported");
 		free(id);
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] checkpolicy: fix the leak memory when uses xperms
  2021-05-10 11:03 [PATCH] checkpolicy: fix the leak memory when uses xperms liwugang
@ 2021-05-22 15:41 ` Christian Göttsche
  2021-05-31  9:14 ` Petr Lautrbach
  1 sibling, 0 replies; 6+ messages in thread
From: Christian Göttsche @ 2021-05-22 15:41 UTC (permalink / raw)
  To: SElinux list; +Cc: liwugang

Am Mo., 10. Mai 2021 um 13:21 Uhr schrieb liwugang <liwugang@163.com>:
>
> In the define_te_avtab_ioctl function:
> 1. the parameter avrule_template has been copies 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)
>
> Signed-off-by: liwugang <liwugang@163.com>

I am able to reproduce this leaks and this patch fixes them.

Tested-By: Christian Göttsche <cgzones@googlemail.com>

> ---
>  checkpolicy/policy_define.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/checkpolicy/policy_define.c b/checkpolicy/policy_define.c
> index 16234f31..04064af2 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, *r2;
>         av_extended_perms_t *complete_driver, *partial_driver, *xperms;
>         unsigned int i;
>
> @@ -2458,6 +2458,13 @@ done:
>         if (partial_driver)
>                 free(partial_driver);
>
> +       r = rangelist;
> +       while (r != NULL) {
> +               r2 = r;
> +               r = r->next;
> +               free(r2);
> +       }
> +
>         return 0;
>  }
>
> @@ -2484,6 +2491,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);
>         } else {
>                 yyerror("only ioctl extended permissions are supported");
>                 free(id);
> --
> 2.17.1
>
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] checkpolicy: fix the leak memory when uses xperms
  2021-05-10 11:03 [PATCH] checkpolicy: fix the leak memory when uses xperms liwugang
  2021-05-22 15:41 ` Christian Göttsche
@ 2021-05-31  9:14 ` Petr Lautrbach
  2021-06-01 15:17   ` [PATCH v2] " liwugang
  1 sibling, 1 reply; 6+ messages in thread
From: Petr Lautrbach @ 2021-05-31  9:14 UTC (permalink / raw)
  To: liwugang, selinux; +Cc: liwugang

liwugang <liwugang@163.com> writes:

> In the define_te_avtab_ioctl function:
> 1. the parameter avrule_template has been copies to

typo? "copied" instead of "copies" ?

> 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)
>
> Signed-off-by: liwugang <liwugang@163.com>
> ---
>  checkpolicy/policy_define.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/checkpolicy/policy_define.c b/checkpolicy/policy_define.c
> index 16234f31..04064af2 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, *r2;
>  	av_extended_perms_t *complete_driver, *partial_driver, *xperms;
>  	unsigned int i;
>  
> @@ -2458,6 +2458,13 @@ done:
>  	if (partial_driver)
>  		free(partial_driver);
>  
> +	r = rangelist;
> +	while (r != NULL) {


Seems like you could loop using `rangelist` directly only with `r`
instead of `r2`


> +		r2 = r;
> +		r = r->next;
> +		free(r2);
> +	}
> +
>  	return 0;
>  }
>  
> @@ -2484,6 +2491,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);

avrule_template should be probably free()'d before `return -1`

>  	} else {
>  		yyerror("only ioctl extended permissions are supported");
>  		free(id);
> -- 
> 2.17.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v2] checkpolicy: fix the leak memory when uses xperms
  2021-05-31  9:14 ` Petr Lautrbach
@ 2021-06-01 15:17   ` liwugang
  2021-06-09 11:17     ` Petr Lautrbach
  0 siblings, 1 reply; 6+ messages in thread
From: liwugang @ 2021-06-01 15:17 UTC (permalink / raw)
  To: selinux; +Cc: plautrba, cgzones, liwugang

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 3317 bytes --]

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 Göttsche <cgzones@googlemail.com>
Signed-off-by: liwugang <liwugang@163.com>
---
 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);
 	} else {
 		yyerror("only ioctl extended permissions are supported");
 		free(id);
-- 
2.30.2



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] checkpolicy: fix the leak memory when uses xperms
  2021-06-01 15:17   ` [PATCH v2] " liwugang
@ 2021-06-09 11:17     ` Petr Lautrbach
  2021-06-09 20:38       ` James Carter
  0 siblings, 1 reply; 6+ messages in thread
From: Petr Lautrbach @ 2021-06-09 11:17 UTC (permalink / raw)
  To: liwugang, selinux; +Cc: cgzones, liwugang

liwugang <liwugang@163.com> 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@googlemail.com>
> Signed-off-by: liwugang <liwugang@163.com>
> ---
>  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





>  	} else {
>  		yyerror("only ioctl extended permissions are supported");
>  		free(id);
> -- 
> 2.30.2


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] checkpolicy: fix the leak memory when uses xperms
  2021-06-09 11:17     ` Petr Lautrbach
@ 2021-06-09 20:38       ` James Carter
  0 siblings, 0 replies; 6+ messages in thread
From: James Carter @ 2021-06-09 20:38 UTC (permalink / raw)
  To: Petr Lautrbach; +Cc: liwugang, SElinux list, Christian Göttsche

On Wed, Jun 9, 2021 at 10:39 AM Petr Lautrbach <plautrba@redhat.com> wrote:
>
> liwugang <liwugang@163.com> 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@googlemail.com>
> > Signed-off-by: liwugang <liwugang@163.com>
> > ---
> >  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
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-06-09 20:39 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-10 11:03 [PATCH] checkpolicy: fix the leak memory when uses xperms liwugang
2021-05-22 15:41 ` Christian Göttsche
2021-05-31  9:14 ` Petr Lautrbach
2021-06-01 15:17   ` [PATCH v2] " liwugang
2021-06-09 11:17     ` Petr Lautrbach
2021-06-09 20:38       ` James Carter

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.