On 1 October 2015 at 16:59, Matt Roper <matthew.d.roper@xxxxxxxxx> 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@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
>
> Sure, that sounds fine too. Thanks!
>
Ack. Amended and pushed to master.
-Emil
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
