Hi Matt,
On 30 September 2015 at 17:30, Matt Roper <matthew.d.roper@xxxxxxxxx> 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@xxxxxxxxx>
> Signed-off-by: Matt Roper <matthew.d.roper@xxxxxxxxx>
> ---
> 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
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
