On Thu, Oct 01, 2015 at 11:12:34AM +0100, Emil Velikov wrote:
> Hi Matt,
>
> On 30 September 2015 at 17:30, Matt Roper <matthew.d.roper at intel.com>
> wrote:
> > If the opendir() call in drmGetDevices() returns failure, we jump to an
> > error label that calls closedir() and then returns. However this means
> > that we're calling closedir(NULL) which may not be safe on all
> > implementations. We are also leaking the local_devices array that was
> > allocated before the opendir() call.
> >
> > Fix both of these issues by jumping to an earlier error label (to free
> > local_devices) and guarding the closedir() call with a NULL test.
> >
> > Cc: Emil Velikov <emil.l.velikov at gmail.com>
> > Signed-off-by: Matt Roper <matthew.d.roper at intel.com>
> > ---
> > xf86drm.c | 7 ++++---
> > 1 file changed, 4 insertions(+), 3 deletions(-)
> >
> > diff --git a/xf86drm.c b/xf86drm.c
> > index c1cab1b..763d710 100644
> > --- a/xf86drm.c
> > +++ b/xf86drm.c
> > @@ -3209,7 +3209,7 @@ int drmGetDevices(drmDevicePtr devices[], int
> > max_devices)
> > sysdir = opendir(DRM_DIR_NAME);
> > if (!sysdir) {
> > ret = -errno;
> > - goto close_sysdir;
> > + goto free_locals;
> > }
> >
> > i = 0;
> > @@ -3280,9 +3280,10 @@ int drmGetDevices(drmDevicePtr devices[], int
> > max_devices)
> >
> > free_devices:
> > drmFreeDevices(local_devices, i);
> > +free_locals:
> > free(local_devices);
> >
> > -close_sysdir:
> > - closedir(sysdir);
> > + if (sysdir)
> > + closedir(sysdir);
> Any objections if we move the new label & free() here and drop the if
> check above? I can do that before pushing if that's ok with you.
>
> Thanks for catching this.
> Emil
Sure, that sounds fine too. Thanks!
Matt
--
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
