All of lore.kernel.org
 help / color / mirror / Atom feed
From: Nicolas Iooss <nicolas.iooss@m4x.org>
To: Petr Lautrbach <plautrba@redhat.com>
Cc: selinux@vger.kernel.org
Subject: Re: [PATCH v2] libselinux: Fix RESOURCE_LEAK defects reported by coverity scan
Date: Sun, 10 Feb 2019 19:32:40 +0100	[thread overview]
Message-ID: <CAJfZ7=n_cfVW6YQwjY3jvXL0o7k=8nFd6=KLCd0S3ncZYgRMJg@mail.gmail.com> (raw)
In-Reply-To: <CAJfZ7=nyFnqP2+bWXjK8zQkwD7T0BvmzH+zQFzoo4wW=97D4Wg@mail.gmail.com>

On Wed, Feb 6, 2019 at 11:40 PM Nicolas Iooss <nicolas.iooss@m4x.org> wrote:
>
> On Wed, Feb 6, 2019 at 9:33 PM Petr Lautrbach <plautrba@redhat.com> wrote:
> >
> > Fixes:
> >
> > libselinux/src/checkAccess.c:93: leaked_storage: Variable "user_context" going out of scope leaks the storage it points to.
> > libselinux/src/label_db.c:286: leaked_storage: Variable "filp" going out of scope leaks the storage it points to.
> > libselinux/src/label_db.c:291: leaked_storage: Variable "filp" going out of scope leaks the storage it points to.
> > libselinux/src/label_file.c:405: leaked_storage: Variable "str_buf" going out of scope leaks the storage it points to.
> > libselinux/src/load_policy.c:266: leaked_storage: Variable "names" going out of scope leaks the storage it points to.
> > libselinux/src/selinux_config.c:183: leaked_storage: Variable "end" going out of scope leaks the storage it points to.
> > libselinux/src/selinux_config.c:184: overwrite_var: Overwriting "end" in "end = type + strlen(type) - 1" leaks the storage that "end" points to.
> > libselinux/src/selinux_restorecon.c:376: leaked_storage: Variable "new_entry" going out of scope leaks the storage it points to.
> > libselinux/src/selinux_restorecon.c:855: leaked_storage: Variable "xattr_value" going out of scope leaks the storage it points to.
> >
> > Signed-off-by: Petr Lautrbach <plautrba@redhat.com>
>
> Acked-by: Nicolas Iooss <nicolas.iooss@m4x.org>

Merged.

Nicolas

