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

[Petr Lautrbach <plautrba@redhat.com>]

Petr Lautrbach <plautrba@redhat.com> writes:

> 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>

Merged.


>
>> ---
>>  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