[Mesa-dev] [PATCH 1/3] egl/x11_dri3: disable WL_bind_wayland_display for devices without render nodes

[Axel Davy]

Hi,

The three patches make sense to me.

Reviewed-by: Axel Davy <axel.davy at ens.fr>

On 17/06/2016 19:41, Frank Binns wrote :
> Up until now, DRI3 was only used for devices that have render nodes, unless
> overridden via an environment variable, with it falling back to DRI2 otherwise.
> This limitation was there in order to support WL_bind_wayland_display as it
> requires client opened device node fds to be authenticated, which isn't possible
> when using DRI3. This is an unfortunate compromise as DRI3 provides security
> benefits over DRI2.
>
> Instead, allow DRI3 to be used for devices without render nodes but don't
> advertise WL_bind_wayland_display in this case. Applications that need this
> extension can still be run by disabling DRI3 support via the LIBGL_DRI3_DISABLE
> environment variable.
>
> Signed-off-by: Frank Binns <frank.binns at imgtec.com>
> ---
>   src/egl/drivers/dri2/platform_x11.c      |  3 ++-
>   src/egl/drivers/dri2/platform_x11_dri3.c | 33 +++++---------------------------
>   2 files changed, 7 insertions(+), 29 deletions(-)
>
> diff --git a/src/egl/drivers/dri2/platform_x11.c b/src/egl/drivers/dri2/platform_x11.c
> index c0a4005..1b50889 100644
> --- a/src/egl/drivers/dri2/platform_x11.c
> +++ b/src/egl/drivers/dri2/platform_x11.c
> @@ -1335,7 +1335,8 @@ dri2_initialize_x11_dri3(_EGLDriver *drv, _EGLDisplay *disp)
>      disp->Extensions.EXT_buffer_age = EGL_TRUE;
>   
>   #ifdef HAVE_WAYLAND_PLATFORM
> -   disp->Extensions.WL_bind_wayland_display = EGL_TRUE;
> +   if (dri2_dpy->device_name)
> +      disp->Extensions.WL_bind_wayland_display = EGL_TRUE;
>   #endif
>   
>      if (dri2_dpy->conn) {
> diff --git a/src/egl/drivers/dri2/platform_x11_dri3.c b/src/egl/drivers/dri2/platform_x11_dri3.c
> index 9363a8a..b781987 100644
> --- a/src/egl/drivers/dri2/platform_x11_dri3.c
> +++ b/src/egl/drivers/dri2/platform_x11_dri3.c
> @@ -437,29 +437,6 @@ struct dri2_egl_display_vtbl dri3_x11_display_vtbl = {
>      .get_dri_drawable = dri3_get_dri_drawable,
>   };
>   
> -static char *
> -dri3_get_device_name(int fd)
> -{
> -   char *ret = NULL;
> -
> -   ret = drmGetRenderDeviceNameFromFd(fd);
> -   if (ret)
> -      return ret;
> -
> -   /* For dri3, render node support is required for WL_bind_wayland_display.
> -    * In order not to regress on older systems without kernel or libdrm
> -    * support, fall back to dri2. User can override it with environment
> -    * variable if they don't need to use that extension.
> -    */
> -   if (getenv("EGL_FORCE_DRI3") == NULL) {
> -      _eglLog(_EGL_WARNING, "Render node support not available, falling back to dri2");
> -      _eglLog(_EGL_WARNING, "If you want to force dri3, set EGL_FORCE_DRI3 environment variable");
> -   } else
> -      ret = loader_get_device_name_for_fd(fd);
> -
> -   return ret;
> -}
> -
>   EGLBoolean
>   dri3_x11_connect(struct dri2_egl_display *dri2_dpy)
>   {
> @@ -539,11 +516,11 @@ dri3_x11_connect(struct dri2_egl_display *dri2_dpy)
>         return EGL_FALSE;
>      }
>   
> -   dri2_dpy->device_name = dri3_get_device_name(dri2_dpy->fd);
> -   if (!dri2_dpy->device_name) {
> -      close(dri2_dpy->fd);
> -      return EGL_FALSE;
> -   }
> +   /* Only try to get a render device name since it's only needed for
> +    * WL_bind_wayland_display and dri3 doesn't provide a mechanism for
> +    * authenticating client opened device node fds. If this fails then
> +    * don't advertise the extension. */
> +   dri2_dpy->device_name = drmGetRenderDeviceNameFromFd(dri2_dpy->fd);
>   
>      return EGL_TRUE;
>   }