> > ---
> >  libselinux/src/checkAccess.c        |  4 +++-
> >  libselinux/src/label_db.c           |  3 +++
> >  libselinux/src/label_file.c         |  4 +++-
> >  libselinux/src/load_policy.c        |  4 +++-
> >  libselinux/src/selinux_config.c     | 17 +++++++++--------
> >  libselinux/src/selinux_restorecon.c | 12 ++++++++++--
> >  6 files changed, 31 insertions(+), 13 deletions(-)
> >
> > diff --git a/libselinux/src/checkAccess.c b/libselinux/src/checkAccess.c
> > index 8de57477..16bfcfb6 100644
> > --- a/libselinux/src/checkAccess.c
> > +++ b/libselinux/src/checkAccess.c
> > @@ -89,8 +89,10 @@ int selinux_check_passwd_access(access_vector_t requested)
> >                 int retval;
> >
> >                 passwd_class = string_to_security_class("passwd");
> > -               if (passwd_class == 0)
> > +               if (passwd_class == 0) {
> > +                       freecon(user_context);
> >                         return 0;
> > +               }
> >
> >                 retval = security_compute_av_raw(user_context,
> >                                                      user_context,
> > diff --git a/libselinux/src/label_db.c b/libselinux/src/label_db.c
> > index c46d0a1d..fba96c92 100644
> > --- a/libselinux/src/label_db.c
> > +++ b/libselinux/src/label_db.c
> > @@ -283,10 +283,12 @@ db_init(const struct selinux_opt *opts, unsigned nopts,
> >         }
> >         if (fstat(fileno(filp), &sb) < 0) {
> >                 free(catalog);
> > +               fclose(filp);
> >                 return NULL;
> >         }
> >         if (!S_ISREG(sb.st_mode)) {
> >                 free(catalog);
> > +               fclose(filp);
> >                 errno = EINVAL;
> >                 return NULL;
> >         }
> > @@ -340,6 +342,7 @@ out_error:
> >                 free(spec->lr.ctx_trans);
> >         }
> >         free(catalog);
> > +       fclose(filp);
> >
> >         return NULL;
> >  }
> > diff --git a/libselinux/src/label_file.c b/libselinux/src/label_file.c
> > index dbf51a93..b81fd552 100644
> > --- a/libselinux/src/label_file.c
> > +++ b/libselinux/src/label_file.c
> > @@ -317,8 +317,10 @@ end_arch_check:
> >                         goto out;
> >                 }
> >                 rc = next_entry(str_buf, mmap_area, entry_len);
> > -               if (rc < 0)
> > +               if (rc < 0) {
> > +                       free(str_buf);
> >                         goto out;
> > +               }
> >
> >                 if (str_buf[entry_len - 1] != '\0') {
> >                         free(str_buf);
> > diff --git a/libselinux/src/load_policy.c b/libselinux/src/load_policy.c
> > index e9f1264a..20052beb 100644
> > --- a/libselinux/src/load_policy.c
> > +++ b/libselinux/src/load_policy.c
> > @@ -262,8 +262,10 @@ checkbool:
> >                         rc = security_get_boolean_names(&names, &len);
> >                         if (!rc) {
> >                                 values = malloc(sizeof(int) * len);
> > -                               if (!values)
> > +                               if (!values) {
> > +                                       free(names);
> >                                         goto unmap;
> > +                               }
> >                                 for (i = 0; i < len; i++)
> >                                         values[i] =
> >                                                 security_get_boolean_active(names[i]);
> > diff --git a/libselinux/src/selinux_config.c b/libselinux/src/selinux_config.c
> > index 292728f3..b06cb63b 100644
> > --- a/libselinux/src/selinux_config.c
> > +++ b/libselinux/src/selinux_config.c
> > @@ -177,8 +177,7 @@ static void init_selinux_config(void)
> >
> >                         if (!strncasecmp(buf_p, SELINUXTYPETAG,
> >                                          sizeof(SELINUXTYPETAG) - 1)) {
> > -                               selinux_policytype = type =
> > -                                   strdup(buf_p + sizeof(SELINUXTYPETAG) - 1);
> > +                               type = strdup(buf_p + sizeof(SELINUXTYPETAG) - 1);
> >                                 if (!type)
> >                                         return;
> >                                 end = type + strlen(type) - 1;
> > @@ -187,6 +186,11 @@ static void init_selinux_config(void)
> >                                         *end = 0;
> >                                         end--;
> >                                 }
> > +                               if (setpolicytype(type) != 0) {
> > +                                       free(type);
> > +                                       return;
> > +                               }
> > +                               free(type);
> >                                 continue;
> >                         } else if (!strncmp(buf_p, SETLOCALDEFS,
> >                                             sizeof(SETLOCALDEFS) - 1)) {
> > @@ -212,13 +216,10 @@ static void init_selinux_config(void)
> >                 fclose(fp);
> >         }
> >
> > -       if (!type) {
> > -               selinux_policytype = type = strdup(SELINUXDEFAULT);
> > -               if (!type)
> > -                       return;
> > -       }
> > +       if (!selinux_policytype && setpolicytype(SELINUXDEFAULT) != 0)
> > +               return;
> >
> > -       if (asprintf(&selinux_policyroot, "%s%s", SELINUXDIR, type) == -1)
> > +       if (asprintf(&selinux_policyroot, "%s%s", SELINUXDIR, selinux_policytype) == -1)
> >                 return;
> >
> >         for (i = 0; i < NEL; i++)
> > diff --git a/libselinux/src/selinux_restorecon.c b/libselinux/src/selinux_restorecon.c
> > index 0f0fd6ca..924d9538 100644
> > --- a/libselinux/src/selinux_restorecon.c
> > +++ b/libselinux/src/selinux_restorecon.c
> > @@ -351,12 +351,19 @@ static int add_xattr_entry(const char *directory, bool delete_nonmatch,
> >         new_entry->next = NULL;
> >
> >         new_entry->directory = strdup(directory);
> > -       if (!new_entry->directory)
> > +       if (!new_entry->directory) {
> > +               free(new_entry);
> > +               free(sha1_buf);
> >                 goto oom;
> > +       }
> >
> >         new_entry->digest = strdup(sha1_buf);
> > -       if (!new_entry->digest)
> > +       if (!new_entry->digest) {
> > +               free(new_entry->directory);
> > +               free(new_entry);
> > +               free(sha1_buf);
> >                 goto oom;
> > +       }
> >
> >         new_entry->result = digest_result;
> >
> > @@ -850,6 +857,7 @@ int selinux_restorecon(const char *pathname_orig,
> >
> >         if (lstat(pathname, &sb) < 0) {
> >                 if (flags.ignore_noent && errno == ENOENT) {
> > +                       free(xattr_value);
> >                         free(pathdnamer);
> >                         free(pathname);
> >                         return 0;
> > --
> > 2.20.1
> >


  reply	other threads:[~2019-02-10 18:32 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-01-31 13:22 [PATCH 1/3] libsepol: Fix RESOURCE_LEAK defects reported by coverity scan Petr Lautrbach
2019-01-31 13:22 ` [PATCH 2/3] libselinux: " Petr Lautrbach
2019-01-31 21:47   ` Nicolas Iooss
2019-02-06 20:33     ` [PATCH v2] " Petr Lautrbach
2019-02-06 22:40       ` Nicolas Iooss
2019-02-10 18:32         ` Nicolas Iooss [this message]
2019-01-31 13:22 ` [PATCH 3/3] libsemanage: Fix USE_AFTER_FREE " Petr Lautrbach
2019-01-31 21:57   ` Nicolas Iooss
2019-02-01 12:45     ` Petr Lautrbach
2019-01-31 21:17 ` [PATCH 1/3] libsepol: Fix RESOURCE_LEAK " Nicolas Iooss
2019-02-01 18:19   ` [Non-DoD Source] " jwcart2
2019-01-31 21:33 ` Nicolas Iooss
2019-02-01 19:32   ` [Non-DoD Source] " jwcart2
2019-02-01 20:07 ` [Non-DoD Source] " jwcart2

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAJfZ7=n_cfVW6YQwjY3jvXL0o7k=8nFd6=KLCd0S3ncZYgRMJg@mail.gmail.com' \
    --to=nicolas.iooss@m4x.org \
    --cc=plautrba@redhat.com \
    --cc=selinux@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.