[PATCH 1/3] libsepol: do not free attr_name twice

[PATCH 1/3] libsepol: do not free attr_name twice

[Nicolas Iooss]

When set_to_names() fails to allocate *names, it frees variable
attr_name even though it either came from attr_list or was newly created
and added to attr_list. By doing so, the name is freed a second time
when attr_list is destroyed (with "attr_list_destroy(&attr_list)").

Avoid this double free by not freeing attr_name when it belongs to
attr_list.

This issue has been found using clang's static analyzer.

Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
---
 libsepol/src/module_to_cil.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/libsepol/src/module_to_cil.c b/libsepol/src/module_to_cil.c
index 3f633fbb0a3f..18b2a6f86fe3 100644
--- a/libsepol/src/module_to_cil.c
+++ b/libsepol/src/module_to_cil.c
@@ -962,7 +962,6 @@ static int set_to_names(struct policydb *pdb, int is_type, void *set, struct lis
 	*names = malloc(sizeof(char *));
 	if (!*names) {
 		log_err("Out of memory");
-		free(attr_name);
 		rc = -1;
 		goto exit;
 	}
-- 
2.12.0

[PATCH 2/3] libsepol: do not leak memory when an error occurs

[Nicolas Iooss]

name_list_to_string() and constraint_expr_to_string() both define an
exit label to clean-up dynamically-allocated memory when an error
occurs, but they miss some variables. Free the missing ones too.

This issue has been found using clang's static analyzer.

Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
---
 libsepol/src/module_to_cil.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/libsepol/src/module_to_cil.c b/libsepol/src/module_to_cil.c
index 18b2a6f86fe3..45acdeb1a4e0 100644
--- a/libsepol/src/module_to_cil.c
+++ b/libsepol/src/module_to_cil.c
@@ -1153,6 +1153,7 @@ static int name_list_to_string(char **names, int num_names, char **string)
 
 	return 0;
 exit:
+	free(str);
 	return rc;
 }
 
@@ -1697,7 +1698,7 @@ static int constraint_expr_to_string(struct policydb *pdb, struct constraint_exp
 	const char *fmt_str;
 	const char *attr1;
 	const char *attr2;
-	char *names;
+	char *names = NULL;
 	char **name_list = NULL;
 	int num_names = 0;
 	struct type_set *ts;
@@ -1798,6 +1799,7 @@ static int constraint_expr_to_string(struct policydb *pdb, struct constraint_exp
 
 				names_destroy(&name_list, &num_names);
 				free(names);
+				names = NULL;
 			}
 
 			num_params = 0;
@@ -1887,6 +1889,7 @@ static int constraint_expr_to_string(struct policydb *pdb, struct constraint_exp
 
 exit:
 	names_destroy(&name_list, &num_names);
+	free(names);
 
 	free(new_val);
 	free(val1);
-- 
2.12.0

[PATCH 3/3] libsepol: correct spelling errors in module_to_cil.c comments

[Nicolas Iooss]

Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
---
 libsepol/src/module_to_cil.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/libsepol/src/module_to_cil.c b/libsepol/src/module_to_cil.c
index 45acdeb1a4e0..ac095c30f4f3 100644
--- a/libsepol/src/module_to_cil.c
+++ b/libsepol/src/module_to_cil.c
@@ -791,8 +791,8 @@ static int cil_print_attr_strs(int indent, struct policydb *pdb, int is_type, vo
 	// CIL doesn't support anonymous positive/negative/complemented sets.  So
 	// instead we create a CIL type/roleattributeset that matches the set. If
 	// the set has a negative set, then convert it to is (P & !N), where P is
-	// the list of members in the positive set , and N is the list of members
-	// in the negative set. Additonally, if the set is complemented, then wrap
+	// the list of members in the positive set and N is the list of members
+	// in the negative set. Additionally, if the set is complemented, then wrap
 	// the whole thing with a negation.
 
 	struct ebitmap_node *node;
@@ -2103,13 +2103,13 @@ static int role_to_cil(int indent, struct policydb *pdb, struct avrule_block *UN
 			// the policy type, it would result in duplicate declarations,
 			// which isn't allowed in CIL. Patches have been made to refpolicy
 			// to remove these duplicate role declarations, but we need to be
-			// backwards compatable and support older policies. Since we know
+			// backwards compatible and support older policies. Since we know
 			// these roles are always declared in base, only print them when we
 			// see them in the base module. If the declarations appear in a
 			// non-base module, ignore their declarations.
 			//
 			// Note that this is a hack, and if a policy author does not define
-			// one of these roles in base, the declaration will not appeaer in
+			// one of these roles in base, the declaration will not appear in
 			// the resulting policy, likely resulting in a compilation error in
 			// CIL.
 			//
@@ -2289,7 +2289,7 @@ static int user_to_cil(int indent, struct policydb *pdb, struct avrule_block *bl
 
 	if (block->flags & AVRULE_OPTIONAL) {
 		// sensitivites in user statements in optionals do not have the
-		// standard -1 offest
+		// standard -1 offset
 		sens_offset = 0;
 	}
 
-- 
2.12.0

Re: [PATCH 3/3] libsepol: correct spelling errors in module_to_cil.c comments

[Stephen Smalley]

On Mon, 2017-04-10 at 21:11 +0200, Nicolas Iooss wrote:
> Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>

Thanks, all three applied.

> ---
>  libsepol/src/module_to_cil.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/libsepol/src/module_to_cil.c
> b/libsepol/src/module_to_cil.c
> index 45acdeb1a4e0..ac095c30f4f3 100644
> --- a/libsepol/src/module_to_cil.c
> +++ b/libsepol/src/module_to_cil.c
> @@ -791,8 +791,8 @@ static int cil_print_attr_strs(int indent, struct
> policydb *pdb, int is_type, vo
>  	// CIL doesn't support anonymous
> positive/negative/complemented sets.  So
>  	// instead we create a CIL type/roleattributeset that
> matches the set. If
>  	// the set has a negative set, then convert it to is (P &
> !N), where P is
> -	// the list of members in the positive set , and N is the
> list of members
> -	// in the negative set. Additonally, if the set is
> complemented, then wrap
> +	// the list of members in the positive set and N is the list
> of members
> +	// in the negative set. Additionally, if the set is
> complemented, then wrap
>  	// the whole thing with a negation.
>  
>  	struct ebitmap_node *node;
> @@ -2103,13 +2103,13 @@ static int role_to_cil(int indent, struct
> policydb *pdb, struct avrule_block *UN
>  			// the policy type, it would result in
> duplicate declarations,
>  			// which isn't allowed in CIL. Patches have
> been made to refpolicy
>  			// to remove these duplicate role
> declarations, but we need to be
> -			// backwards compatable and support older
> policies. Since we know
> +			// backwards compatible and support older
> policies. Since we know
>  			// these roles are always declared in base,
> only print them when we
>  			// see them in the base module. If the
> declarations appear in a
>  			// non-base module, ignore their
> declarations.
>  			//
>  			// Note that this is a hack, and if a policy
> author does not define
> -			// one of these roles in base, the
> declaration will not appeaer in
> +			// one of these roles in base, the
> declaration will not appear in
>  			// the resulting policy, likely resulting in
> a compilation error in
>  			// CIL.
>  			//
> @@ -2289,7 +2289,7 @@ static int user_to_cil(int indent, struct
> policydb *pdb, struct avrule_block *bl
>  
>  	if (block->flags & AVRULE_OPTIONAL) {
>  		// sensitivites in user statements in optionals do
> not have the
> -		// standard -1 offest
> +		// standard -1 offset
>  		sens_offset = 0;
>  	}
>