* [PATCH] libsepol: fix memory leak in expand.c
@ 2016-08-08 17:28 william.c.roberts
2016-08-08 17:29 ` Roberts, William C
0 siblings, 1 reply; 4+ messages in thread
From: william.c.roberts @ 2016-08-08 17:28 UTC (permalink / raw)
To: selinux, seandroid-list, sds
From: William Roberts <william.c.roberts@intel.com>
ebitmap_set_bit() can possible allocate nodes, however, the bail early
style of type_set_expand() could leave internal ebitmaps allocated
but not free'd.
Modify type_set_expand() so that it free's all allocated ebitmaps
before returning the error code to the calling routine.
Signed-off-by: William Roberts <william.c.roberts@intel.com>
---
libsepol/src/expand.c | 30 +++++++++++++++++-------------
1 file changed, 17 insertions(+), 13 deletions(-)
diff --git a/libsepol/src/expand.c b/libsepol/src/expand.c
index 4d3c623..0ad57f5 100644
--- a/libsepol/src/expand.c
+++ b/libsepol/src/expand.c
@@ -2497,6 +2497,7 @@ int type_set_expand(type_set_t * set, ebitmap_t * t, policydb_t * p,
unsigned int i;
ebitmap_t types, neg_types;
ebitmap_node_t *tnode;
+ int rc =-1;
ebitmap_init(&types);
ebitmap_init(t);
@@ -2511,7 +2512,7 @@ int type_set_expand(type_set_t * set, ebitmap_t * t, policydb_t * p,
* what's available in the type_val_to_struct mapping
*/
if (i > p->p_types.nprim - 1)
- return -1;
+ goto err_types;
if (p->type_val_to_struct[i]->flavor ==
TYPE_ATTRIB) {
@@ -2519,11 +2520,11 @@ int type_set_expand(type_set_t * set, ebitmap_t * t, policydb_t * p,
(&types,
&p->type_val_to_struct[i]->
types)) {
- return -1;
+ goto err_types;
}
} else {
if (ebitmap_set_bit(&types, i, 1)) {
- return -1;
+ goto err_types;
}
}
}
@@ -2531,7 +2532,7 @@ int type_set_expand(type_set_t * set, ebitmap_t * t, policydb_t * p,
} else {
/* No expansion of attributes, just copy the set as is. */
if (ebitmap_cpy(&types, &set->types))
- return -1;
+ goto err_types;
}
/* Now do the same thing for negset */
@@ -2543,11 +2544,11 @@ int type_set_expand(type_set_t * set, ebitmap_t * t, policydb_t * p,
if (ebitmap_union
(&neg_types,
&p->type_val_to_struct[i]->types)) {
- return -1;
+ goto err_neg;
}
} else {
if (ebitmap_set_bit(&neg_types, i, 1)) {
- return -1;
+ goto err_neg;
}
}
}
@@ -2562,7 +2563,7 @@ int type_set_expand(type_set_t * set, ebitmap_t * t, policydb_t * p,
p->type_val_to_struct[i]->flavor == TYPE_ATTRIB)
continue;
if (ebitmap_set_bit(t, i, 1))
- return -1;
+ goto err_neg;
}
goto out;
}
@@ -2571,7 +2572,7 @@ int type_set_expand(type_set_t * set, ebitmap_t * t, policydb_t * p,
if (ebitmap_node_get_bit(tnode, i)
&& (!ebitmap_get_bit(&neg_types, i)))
if (ebitmap_set_bit(t, i, 1))
- return -1;
+ goto err_neg;
}
if (set->flags & TYPE_COMP) {
@@ -2583,20 +2584,23 @@ int type_set_expand(type_set_t * set, ebitmap_t * t, policydb_t * p,
}
if (ebitmap_get_bit(t, i)) {
if (ebitmap_set_bit(t, i, 0))
- return -1;
+ goto err_neg;
} else {
if (ebitmap_set_bit(t, i, 1))
- return -1;
+ goto err_neg;
}
}
}
- out:
+ out:
+ rc = 0;
- ebitmap_destroy(&types);
+ err_neg:
ebitmap_destroy(&neg_types);
+ err_types:
+ ebitmap_destroy(&types);
- return 0;
+ return rc;
}
static int copy_neverallow(policydb_t * dest_pol, uint32_t * typemap,
--
1.9.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* RE: [PATCH] libsepol: fix memory leak in expand.c
2016-08-08 17:28 [PATCH] libsepol: fix memory leak in expand.c william.c.roberts
@ 2016-08-08 17:29 ` Roberts, William C
2016-08-09 16:12 ` Roberts, William C
0 siblings, 1 reply; 4+ messages in thread
From: Roberts, William C @ 2016-08-08 17:29 UTC (permalink / raw)
To: selinux, seandroid-list, sds
> -----Original Message-----
> From: Roberts, William C
> Sent: Monday, August 8, 2016 10:28 AM
> To: selinux@tycho.nsa.gov; seandroid-list@tycho.nsa.gov; sds@tycho.nsa.gov
> Cc: Roberts, William C <william.c.roberts@intel.com>
> Subject: [PATCH] libsepol: fix memory leak in expand.c
>
> From: William Roberts <william.c.roberts@intel.com>
>
> ebitmap_set_bit() can possible allocate nodes, however, the bail early style of
> type_set_expand() could leave internal ebitmaps allocated but not free'd.
>
> Modify type_set_expand() so that it free's all allocated ebitmaps before
> returning the error code to the calling routine.
>
> Signed-off-by: William Roberts <william.c.roberts@intel.com>
> ---
> libsepol/src/expand.c | 30 +++++++++++++++++-------------
> 1 file changed, 17 insertions(+), 13 deletions(-)
>
> diff --git a/libsepol/src/expand.c b/libsepol/src/expand.c index 4d3c623..0ad57f5
> 100644
> --- a/libsepol/src/expand.c
> +++ b/libsepol/src/expand.c
> @@ -2497,6 +2497,7 @@ int type_set_expand(type_set_t * set, ebitmap_t * t,
> policydb_t * p,
> unsigned int i;
> ebitmap_t types, neg_types;
> ebitmap_node_t *tnode;
> + int rc =-1;
>
> ebitmap_init(&types);
> ebitmap_init(t);
> @@ -2511,7 +2512,7 @@ int type_set_expand(type_set_t * set, ebitmap_t * t,
> policydb_t * p,
> * what's available in the type_val_to_struct
> mapping
> */
> if (i > p->p_types.nprim - 1)
> - return -1;
> + goto err_types;
>
> if (p->type_val_to_struct[i]->flavor ==
> TYPE_ATTRIB) {
> @@ -2519,11 +2520,11 @@ int type_set_expand(type_set_t * set, ebitmap_t *
> t, policydb_t * p,
> (&types,
> &p->type_val_to_struct[i]->
> types)) {
> - return -1;
> + goto err_types;
> }
> } else {
> if (ebitmap_set_bit(&types, i, 1)) {
> - return -1;
> + goto err_types;
> }
> }
> }
> @@ -2531,7 +2532,7 @@ int type_set_expand(type_set_t * set, ebitmap_t * t,
> policydb_t * p,
> } else {
> /* No expansion of attributes, just copy the set as is. */
> if (ebitmap_cpy(&types, &set->types))
> - return -1;
> + goto err_types;
> }
>
> /* Now do the same thing for negset */ @@ -2543,11 +2544,11 @@ int
> type_set_expand(type_set_t * set, ebitmap_t * t, policydb_t * p,
> if (ebitmap_union
> (&neg_types,
> &p->type_val_to_struct[i]->types)) {
> - return -1;
> + goto err_neg;
> }
> } else {
> if (ebitmap_set_bit(&neg_types, i, 1)) {
> - return -1;
> + goto err_neg;
> }
> }
> }
> @@ -2562,7 +2563,7 @@ int type_set_expand(type_set_t * set, ebitmap_t * t,
> policydb_t * p,
> p->type_val_to_struct[i]->flavor == TYPE_ATTRIB)
> continue;
> if (ebitmap_set_bit(t, i, 1))
> - return -1;
> + goto err_neg;
> }
> goto out;
> }
> @@ -2571,7 +2572,7 @@ int type_set_expand(type_set_t * set, ebitmap_t * t,
> policydb_t * p,
> if (ebitmap_node_get_bit(tnode, i)
> && (!ebitmap_get_bit(&neg_types, i)))
> if (ebitmap_set_bit(t, i, 1))
> - return -1;
> + goto err_neg;
> }
>
> if (set->flags & TYPE_COMP) {
> @@ -2583,20 +2584,23 @@ int type_set_expand(type_set_t * set, ebitmap_t *
> t, policydb_t * p,
> }
> if (ebitmap_get_bit(t, i)) {
> if (ebitmap_set_bit(t, i, 0))
> - return -1;
> + goto err_neg;
> } else {
> if (ebitmap_set_bit(t, i, 1))
> - return -1;
> + goto err_neg;
> }
> }
> }
>
> - out:
> + out:
> + rc = 0;
>
> - ebitmap_destroy(&types);
> + err_neg:
> ebitmap_destroy(&neg_types);
> + err_types:
> + ebitmap_destroy(&types);
>
> - return 0;
> + return rc;
> }
>
> static int copy_neverallow(policydb_t * dest_pol, uint32_t * typemap,
> --
> 1.9.1
Sorry for the disorganization in not sending these out as a series, I didn't see the memory leak, but this applies on-top of:
[PATCH] libsepol: fix invalid read when policy file is corrupt
Bill
^ permalink raw reply [flat|nested] 4+ messages in thread* RE: [PATCH] libsepol: fix memory leak in expand.c
2016-08-08 17:29 ` Roberts, William C
@ 2016-08-09 16:12 ` Roberts, William C
2016-08-09 20:26 ` James Carter
0 siblings, 1 reply; 4+ messages in thread
From: Roberts, William C @ 2016-08-09 16:12 UTC (permalink / raw)
To: 'selinux@tycho.nsa.gov',
'seandroid-list@tycho.nsa.gov',
'sds@tycho.nsa.gov'
<snip>
> >
> > - out:
> > + out:
> > + rc = 0;
> >
> > - ebitmap_destroy(&types);
> > + err_neg:
> > ebitmap_destroy(&neg_types);
> > + err_types:
> > + ebitmap_destroy(&types);
> >
> > - return 0;
> > + return rc;
> > }
I just noticed the indenting on the label changed, I went through the
File and it has:
1. labels with 7 leading spaces
2. labels left justified
Which one is the way you want them, left justified?
Bill
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] libsepol: fix memory leak in expand.c
2016-08-09 16:12 ` Roberts, William C
@ 2016-08-09 20:26 ` James Carter
0 siblings, 0 replies; 4+ messages in thread
From: James Carter @ 2016-08-09 20:26 UTC (permalink / raw)
To: Roberts, William C, 'selinux@tycho.nsa.gov',
'seandroid-list@tycho.nsa.gov',
'sds@tycho.nsa.gov'
On 08/09/2016 12:12 PM, Roberts, William C wrote:
> <snip>
>>>
>>> - out:
>>> + out:
>>> + rc = 0;
>>>
>>> - ebitmap_destroy(&types);
>>> + err_neg:
>>> ebitmap_destroy(&neg_types);
>>> + err_types:
>>> + ebitmap_destroy(&types);
>>>
>>> - return 0;
>>> + return rc;
>>> }
>
> I just noticed the indenting on the label changed, I went through the
> File and it has:
> 1. labels with 7 leading spaces
> 2. labels left justified
>
> Which one is the way you want them, left justified?
I think the left justified ones are from me, your patches matches with the
common usage, so I applied it.
Thanks,
Jim
>
> Bill
>
>
> _______________________________________________
> Selinux mailing list
> Selinux@tycho.nsa.gov
> To unsubscribe, send email to Selinux-leave@tycho.nsa.gov.
> To get help, send an email containing "help" to Selinux-request@tycho.nsa.gov.
>
--
James Carter <jwcart2@tycho.nsa.gov>
National Security Agency
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-08-09 20:26 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-08 17:28 [PATCH] libsepol: fix memory leak in expand.c william.c.roberts
2016-08-08 17:29 ` Roberts, William C
2016-08-09 16:12 ` Roberts, William C
2016-08-09 20:26 ` 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.