Re: [PATCH] libsepol/cil: Free duplicate datums in original calling function

[Petr Lautrbach <plautrba@redhat.com>]

James Carter <jwcart2@gmail.com> writes:

> Duplicate declarations are allowed for type, typeattribute, and
> optional statements. When an allowed duplicate declaration is found,
> the duplicate datum is free'd in cil_add_decl_to_symtab() and SEPOL_OK
> is returned. This works for all the rules where a duplicate declaration
> is allowed, but it confuses scanning tools.
>
> When cil_add_decl_to_symtab() finds an allowed duplicate declaration,
> return SEPOL_EEXIST and free the duplicate datum in the original
> calling function.
>
> Signed-off-by: James Carter <jwcart2@gmail.com>
>

I've tested the patch and the scanner doesn't report defects from
https://lore.kernel.org/selinux/874kat9dch.fsf@redhat.com/T/#t anymore.

Thanks!

Acked-by: Petr Lautrbach <plautrba@redhat.com>


> ---
>  libsepol/cil/src/cil_build_ast.c | 27 +++++++++++++++++++++------
>  libsepol/cil/src/cil_copy_ast.c  |  9 ++++++++-
>  2 files changed, 29 insertions(+), 7 deletions(-)
>
> diff --git a/libsepol/cil/src/cil_build_ast.c b/libsepol/cil/src/cil_build_ast.c
> index f1f09f11..9c34be23 100644
> --- a/libsepol/cil/src/cil_build_ast.c
> +++ b/libsepol/cil/src/cil_build_ast.c
> @@ -134,8 +134,7 @@ int cil_add_decl_to_symtab(struct cil_db *db, symtab_t *symtab, hashtab_key_t ke
>  		/* multiple_decls is enabled and works for this datum type, add node */
>  		cil_list_append(prev->nodes, CIL_NODE, node);
>  		node->data = prev;
> -		cil_symtab_datum_destroy(datum);
> -		free(datum);
> +		return SEPOL_EEXIST;
>  	}
>  
>  	return SEPOL_OK;
> @@ -2367,7 +2366,12 @@ int cil_gen_type(struct cil_db *db, struct cil_tree_node *parse_current, struct
>  	key = parse_current->next->data;
>  	rc = cil_gen_node(db, ast_node, (struct cil_symtab_datum*)type, (hashtab_key_t)key, CIL_SYM_TYPES, CIL_TYPE);
>  	if (rc != SEPOL_OK) {
> -		goto exit;
> +		if (rc == SEPOL_EEXIST) {
> +			cil_destroy_type(type);
> +			type = NULL;
> +		} else {
> +			goto exit;
> +		}
>  	}
>  
>  	return SEPOL_OK;
> @@ -2415,7 +2419,12 @@ int cil_gen_typeattribute(struct cil_db *db, struct cil_tree_node *parse_current
>  	key = parse_current->next->data;
>  	rc = cil_gen_node(db, ast_node, (struct cil_symtab_datum*)attr, (hashtab_key_t)key, CIL_SYM_TYPES, CIL_TYPEATTRIBUTE);
>  	if (rc != SEPOL_OK) {
> -		goto exit;
> +		if (rc == SEPOL_EEXIST) {
> +			cil_destroy_typeattribute(attr);
> +			attr = NULL;
> +		} else {
> +			goto exit;
> +		}
>  	}
>  
>  	return SEPOL_OK;
> @@ -5480,8 +5489,14 @@ int cil_gen_optional(struct cil_db *db, struct cil_tree_node *parse_current, str
>  	key = parse_current->next->data;
>  
>  	rc = cil_gen_node(db, ast_node, (struct cil_symtab_datum*)optional, (hashtab_key_t)key, CIL_SYM_BLOCKS, CIL_OPTIONAL);
> -	if (rc != SEPOL_OK)
> -		goto exit;
> +	if (rc != SEPOL_OK) {
> +		if (rc == SEPOL_EEXIST) {
> +			cil_destroy_optional(optional);
> +			optional = NULL;
> +		} else {
> +			goto exit;
> +		}
> +	}
>  
>  	return SEPOL_OK;
>  
> diff --git a/libsepol/cil/src/cil_copy_ast.c b/libsepol/cil/src/cil_copy_ast.c
> index cdbc84e7..7c5ae9e1 100644
> --- a/libsepol/cil/src/cil_copy_ast.c
> +++ b/libsepol/cil/src/cil_copy_ast.c
> @@ -2056,7 +2056,14 @@ int __cil_copy_node_helper(struct cil_tree_node *orig, uint32_t *finished, void
>  
>  			rc = cil_add_decl_to_symtab(db, symtab, DATUM(orig->data)->name, DATUM(data), new);
>  			if (rc != SEPOL_OK) {
> -				goto exit;
> +				if (rc == SEPOL_EEXIST) {
> +					cil_symtab_datum_destroy(data);
> +					free(data);
> +					data = NULL;
> +					rc = SEPOL_OK;
> +				} else {
> +					goto exit;
> +				}
>  			}
>  
>  			namespace = new;
> -- 
> 2.31.1