Re: [Mesa-dev] [PATCH v2] vulkan/wsi: check if the display_fd given is master
On 2019-04-19 at 16:01, Emil Velikov wrote:
> From: Emil Velikov
>
> As effectively required by the extension, we need to ensure we're master
>
> Currently drivers employ vendor specific solutions, which check if the
> device behind the fd is capable*, yet none of them do the master check.
>
> *In the radv case, if acceleration is available.
>
> Instead of duplicating the check in each driver, keep it where it's
> needed and used.
>
> Note this copies libdrm's drmIsMaster() to avoid depending on bleeding
> edge version of the library.
>
> v2: set the fd to -1 if not master (Bas)
>
> Cc: Keith Packard
> Cc: Jason Ekstrand
> Cc: Bas Nieuwenhuizen
> Cc: Andres Rodriguez
> Reported-by: Andres Rodriguez
> Fixes: da997ebec92 ("vulkan: Add KHR_display extension using DRM [v10]")
> Signed-off-by: Emil Velikov
> ---
> src/vulkan/wsi/wsi_common_display.c | 27 +++
> 1 file changed, 27 insertions(+)
>
> diff --git a/src/vulkan/wsi/wsi_common_display.c
> b/src/vulkan/wsi/wsi_common_display.c
> index 74ed36ed646..2be20e85046 100644
> --- a/src/vulkan/wsi/wsi_common_display.c
> +++ b/src/vulkan/wsi/wsi_common_display.c
> @@ -1812,6 +1812,30 @@ fail_attr_init:
> return ret;
> }
>
> +
> +/*
> + * Local version fo the libdrm helper. Added to avoid depending on bleeding
> + * edge version of the library.
Could you specify the actual version in this comment, so that we can drop this
copy
once we start depending on that version anyway?
With that:
Acked-by: Eric Engestrom
> + */
> +static int
> +local_drmIsMaster(int fd)
> +{
> + /* Detect master by attempting something that requires master.
> +*
> +* Authenticating magic tokens requires master and 0 is an
> +* internal kernel detail which we could use. Attempting this on
> +* a master fd would fail therefore fail with EINVAL because 0
> +* is invalid.
> +*
> +* A non-master fd will fail with EACCES, as the kernel checks
> +* for master before attempting to do anything else.
> +*
> +* Since we don't want to leak implementation details, use
> +* EACCES.
> +*/
> + return drmAuthMagic(fd, 0) != -EACCES;
> +}
> +
> VkResult
> wsi_display_init_wsi(struct wsi_device *wsi_device,
> const VkAllocationCallbacks *alloc,
> @@ -1827,6 +1851,9 @@ wsi_display_init_wsi(struct wsi_device *wsi_device,
> }
>
> wsi->fd = display_fd;
> + if (wsi->fd != -1 && !local_drmIsMaster(wsi->fd))
> + wsi->fd = -1;
> +
> wsi->alloc = alloc;
>
> list_inithead(>connectors);
> --
> 2.21.0
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2] vulkan/wsi: check if the display_fd given is master
r-b
On Thu, Apr 25, 2019 at 12:22 PM Emil Velikov wrote:
>
> On Fri, 19 Apr 2019 at 16:01, Emil Velikov wrote:
> >
> > From: Emil Velikov
> >
> > As effectively required by the extension, we need to ensure we're master
> >
> > Currently drivers employ vendor specific solutions, which check if the
> > device behind the fd is capable*, yet none of them do the master check.
> >
> > *In the radv case, if acceleration is available.
> >
> > Instead of duplicating the check in each driver, keep it where it's
> > needed and used.
> >
> > Note this copies libdrm's drmIsMaster() to avoid depending on bleeding
> > edge version of the library.
> >
> > v2: set the fd to -1 if not master (Bas)
> >
> > Cc: Keith Packard
> > Cc: Jason Ekstrand
> > Cc: Bas Nieuwenhuizen
> > Cc: Andres Rodriguez
> > Reported-by: Andres Rodriguez
> > Fixes: da997ebec92 ("vulkan: Add KHR_display extension using DRM [v10]")
> > Signed-off-by: Emil Velikov
> > ---
> > src/vulkan/wsi/wsi_common_display.c | 27 +++
> > 1 file changed, 27 insertions(+)
> >
> > diff --git a/src/vulkan/wsi/wsi_common_display.c
> > b/src/vulkan/wsi/wsi_common_display.c
> > index 74ed36ed646..2be20e85046 100644
> > --- a/src/vulkan/wsi/wsi_common_display.c
> > +++ b/src/vulkan/wsi/wsi_common_display.c
> > @@ -1812,6 +1812,30 @@ fail_attr_init:
> > return ret;
> > }
> >
> > +
> > +/*
> > + * Local version fo the libdrm helper. Added to avoid depending on bleeding
> > + * edge version of the library.
> > + */
> > +static int
> > +local_drmIsMaster(int fd)
> > +{
> > + /* Detect master by attempting something that requires master.
> > +*
> > +* Authenticating magic tokens requires master and 0 is an
> > +* internal kernel detail which we could use. Attempting this on
> > +* a master fd would fail therefore fail with EINVAL because 0
> > +* is invalid.
> > +*
> > +* A non-master fd will fail with EACCES, as the kernel checks
> > +* for master before attempting to do anything else.
> > +*
> > +* Since we don't want to leak implementation details, use
> > +* EACCES.
> > +*/
> > + return drmAuthMagic(fd, 0) != -EACCES;
> > +}
> > +
> > VkResult
> > wsi_display_init_wsi(struct wsi_device *wsi_device,
> > const VkAllocationCallbacks *alloc,
> > @@ -1827,6 +1851,9 @@ wsi_display_init_wsi(struct wsi_device *wsi_device,
> > }
> >
> > wsi->fd = display_fd;
> > + if (wsi->fd != -1 && !local_drmIsMaster(wsi->fd))
> > + wsi->fd = -1;
> > +
> > wsi->alloc = alloc;
> >
> Humble ping?
>
> -Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2] vulkan/wsi: check if the display_fd given is master
On Fri, 19 Apr 2019 at 16:01, Emil Velikov wrote:
>
> From: Emil Velikov
>
> As effectively required by the extension, we need to ensure we're master
>
> Currently drivers employ vendor specific solutions, which check if the
> device behind the fd is capable*, yet none of them do the master check.
>
> *In the radv case, if acceleration is available.
>
> Instead of duplicating the check in each driver, keep it where it's
> needed and used.
>
> Note this copies libdrm's drmIsMaster() to avoid depending on bleeding
> edge version of the library.
>
> v2: set the fd to -1 if not master (Bas)
>
> Cc: Keith Packard
> Cc: Jason Ekstrand
> Cc: Bas Nieuwenhuizen
> Cc: Andres Rodriguez
> Reported-by: Andres Rodriguez
> Fixes: da997ebec92 ("vulkan: Add KHR_display extension using DRM [v10]")
> Signed-off-by: Emil Velikov
> ---
> src/vulkan/wsi/wsi_common_display.c | 27 +++
> 1 file changed, 27 insertions(+)
>
> diff --git a/src/vulkan/wsi/wsi_common_display.c
> b/src/vulkan/wsi/wsi_common_display.c
> index 74ed36ed646..2be20e85046 100644
> --- a/src/vulkan/wsi/wsi_common_display.c
> +++ b/src/vulkan/wsi/wsi_common_display.c
> @@ -1812,6 +1812,30 @@ fail_attr_init:
> return ret;
> }
>
> +
> +/*
> + * Local version fo the libdrm helper. Added to avoid depending on bleeding
> + * edge version of the library.
> + */
> +static int
> +local_drmIsMaster(int fd)
> +{
> + /* Detect master by attempting something that requires master.
> +*
> +* Authenticating magic tokens requires master and 0 is an
> +* internal kernel detail which we could use. Attempting this on
> +* a master fd would fail therefore fail with EINVAL because 0
> +* is invalid.
> +*
> +* A non-master fd will fail with EACCES, as the kernel checks
> +* for master before attempting to do anything else.
> +*
> +* Since we don't want to leak implementation details, use
> +* EACCES.
> +*/
> + return drmAuthMagic(fd, 0) != -EACCES;
> +}
> +
> VkResult
> wsi_display_init_wsi(struct wsi_device *wsi_device,
> const VkAllocationCallbacks *alloc,
> @@ -1827,6 +1851,9 @@ wsi_display_init_wsi(struct wsi_device *wsi_device,
> }
>
> wsi->fd = display_fd;
> + if (wsi->fd != -1 && !local_drmIsMaster(wsi->fd))
> + wsi->fd = -1;
> +
> wsi->alloc = alloc;
>
Humble ping?
-Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v2] vulkan/wsi: check if the display_fd given is master
From: Emil Velikov
As effectively required by the extension, we need to ensure we're master
Currently drivers employ vendor specific solutions, which check if the
device behind the fd is capable*, yet none of them do the master check.
*In the radv case, if acceleration is available.
Instead of duplicating the check in each driver, keep it where it's
needed and used.
Note this copies libdrm's drmIsMaster() to avoid depending on bleeding
edge version of the library.
v2: set the fd to -1 if not master (Bas)
Cc: Keith Packard
Cc: Jason Ekstrand
Cc: Bas Nieuwenhuizen
Cc: Andres Rodriguez
Reported-by: Andres Rodriguez
Fixes: da997ebec92 ("vulkan: Add KHR_display extension using DRM [v10]")
Signed-off-by: Emil Velikov
---
src/vulkan/wsi/wsi_common_display.c | 27 +++
1 file changed, 27 insertions(+)
diff --git a/src/vulkan/wsi/wsi_common_display.c
b/src/vulkan/wsi/wsi_common_display.c
index 74ed36ed646..2be20e85046 100644
--- a/src/vulkan/wsi/wsi_common_display.c
+++ b/src/vulkan/wsi/wsi_common_display.c
@@ -1812,6 +1812,30 @@ fail_attr_init:
return ret;
}
+
+/*
+ * Local version fo the libdrm helper. Added to avoid depending on bleeding
+ * edge version of the library.
+ */
+static int
+local_drmIsMaster(int fd)
+{
+ /* Detect master by attempting something that requires master.
+*
+* Authenticating magic tokens requires master and 0 is an
+* internal kernel detail which we could use. Attempting this on
+* a master fd would fail therefore fail with EINVAL because 0
+* is invalid.
+*
+* A non-master fd will fail with EACCES, as the kernel checks
+* for master before attempting to do anything else.
+*
+* Since we don't want to leak implementation details, use
+* EACCES.
+*/
+ return drmAuthMagic(fd, 0) != -EACCES;
+}
+
VkResult
wsi_display_init_wsi(struct wsi_device *wsi_device,
const VkAllocationCallbacks *alloc,
@@ -1827,6 +1851,9 @@ wsi_display_init_wsi(struct wsi_device *wsi_device,
}
wsi->fd = display_fd;
+ if (wsi->fd != -1 && !local_drmIsMaster(wsi->fd))
+ wsi->fd = -1;
+
wsi->alloc = alloc;
list_inithead(>connectors);
--
2.21.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
