Re: [Non-DoD Source] Re: [PATCH 1/3] libsepol: Fix RESOURCE_LEAK defects reported by coverity scan

[jwcart2 <jwcart2@tycho.nsa.gov>]

On 1/31/19 4:17 PM, Nicolas Iooss wrote:
> On Thu, Jan 31, 2019 at 2:22 PM Petr Lautrbach <plautrba@redhat.com> wrote:
>>
>> Signed-off-by: Petr Lautrbach <plautrba@redhat.com>
>> ---
>>   libsepol/cil/src/cil_binary.c      | 12 ++++++++++++
>>   libsepol/cil/src/cil_resolve_ast.c | 10 ++++++++++
>>   libsepol/cil/src/cil_symtab.c      |  1 +
>>   libsepol/src/expand.c              |  3 +++
>>   libsepol/src/kernel_to_cil.c       |  2 ++
>>   libsepol/src/kernel_to_conf.c      |  2 ++
>>   6 files changed, 30 insertions(+)
>>
>> diff --git a/libsepol/cil/src/cil_binary.c b/libsepol/cil/src/cil_binary.c
>> index 0cc6eeb1..a645c95d 100644
>> --- a/libsepol/cil/src/cil_binary.c
>> +++ b/libsepol/cil/src/cil_binary.c
>> @@ -912,6 +912,8 @@ int cil_sensalias_to_policydb(policydb_t *pdb, struct cil_alias *cil_alias)
>>          key = cil_strdup(cil_alias->datum.fqn);
>>          rc = symtab_insert(pdb, SYM_LEVELS, key, sepol_alias, SCOPE_DECL, 0, NULL);
>>          if (rc != SEPOL_OK) {
>> +               if (rc == 1)
>> +                       free(sepol_alias);
>>                  goto exit;
>>          }
> 
> There is something weird here. The exit label starts with
> "level_datum_destroy(sepol_alias);". This is not a serious issue
> because level_datum_destroy() does not do anything, but after this
> patch, cil_sensalias_to_policydb()'s code seems to use sepol_alias
> after freeing it.
> 
> Should the call to level_datum_destroy(sepol_alias) be removed, or
> moved before free(sepol_alias)?
> 

The real problem is the statement after the "level_datum_destroy(sepol_alias);".
The "free(sepol_level);" should be "free(sepol_alias);".
The sepol_level references a datum passed back by hashtab_search() in 
__cil_get_sepol_class_datum() and is not a new object.

Jim

> Thanks,
> Nicolas
> 
> 


-- 
James Carter <jwcart2@tycho.nsa.gov>
National Security Agency