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

[Emil Velikov]

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