[PATCH libdrm 3/4] xf86drm: fix incorrect fd comparison in drmOpenOnce{, WithType}

[Emil Velikov]

On 15 July 2015 at 13:47, Thierry Reding <thierry.reding at gmail.com> wrote:
> On Wed, Jul 15, 2015 at 01:37:22PM +0100, Emil Velikov wrote:
>> On 15 July 2015 at 12:47, Thierry Reding <thierry.reding at gmail.com> wrote:
>> > On Tue, Jul 14, 2015 at 03:10:04PM +0100, Emil Velikov wrote:
>> >> Spotted by looking for similar "let's assume fd == 0 is invalid" bugs.
>> >>
>> >> Cc: Thierry Reding <thierry.reding at gmail.com>
>> >> Signed-off-by: Emil Velikov <emil.l.velikov at gmail.com>
>> >> ---
>> >>  xf86drm.c | 2 +-
>> >>  1 file changed, 1 insertion(+), 1 deletion(-)
>> >>
>> >> diff --git a/xf86drm.c b/xf86drm.c
>> >> index 2c17d11..39c6e2d 100644
>> >> --- a/xf86drm.c
>> >> +++ b/xf86drm.c
>> >> @@ -2619,7 +2619,7 @@ int drmOpenOnceWithType(const char *BusID, int *newlyopened, int type)
>> >>       }
>> >>
>> >>      fd = drmOpenWithType(NULL, BusID, type);
>> >> -    if (fd <= 0 || nr_fds == DRM_MAX_FDS)
>> >> +    if (fd < 0 || nr_fds == DRM_MAX_FDS)
>> >
>> > Consider what happens if we have DRM_MAX_FDS file descriptors open and
>> > the call to drmOpenWithType() succeeds. We'll end up returning the file
>> > descriptor as is, but we won't keep track.
>> >
>> > I suppose this could have been on purpose, so that the device could be
>> > opened even if the file descriptor couldn't be cached anymore. One
>> > potential problem with that could be that the open-once restriction
>> > would be silently ignored. That may not be desirable.
>> >
>> Thanks for reviewing !
>>
>> Yes I have considered the issue. It's slightly different bug (fixed by
>> using dynamic allocation?) than what this patch aims at. This fix came
>> along for consistency sake rather than me caring about this legacy
>> API.
>
> Right, I had meant to say:
>
> Reviewed-by: Thierry Reding <treding at nvidia.com>
>
> irrespective of the above, given that they are two orthogonal issues.
>
Ack. Greatly appreciated.

-Emil