[PATCH libdrm] xf86drm: Fix error handling for drmGetDevices()

[Matt Roper]

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