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

[Emil Velikov]

On 1 October 2015 at 16:59, Matt Roper <matthew.d.roper at intel.com> wrote:
> 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!
>
Ack. Amended and pushed to master.

-Emil