Re: [PATCH 1/3] libselinux: free empty scandir(3) result

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Apr 30, 2024 at 10:57 AM William Roberts
<bill.c.roberts@xxxxxxxxx> wrote:
>
> On Tue, Apr 30, 2024 at 9:35 AM Christian Göttsche
> <cgzones@xxxxxxxxxxxxxx> wrote:
> >
> > On Mon, 29 Apr 2024 at 22:35, William Roberts <bill.c.roberts@xxxxxxxxx> wrote:
> > >
> > > On Mon, Apr 29, 2024 at 11:39 AM Christian Göttsche
> > > <cgoettsche@xxxxxxxxxxxxx> wrote:
> > > >
> > > > From: Christian Göttsche <cgzones@xxxxxxxxxxxxxx>
> > > >
> > > > In case scandir(3) finds no entries still free the returned result to
> > > > avoid leaking it.
> > > >
> > > > Also do not override errno in case of a failure.
> > > >
> > > > Reported.by: Cppcheck
> > > >
> > > > Signed-off-by: Christian Göttsche <cgzones@xxxxxxxxxxxxxx>
> > > > ---
> > > >  libselinux/src/booleans.c | 6 +++++-
> > > >  1 file changed, 5 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/libselinux/src/booleans.c b/libselinux/src/booleans.c
> > > > index c557df65..1ede8e2d 100644
> > > > --- a/libselinux/src/booleans.c
> > > > +++ b/libselinux/src/booleans.c
> > > > @@ -53,7 +53,11 @@ int security_get_boolean_names(char ***names, int *len)
> > > >
> > > >         snprintf(path, sizeof path, "%s%s", selinux_mnt, SELINUX_BOOL_DIR);
> > > >         *len = scandir(path, &namelist, &filename_select, alphasort);
> > > > -       if (*len <= 0) {
> > > > +       if (*len < 0) {
> > > > +               return -1;
> > > > +       }
> > > > +       if (*len == 0) {
> > >
> > > Changing this will allow scandir to fail and it continue, what's the point?
> >
> > What do you mean by "continue"?
> > The function will still return -1 with errno set if scandir(3) returns
> > <= 0, like it does currently.
> > But currently if scandir() returns 0, we currently leak the pointer to
> > the empty array.
>
> I completely misread that, my apologies. I just mocked scandir for all
> those paths
> with the leak sanitizer enabled, lgtm.
>

I assume that is an ack for this patch as well as the other two?

Thanks,
Jim


> >
> > >
> > > > +               free(namelist);
> > > >                 errno = ENOENT;
> > > >                 return -1;
> > > >         }
> > > > --
> > > > 2.43.0
> > > >
> > > >
>





[Index of Archives]     [Selinux Refpolicy]     [Linux SGX]     [Fedora Users]     [Fedora Desktop]     [Yosemite Photos]     [Yosemite Camping]     [Yosemite Campsites]     [KDE Users]     [Gnome Users]

  Powered by Linux