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

From: Thierry Reding <thierry.reding@gmail.com>
To: Emil Velikov <emil.l.velikov@gmail.com>
Cc: dri-devel@lists.freedesktop.org
Subject: Re: [PATCH libdrm 3/4] xf86drm: fix incorrect fd comparison in drmOpenOnce{,WithType}
Date: Wed, 15 Jul 2015 13:47:24 +0200	[thread overview]
Message-ID: <20150715114723.GC15045@ulmo.nvidia.com> (raw)
In-Reply-To: <1436883005-6163-3-git-send-email-emil.l.velikov@gmail.com>

[-- Attachment #1.1: Type: text/plain, Size: 1155 bytes --]

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@gmail.com>
> Signed-off-by: Emil Velikov <emil.l.velikov@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.

Thierry

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel