Coverity: filename_trans_read_one(): Resource leaks

Coverity: filename_trans_read_one(): Resource leaks

[coverity-bot]

Hello!

This is an experimental automated report about issues detected by Coverity
from a scan of next-20200413 as part of the linux-next weekly scan project:
https://scan.coverity.com/projects/linux-next-weekly-scan

You're getting this email because you were associated with the identified
lines of code (noted below) that were touched by commits:

  Tue Feb 18 12:27:34 2020 +0100
    c3a276111ea2 ("selinux: optimize storage of filename transitions")

Coverity reported the following:

*** CID 1461665:  Resource leaks  (RESOURCE_LEAK)
/security/selinux/ss/policydb.c: 1862 in filename_trans_read_one()
1856     		return rc;
1857     	len = le32_to_cpu(buf[0]);
1858
1859     	/* path component string */
1860     	rc = str_read(&name, GFP_KERNEL, fp, len);
1861     	if (rc)
vvv     CID 1461665:  Resource leaks  (RESOURCE_LEAK)
vvv     Variable "name" going out of scope leaks the storage it points to.
1862     		return rc;
1863
1864     	rc = next_entry(buf, fp, sizeof(u32) * 4);
1865     	if (rc)
1866     		goto out;
1867

If this is a false positive, please let us know so we can mark it as
such, or teach the Coverity rules to be smarter. If not, please make
sure fixes get into linux-next. :) For patches fixing this, please
include these lines (but double-check the "Fixes" first):

Reported-by: coverity-bot <keescook+coverity-bot@chromium.org>
Addresses-Coverity-ID: 1461665 ("Resource leaks")
Fixes: c3a276111ea2 ("selinux: optimize storage of filename transitions")

Thanks for your attention!

-- 
Coverity-bot

Re: Coverity: filename_trans_read_one(): Resource leaks

[Ondrej Mosnacek]

On Mon, Apr 13, 2020 at 6:18 PM coverity-bot <keescook@chromium.org> wrote:
> Hello!
>
> This is an experimental automated report about issues detected by Coverity
> from a scan of next-20200413 as part of the linux-next weekly scan project:
> https://scan.coverity.com/projects/linux-next-weekly-scan
>
> You're getting this email because you were associated with the identified
> lines of code (noted below) that were touched by commits:
>
>   Tue Feb 18 12:27:34 2020 +0100
>     c3a276111ea2 ("selinux: optimize storage of filename transitions")
>
> Coverity reported the following:
>
> *** CID 1461665:  Resource leaks  (RESOURCE_LEAK)
> /security/selinux/ss/policydb.c: 1862 in filename_trans_read_one()
> 1856                    return rc;
> 1857            len = le32_to_cpu(buf[0]);
> 1858
> 1859            /* path component string */
> 1860            rc = str_read(&name, GFP_KERNEL, fp, len);
> 1861            if (rc)
> vvv     CID 1461665:  Resource leaks  (RESOURCE_LEAK)
> vvv     Variable "name" going out of scope leaks the storage it points to.
> 1862                    return rc;
> 1863
> 1864            rc = next_entry(buf, fp, sizeof(u32) * 4);
> 1865            if (rc)
> 1866                    goto out;
> 1867

Right, I missed the fact that str_read() may give us back an allocated
pointer even if it returns an error. I'll send a fix probably
tomorrow. And I plan to have a look at refactoring the function so it
cleans up upon error on its own (+ updating the caller accordingly).
Its current interface just begs for trouble...

Thank you for running the bot, Kees! It's cool :)

>
> If this is a false positive, please let us know so we can mark it as
> such, or teach the Coverity rules to be smarter. If not, please make
> sure fixes get into linux-next. :) For patches fixing this, please
> include these lines (but double-check the "Fixes" first):
>
> Reported-by: coverity-bot <keescook+coverity-bot@chromium.org>
> Addresses-Coverity-ID: 1461665 ("Resource leaks")
> Fixes: c3a276111ea2 ("selinux: optimize storage of filename transitions")
>
> Thanks for your attention!
>
> --
> Coverity-bot
>

-- 
Ondrej Mosnacek <omosnace at redhat dot com>
Software Engineer, Security Technologies
Red Hat, Inc.