[PATCH 1/5] drm/vmwgfx: move the require_exist handling together

[PATCH 1/5] drm/vmwgfx: move the require_exist handling together

[Emil Velikov]

From: Emil Velikov <emil.velikov@collabora.com>

Move the render_client hunk for require_exist alongside the rest.
Keeping all the reasons why an existing object is needed, in a single
place makes it easier to follow.

Cc: VMware Graphics <linux-graphics-maintainer@vmware.com>
Cc: Thomas Hellstrom <thellstrom@vmware.com>
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
---
 drivers/gpu/drm/vmwgfx/vmwgfx_surface.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
index 29d8794f0421..1f989f3605c8 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
@@ -909,16 +909,12 @@ vmw_surface_handle_reference(struct vmw_private *dev_priv,
 	uint32_t handle;
 	struct ttm_base_object *base;
 	int ret;
-	bool require_exist = false;
 
 	if (handle_type == DRM_VMW_HANDLE_PRIME) {
 		ret = ttm_prime_fd_to_handle(tfile, u_handle, &handle);
 		if (unlikely(ret != 0))
 			return ret;
 	} else {
-		if (unlikely(drm_is_render_client(file_priv)))
-			require_exist = true;
-
 		handle = u_handle;
 	}
 
@@ -935,6 +931,8 @@ vmw_surface_handle_reference(struct vmw_private *dev_priv,
 	}
 
 	if (handle_type != DRM_VMW_HANDLE_PRIME) {
+		bool require_exist = false;
+
 		user_srf = container_of(base, struct vmw_user_surface,
 					prime.base);
 
@@ -946,6 +944,9 @@ vmw_surface_handle_reference(struct vmw_private *dev_priv,
 		    user_srf->master != file_priv->master)
 			require_exist = true;
 
+		if (unlikely(drm_is_render_client(file_priv)))
+			require_exist = true;
+
 		ret = ttm_ref_object_add(tfile, base, TTM_REF_USAGE, NULL,
 					 require_exist);
 		if (unlikely(ret != 0)) {
-- 
2.23.0

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

[PATCH 2/5] drm/vmwgfx: check master authentication in surface_ref ioctls

[Emil Velikov]

From: Emil Velikov <emil.velikov@collabora.com>

With later commit we'll rework DRM authentication handling. Namely
DRM_AUTH will not be a requirement for DRM_RENDER_ALLOW ioctls.

Since vmwgfx does isolation for primary clients in different master
realms, the DRM_AUTH can be dropped.

The only place where authentication matters, is surface_reference ioctls
whenever a legacy (non-prime) handle is used. For those ioctls we call
vmw_surface_handle_reference(), where we explicitly check if the client
is both a) master and b) unauthenticated - bailing out as result.

Otherwise the usual isolation path kicks in and we're all good.

v2: Reword commit message, since the isolation work has landed.

Cc: VMware Graphics <linux-graphics-maintainer@vmware.com>
Cc: Thomas Hellstrom <thellstrom@vmware.com>
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
---
 drivers/gpu/drm/vmwgfx/vmwgfx_surface.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
index 1f989f3605c8..596e5c1bc2c1 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
@@ -936,6 +936,13 @@ vmw_surface_handle_reference(struct vmw_private *dev_priv,
 		user_srf = container_of(base, struct vmw_user_surface,
 					prime.base);
 
+		/* Error out if we are unauthenticated master */
+		if (drm_is_primary_client(file_priv) &&
+		    !file_priv->authenticated) {
+			ret = -EACCES;
+			goto out_bad_resource;
+		}
+
 		/*
 		 * Make sure the surface creator has the same
 		 * authenticating master, or is already registered with us.
-- 
2.23.0

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

Re: [PATCH 2/5] drm/vmwgfx: check master authentication in surface_ref ioctls

[Thomas Hellstrom]

On 11/1/19 2:05 PM, Emil Velikov wrote:
> From: Emil Velikov <emil.velikov@collabora.com>
>
> With later commit we'll rework DRM authentication handling. Namely
> DRM_AUTH will not be a requirement for DRM_RENDER_ALLOW ioctls.
>
> Since vmwgfx does isolation for primary clients in different master
> realms, the DRM_AUTH can be dropped.
>
> The only place where authentication matters, is surface_reference ioctls
> whenever a legacy (non-prime) handle is used. For those ioctls we call
> vmw_surface_handle_reference(), where we explicitly check if the client
> is both a) master and b) unauthenticated - bailing out as result.
>
> Otherwise the usual isolation path kicks in and we're all good.
>
> v2: Reword commit message, since the isolation work has landed.
>
> Cc: VMware Graphics <linux-graphics-maintainer@vmware.com>
> Cc: Thomas Hellstrom <thellstrom@vmware.com>
> Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
> ---
>  drivers/gpu/drm/vmwgfx/vmwgfx_surface.c | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
> index 1f989f3605c8..596e5c1bc2c1 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
> @@ -936,6 +936,13 @@ vmw_surface_handle_reference(struct vmw_private *dev_priv,
>  		user_srf = container_of(base, struct vmw_user_surface,
>  					prime.base);
>  
> +		/* Error out if we are unauthenticated master */

Shouldn't this be "Error out if we are unauthenticated primary" ?

Otherwise

Reviewed-by: Thomas Hellstrom <thellstrom@vmware.com>


> +		if (drm_is_primary_client(file_priv) &&
> +		    !file_priv->authenticated) {
> +			ret = -EACCES;
> +			goto out_bad_resource;
> +		}
> +
>  		/*
>  		 * Make sure the surface creator has the same
>  		 * authenticating master, or is already registered with us.


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

[PATCH 3/5] drm/vmwgfx: drop DRM_AUTH for render ioctls

[Emil Velikov]

From: Emil Velikov <emil.velikov@collabora.com>

With earlier commit 9c84aeba67cc ("drm/vmwgfx: Kill unneeded legacy
security features") we removed the no longer applicable validation, as
we now have isolation of primary clients from different master realms.

As of last commit, we're explicitly checking for authentication in the
only render ioctls which care about one.

With those in place, the DRM_AUTH token serves no real purpose. Let's
drop it.

Cc: VMware Graphics <linux-graphics-maintainer@vmware.com>
Cc: Thomas Hellstrom <thellstrom@vmware.com>
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
---
 drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
index 81a95651643f..253fae160175 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
@@ -165,9 +165,9 @@
 
 static const struct drm_ioctl_desc vmw_ioctls[] = {
 	VMW_IOCTL_DEF(VMW_GET_PARAM, vmw_getparam_ioctl,
-		      DRM_AUTH | DRM_RENDER_ALLOW),
+		      DRM_RENDER_ALLOW),
 	VMW_IOCTL_DEF(VMW_ALLOC_DMABUF, vmw_bo_alloc_ioctl,
-		      DRM_AUTH | DRM_RENDER_ALLOW),
+		      DRM_RENDER_ALLOW),
 	VMW_IOCTL_DEF(VMW_UNREF_DMABUF, vmw_bo_unref_ioctl,
 		      DRM_RENDER_ALLOW),
 	VMW_IOCTL_DEF(VMW_CURSOR_BYPASS,
@@ -182,16 +182,16 @@ static const struct drm_ioctl_desc vmw_ioctls[] = {
 		      DRM_MASTER),
 
 	VMW_IOCTL_DEF(VMW_CREATE_CONTEXT, vmw_context_define_ioctl,
-		      DRM_AUTH | DRM_RENDER_ALLOW),
+		      DRM_RENDER_ALLOW),
 	VMW_IOCTL_DEF(VMW_UNREF_CONTEXT, vmw_context_destroy_ioctl,
 		      DRM_RENDER_ALLOW),
 	VMW_IOCTL_DEF(VMW_CREATE_SURFACE, vmw_surface_define_ioctl,
-		      DRM_AUTH | DRM_RENDER_ALLOW),
+		      DRM_RENDER_ALLOW),
 	VMW_IOCTL_DEF(VMW_UNREF_SURFACE, vmw_surface_destroy_ioctl,
 		      DRM_RENDER_ALLOW),
 	VMW_IOCTL_DEF(VMW_REF_SURFACE, vmw_surface_reference_ioctl,
-		      DRM_AUTH | DRM_RENDER_ALLOW),
-	VMW_IOCTL_DEF(VMW_EXECBUF, vmw_execbuf_ioctl, DRM_AUTH |
+		      DRM_RENDER_ALLOW),
+	VMW_IOCTL_DEF(VMW_EXECBUF, vmw_execbuf_ioctl,
 		      DRM_RENDER_ALLOW),
 	VMW_IOCTL_DEF(VMW_FENCE_WAIT, vmw_fence_obj_wait_ioctl,
 		      DRM_RENDER_ALLOW),
@@ -201,9 +201,9 @@ static const struct drm_ioctl_desc vmw_ioctls[] = {
 	VMW_IOCTL_DEF(VMW_FENCE_UNREF, vmw_fence_obj_unref_ioctl,
 		      DRM_RENDER_ALLOW),
 	VMW_IOCTL_DEF(VMW_FENCE_EVENT, vmw_fence_event_ioctl,
-		      DRM_AUTH | DRM_RENDER_ALLOW),
+		      DRM_RENDER_ALLOW),
 	VMW_IOCTL_DEF(VMW_GET_3D_CAP, vmw_get_cap_3d_ioctl,
-		      DRM_AUTH | DRM_RENDER_ALLOW),
+		      DRM_RENDER_ALLOW),
 
 	/* these allow direct access to the framebuffers mark as master only */
 	VMW_IOCTL_DEF(VMW_PRESENT, vmw_present_ioctl,
@@ -221,28 +221,28 @@ static const struct drm_ioctl_desc vmw_ioctls[] = {
 		      DRM_RENDER_ALLOW),
 	VMW_IOCTL_DEF(VMW_CREATE_SHADER,
 		      vmw_shader_define_ioctl,
-		      DRM_AUTH | DRM_RENDER_ALLOW),
+		      DRM_RENDER_ALLOW),
 	VMW_IOCTL_DEF(VMW_UNREF_SHADER,
 		      vmw_shader_destroy_ioctl,
 		      DRM_RENDER_ALLOW),
 	VMW_IOCTL_DEF(VMW_GB_SURFACE_CREATE,
 		      vmw_gb_surface_define_ioctl,
-		      DRM_AUTH | DRM_RENDER_ALLOW),
+		      DRM_RENDER_ALLOW),
 	VMW_IOCTL_DEF(VMW_GB_SURFACE_REF,
 		      vmw_gb_surface_reference_ioctl,
-		      DRM_AUTH | DRM_RENDER_ALLOW),
+		      DRM_RENDER_ALLOW),
 	VMW_IOCTL_DEF(VMW_SYNCCPU,
 		      vmw_user_bo_synccpu_ioctl,
 		      DRM_RENDER_ALLOW),
 	VMW_IOCTL_DEF(VMW_CREATE_EXTENDED_CONTEXT,
 		      vmw_extended_context_define_ioctl,
-		      DRM_AUTH | DRM_RENDER_ALLOW),
+		      DRM_RENDER_ALLOW),
 	VMW_IOCTL_DEF(VMW_GB_SURFACE_CREATE_EXT,
 		      vmw_gb_surface_define_ext_ioctl,
-		      DRM_AUTH | DRM_RENDER_ALLOW),
+		      DRM_RENDER_ALLOW),
 	VMW_IOCTL_DEF(VMW_GB_SURFACE_REF_EXT,
 		      vmw_gb_surface_reference_ext_ioctl,
-		      DRM_AUTH | DRM_RENDER_ALLOW),
+		      DRM_RENDER_ALLOW),
 };
 
 static const struct pci_device_id vmw_pci_id_list[] = {
-- 
2.23.0

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

Re: [PATCH 3/5] drm/vmwgfx: drop DRM_AUTH for render ioctls

[Thomas Hellstrom]

On 11/1/19 2:05 PM, Emil Velikov wrote:
> From: Emil Velikov <emil.velikov@collabora.com>
>
> With earlier commit 9c84aeba67cc ("drm/vmwgfx: Kill unneeded legacy
> security features") we removed the no longer applicable validation, as
> we now have isolation of primary clients from different master realms.
>
> As of last commit, we're explicitly checking for authentication in the
> only render ioctls which care about one.
>
> With those in place, the DRM_AUTH token serves no real purpose. Let's
> drop it.
>
> Cc: VMware Graphics <linux-graphics-maintainer@vmware.com>
> Cc: Thomas Hellstrom <thellstrom@vmware.com>
> Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
> ---
>  drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 28 ++++++++++++++--------------
>  1 file changed, 14 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> index 81a95651643f..253fae160175 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> @@ -165,9 +165,9 @@
>  
>  static const struct drm_ioctl_desc vmw_ioctls[] = {
>  	VMW_IOCTL_DEF(VMW_GET_PARAM, vmw_getparam_ioctl,
> -		      DRM_AUTH | DRM_RENDER_ALLOW),
> +		      DRM_RENDER_ALLOW),
>  	VMW_IOCTL_DEF(VMW_ALLOC_DMABUF, vmw_bo_alloc_ioctl,
> -		      DRM_AUTH | DRM_RENDER_ALLOW),
> +		      DRM_RENDER_ALLOW),
>  	VMW_IOCTL_DEF(VMW_UNREF_DMABUF, vmw_bo_unref_ioctl,
>  		      DRM_RENDER_ALLOW),
>  	VMW_IOCTL_DEF(VMW_CURSOR_BYPASS,
> @@ -182,16 +182,16 @@ static const struct drm_ioctl_desc vmw_ioctls[] = {
>  		      DRM_MASTER),
>  
>  	VMW_IOCTL_DEF(VMW_CREATE_CONTEXT, vmw_context_define_ioctl,
> -		      DRM_AUTH | DRM_RENDER_ALLOW),
> +		      DRM_RENDER_ALLOW),
>  	VMW_IOCTL_DEF(VMW_UNREF_CONTEXT, vmw_context_destroy_ioctl,
>  		      DRM_RENDER_ALLOW),
>  	VMW_IOCTL_DEF(VMW_CREATE_SURFACE, vmw_surface_define_ioctl,
> -		      DRM_AUTH | DRM_RENDER_ALLOW),
> +		      DRM_RENDER_ALLOW),
>  	VMW_IOCTL_DEF(VMW_UNREF_SURFACE, vmw_surface_destroy_ioctl,
>  		      DRM_RENDER_ALLOW),
>  	VMW_IOCTL_DEF(VMW_REF_SURFACE, vmw_surface_reference_ioctl,
> -		      DRM_AUTH | DRM_RENDER_ALLOW),
> -	VMW_IOCTL_DEF(VMW_EXECBUF, vmw_execbuf_ioctl, DRM_AUTH |
> +		      DRM_RENDER_ALLOW),
> +	VMW_IOCTL_DEF(VMW_EXECBUF, vmw_execbuf_ioctl,
>  		      DRM_RENDER_ALLOW),
>  	VMW_IOCTL_DEF(VMW_FENCE_WAIT, vmw_fence_obj_wait_ioctl,
>  		      DRM_RENDER_ALLOW),
> @@ -201,9 +201,9 @@ static const struct drm_ioctl_desc vmw_ioctls[] = {
>  	VMW_IOCTL_DEF(VMW_FENCE_UNREF, vmw_fence_obj_unref_ioctl,
>  		      DRM_RENDER_ALLOW),
>  	VMW_IOCTL_DEF(VMW_FENCE_EVENT, vmw_fence_event_ioctl,
> -		      DRM_AUTH | DRM_RENDER_ALLOW),
> +		      DRM_RENDER_ALLOW),
>  	VMW_IOCTL_DEF(VMW_GET_3D_CAP, vmw_get_cap_3d_ioctl,
> -		      DRM_AUTH | DRM_RENDER_ALLOW),
> +		      DRM_RENDER_ALLOW),
>  
>  	/* these allow direct access to the framebuffers mark as master only */
>  	VMW_IOCTL_DEF(VMW_PRESENT, vmw_present_ioctl,
> @@ -221,28 +221,28 @@ static const struct drm_ioctl_desc vmw_ioctls[] = {
>  		      DRM_RENDER_ALLOW),
>  	VMW_IOCTL_DEF(VMW_CREATE_SHADER,
>  		      vmw_shader_define_ioctl,
> -		      DRM_AUTH | DRM_RENDER_ALLOW),
> +		      DRM_RENDER_ALLOW),
>  	VMW_IOCTL_DEF(VMW_UNREF_SHADER,
>  		      vmw_shader_destroy_ioctl,
>  		      DRM_RENDER_ALLOW),
>  	VMW_IOCTL_DEF(VMW_GB_SURFACE_CREATE,
>  		      vmw_gb_surface_define_ioctl,
> -		      DRM_AUTH | DRM_RENDER_ALLOW),
> +		      DRM_RENDER_ALLOW),
>  	VMW_IOCTL_DEF(VMW_GB_SURFACE_REF,
>  		      vmw_gb_surface_reference_ioctl,
> -		      DRM_AUTH | DRM_RENDER_ALLOW),
> +		      DRM_RENDER_ALLOW),
>  	VMW_IOCTL_DEF(VMW_SYNCCPU,
>  		      vmw_user_bo_synccpu_ioctl,
>  		      DRM_RENDER_ALLOW),
>  	VMW_IOCTL_DEF(VMW_CREATE_EXTENDED_CONTEXT,
>  		      vmw_extended_context_define_ioctl,
> -		      DRM_AUTH | DRM_RENDER_ALLOW),
> +		      DRM_RENDER_ALLOW),
>  	VMW_IOCTL_DEF(VMW_GB_SURFACE_CREATE_EXT,
>  		      vmw_gb_surface_define_ext_ioctl,
> -		      DRM_AUTH | DRM_RENDER_ALLOW),
> +		      DRM_RENDER_ALLOW),
>  	VMW_IOCTL_DEF(VMW_GB_SURFACE_REF_EXT,
>  		      vmw_gb_surface_reference_ext_ioctl,
> -		      DRM_AUTH | DRM_RENDER_ALLOW),
> +		      DRM_RENDER_ALLOW),
>  };
>  
>  static const struct pci_device_id vmw_pci_id_list[] = {

Reviewed-by: Thomas Hellstrom <thellstrom@vmware.com>

Do you want me to take this through the vmwgfx tree?

Thanks,

Thomas



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

Re: [PATCH 3/5] drm/vmwgfx: drop DRM_AUTH for render ioctls

[Emil Velikov]

On Tue, 12 Nov 2019 at 12:54, Thomas Hellstrom <thellstrom@vmware.com> wrote:
>
> On 11/1/19 2:05 PM, Emil Velikov wrote:
> > From: Emil Velikov <emil.velikov@collabora.com>
> >
> > With earlier commit 9c84aeba67cc ("drm/vmwgfx: Kill unneeded legacy
> > security features") we removed the no longer applicable validation, as
> > we now have isolation of primary clients from different master realms.
> >
> > As of last commit, we're explicitly checking for authentication in the
> > only render ioctls which care about one.
> >
> > With those in place, the DRM_AUTH token serves no real purpose. Let's
> > drop it.
> >
> > Cc: VMware Graphics <linux-graphics-maintainer@vmware.com>
> > Cc: Thomas Hellstrom <thellstrom@vmware.com>
> > Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
> > ---
> >  drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 28 ++++++++++++++--------------
> >  1 file changed, 14 insertions(+), 14 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> > index 81a95651643f..253fae160175 100644
> > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> > @@ -165,9 +165,9 @@
> >
> >  static const struct drm_ioctl_desc vmw_ioctls[] = {
> >       VMW_IOCTL_DEF(VMW_GET_PARAM, vmw_getparam_ioctl,
> > -                   DRM_AUTH | DRM_RENDER_ALLOW),
> > +                   DRM_RENDER_ALLOW),
> >       VMW_IOCTL_DEF(VMW_ALLOC_DMABUF, vmw_bo_alloc_ioctl,
> > -                   DRM_AUTH | DRM_RENDER_ALLOW),
> > +                   DRM_RENDER_ALLOW),
> >       VMW_IOCTL_DEF(VMW_UNREF_DMABUF, vmw_bo_unref_ioctl,
> >                     DRM_RENDER_ALLOW),
> >       VMW_IOCTL_DEF(VMW_CURSOR_BYPASS,
> > @@ -182,16 +182,16 @@ static const struct drm_ioctl_desc vmw_ioctls[] = {
> >                     DRM_MASTER),
> >
> >       VMW_IOCTL_DEF(VMW_CREATE_CONTEXT, vmw_context_define_ioctl,
> > -                   DRM_AUTH | DRM_RENDER_ALLOW),
> > +                   DRM_RENDER_ALLOW),
> >       VMW_IOCTL_DEF(VMW_UNREF_CONTEXT, vmw_context_destroy_ioctl,
> >                     DRM_RENDER_ALLOW),
> >       VMW_IOCTL_DEF(VMW_CREATE_SURFACE, vmw_surface_define_ioctl,
> > -                   DRM_AUTH | DRM_RENDER_ALLOW),
> > +                   DRM_RENDER_ALLOW),
> >       VMW_IOCTL_DEF(VMW_UNREF_SURFACE, vmw_surface_destroy_ioctl,
> >                     DRM_RENDER_ALLOW),
> >       VMW_IOCTL_DEF(VMW_REF_SURFACE, vmw_surface_reference_ioctl,
> > -                   DRM_AUTH | DRM_RENDER_ALLOW),
> > -     VMW_IOCTL_DEF(VMW_EXECBUF, vmw_execbuf_ioctl, DRM_AUTH |
> > +                   DRM_RENDER_ALLOW),
> > +     VMW_IOCTL_DEF(VMW_EXECBUF, vmw_execbuf_ioctl,
> >                     DRM_RENDER_ALLOW),
> >       VMW_IOCTL_DEF(VMW_FENCE_WAIT, vmw_fence_obj_wait_ioctl,
> >                     DRM_RENDER_ALLOW),
> > @@ -201,9 +201,9 @@ static const struct drm_ioctl_desc vmw_ioctls[] = {
> >       VMW_IOCTL_DEF(VMW_FENCE_UNREF, vmw_fence_obj_unref_ioctl,
> >                     DRM_RENDER_ALLOW),
> >       VMW_IOCTL_DEF(VMW_FENCE_EVENT, vmw_fence_event_ioctl,
> > -                   DRM_AUTH | DRM_RENDER_ALLOW),
> > +                   DRM_RENDER_ALLOW),
> >       VMW_IOCTL_DEF(VMW_GET_3D_CAP, vmw_get_cap_3d_ioctl,
> > -                   DRM_AUTH | DRM_RENDER_ALLOW),
> > +                   DRM_RENDER_ALLOW),
> >
> >       /* these allow direct access to the framebuffers mark as master only */
> >       VMW_IOCTL_DEF(VMW_PRESENT, vmw_present_ioctl,
> > @@ -221,28 +221,28 @@ static const struct drm_ioctl_desc vmw_ioctls[] = {
> >                     DRM_RENDER_ALLOW),
> >       VMW_IOCTL_DEF(VMW_CREATE_SHADER,
> >                     vmw_shader_define_ioctl,
> > -                   DRM_AUTH | DRM_RENDER_ALLOW),
> > +                   DRM_RENDER_ALLOW),
> >       VMW_IOCTL_DEF(VMW_UNREF_SHADER,
> >                     vmw_shader_destroy_ioctl,
> >                     DRM_RENDER_ALLOW),
> >       VMW_IOCTL_DEF(VMW_GB_SURFACE_CREATE,
> >                     vmw_gb_surface_define_ioctl,
> > -                   DRM_AUTH | DRM_RENDER_ALLOW),
> > +                   DRM_RENDER_ALLOW),
> >       VMW_IOCTL_DEF(VMW_GB_SURFACE_REF,
> >                     vmw_gb_surface_reference_ioctl,
> > -                   DRM_AUTH | DRM_RENDER_ALLOW),
> > +                   DRM_RENDER_ALLOW),
> >       VMW_IOCTL_DEF(VMW_SYNCCPU,
> >                     vmw_user_bo_synccpu_ioctl,
> >                     DRM_RENDER_ALLOW),
> >       VMW_IOCTL_DEF(VMW_CREATE_EXTENDED_CONTEXT,
> >                     vmw_extended_context_define_ioctl,
> > -                   DRM_AUTH | DRM_RENDER_ALLOW),
> > +                   DRM_RENDER_ALLOW),
> >       VMW_IOCTL_DEF(VMW_GB_SURFACE_CREATE_EXT,
> >                     vmw_gb_surface_define_ext_ioctl,
> > -                   DRM_AUTH | DRM_RENDER_ALLOW),
> > +                   DRM_RENDER_ALLOW),
> >       VMW_IOCTL_DEF(VMW_GB_SURFACE_REF_EXT,
> >                     vmw_gb_surface_reference_ext_ioctl,
> > -                   DRM_AUTH | DRM_RENDER_ALLOW),
> > +                   DRM_RENDER_ALLOW),
> >  };
> >
> >  static const struct pci_device_id vmw_pci_id_list[] = {
>
> Reviewed-by: Thomas Hellstrom <thellstrom@vmware.com>
>
Great thanks.

> Do you want me to take this through the vmwgfx tree?
>
Getting the patches through your tree sounds better. This way the
VMware team can do extra testing, just in case :-)
I'd imagine you will squash the typo in 2/5 when applying, correct?

May i interest you in reviewing the final patch of this series [1]?

Thanks
Emil
[1] https://patchwork.freedesktop.org/patch/338585/?series=68863&rev=1
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 4/5] drm/panfrost: remove DRM_AUTH and respective comment

[Emil Velikov]

From: Emil Velikov <emil.velikov@collabora.com>

As of earlier commit we have address space separation. Yet we forgot to
remove the respective comment and DRM_AUTH in the ioctl declaration.

Cc: Tomeu Vizoso <tomeu.vizoso@collabora.com>
Cc: David Airlie <airlied@linux.ie>
Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: Robin Murphy <robin.murphy@arm.com>
Cc: Steven Price <steven.price@arm.com>
Fixes: 7282f7645d06 ("drm/panfrost: Implement per FD address spaces")
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
---
 drivers/gpu/drm/panfrost/panfrost_drv.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c
index bc2ddeb55f5d..c677b2c9e409 100644
--- a/drivers/gpu/drm/panfrost/panfrost_drv.c
+++ b/drivers/gpu/drm/panfrost/panfrost_drv.c
@@ -451,15 +451,11 @@ panfrost_postclose(struct drm_device *dev, struct drm_file *file)
 	kfree(panfrost_priv);
 }
 
-/* DRM_AUTH is required on SUBMIT for now, while all clients share a single
- * address space.  Note that render nodes would be able to submit jobs that
- * could access BOs from clients authenticated with the master node.
- */
 static const struct drm_ioctl_desc panfrost_drm_driver_ioctls[] = {
 #define PANFROST_IOCTL(n, func, flags) \
 	DRM_IOCTL_DEF_DRV(PANFROST_##n, panfrost_ioctl_##func, flags)
 
-	PANFROST_IOCTL(SUBMIT,		submit,		DRM_RENDER_ALLOW | DRM_AUTH),
+	PANFROST_IOCTL(SUBMIT,		submit,		DRM_RENDER_ALLOW),
 	PANFROST_IOCTL(WAIT_BO,		wait_bo,	DRM_RENDER_ALLOW),
 	PANFROST_IOCTL(CREATE_BO,	create_bo,	DRM_RENDER_ALLOW),
 	PANFROST_IOCTL(MMAP_BO,		mmap_bo,	DRM_RENDER_ALLOW),
-- 
2.23.0

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

Re: [PATCH 4/5] drm/panfrost: remove DRM_AUTH and respective comment

[Steven Price]

On 01/11/2019 13:03, Emil Velikov wrote:
> From: Emil Velikov <emil.velikov@collabora.com>
> 
> As of earlier commit we have address space separation. Yet we forgot to
> remove the respective comment and DRM_AUTH in the ioctl declaration.
> 
> Cc: Tomeu Vizoso <tomeu.vizoso@collabora.com>
> Cc: David Airlie <airlied@linux.ie>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Cc: Robin Murphy <robin.murphy@arm.com>
> Cc: Steven Price <steven.price@arm.com>
> Fixes: 7282f7645d06 ("drm/panfrost: Implement per FD address spaces")
> Signed-off-by: Emil Velikov <emil.velikov@collabora.com>

Reviewed-by: Steven Price <steven.price@arm.com>

I'm not sure DRM_AUTH provided us with much in the first place (because
render nodes could snoop/affect the primary node), but since we have
address space separation it's clearly not required now.

Steve

> ---
>  drivers/gpu/drm/panfrost/panfrost_drv.c | 6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c
> index bc2ddeb55f5d..c677b2c9e409 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_drv.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c
> @@ -451,15 +451,11 @@ panfrost_postclose(struct drm_device *dev, struct drm_file *file)
>  	kfree(panfrost_priv);
>  }
>  
> -/* DRM_AUTH is required on SUBMIT for now, while all clients share a single
> - * address space.  Note that render nodes would be able to submit jobs that
> - * could access BOs from clients authenticated with the master node.
> - */
>  static const struct drm_ioctl_desc panfrost_drm_driver_ioctls[] = {
>  #define PANFROST_IOCTL(n, func, flags) \
>  	DRM_IOCTL_DEF_DRV(PANFROST_##n, panfrost_ioctl_##func, flags)
>  
> -	PANFROST_IOCTL(SUBMIT,		submit,		DRM_RENDER_ALLOW | DRM_AUTH),
> +	PANFROST_IOCTL(SUBMIT,		submit,		DRM_RENDER_ALLOW),
>  	PANFROST_IOCTL(WAIT_BO,		wait_bo,	DRM_RENDER_ALLOW),
>  	PANFROST_IOCTL(CREATE_BO,	create_bo,	DRM_RENDER_ALLOW),
>  	PANFROST_IOCTL(MMAP_BO,		mmap_bo,	DRM_RENDER_ALLOW),
> 

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

Re: [PATCH 4/5] drm/panfrost: remove DRM_AUTH and respective comment

[Emil Velikov]

On Fri, 1 Nov 2019 at 13:34, Steven Price <steven.price@arm.com> wrote:
>
> On 01/11/2019 13:03, Emil Velikov wrote:
> > From: Emil Velikov <emil.velikov@collabora.com>
> >
> > As of earlier commit we have address space separation. Yet we forgot to
> > remove the respective comment and DRM_AUTH in the ioctl declaration.
> >
> > Cc: Tomeu Vizoso <tomeu.vizoso@collabora.com>
> > Cc: David Airlie <airlied@linux.ie>
> > Cc: Daniel Vetter <daniel@ffwll.ch>
> > Cc: Robin Murphy <robin.murphy@arm.com>
> > Cc: Steven Price <steven.price@arm.com>
> > Fixes: 7282f7645d06 ("drm/panfrost: Implement per FD address spaces")
> > Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
>
> Reviewed-by: Steven Price <steven.price@arm.com>
>
> I'm not sure DRM_AUTH provided us with much in the first place (because
> render nodes could snoop/affect the primary node), but since we have
> address space separation it's clearly not required now.
>
Thanks Steve. This is exactly the reason why I removed it from most
other drivers.
There are equivalent vmwgfx changes and a DRM core patch in this series.

Do you think you'll have some time to check those over? Would be
amazing if I can apply the lot in one go to drm-misc.

Thanks
Emil
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 4/5] drm/panfrost: remove DRM_AUTH and respective comment

[Steven Price]

On 08/11/2019 13:10, Emil Velikov wrote:
> On Fri, 1 Nov 2019 at 13:34, Steven Price <steven.price@arm.com> wrote:
>>
>> On 01/11/2019 13:03, Emil Velikov wrote:
>>> From: Emil Velikov <emil.velikov@collabora.com>
>>>
>>> As of earlier commit we have address space separation. Yet we forgot to
>>> remove the respective comment and DRM_AUTH in the ioctl declaration.
>>>
>>> Cc: Tomeu Vizoso <tomeu.vizoso@collabora.com>
>>> Cc: David Airlie <airlied@linux.ie>
>>> Cc: Daniel Vetter <daniel@ffwll.ch>
>>> Cc: Robin Murphy <robin.murphy@arm.com>
>>> Cc: Steven Price <steven.price@arm.com>
>>> Fixes: 7282f7645d06 ("drm/panfrost: Implement per FD address spaces")
>>> Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
>>
>> Reviewed-by: Steven Price <steven.price@arm.com>
>>
>> I'm not sure DRM_AUTH provided us with much in the first place (because
>> render nodes could snoop/affect the primary node), but since we have
>> address space separation it's clearly not required now.
>>
> Thanks Steve. This is exactly the reason why I removed it from most
> other drivers.
> There are equivalent vmwgfx changes and a DRM core patch in this series.
> 
> Do you think you'll have some time to check those over? Would be
> amazing if I can apply the lot in one go to drm-misc.

I'm afraid I don't know enough about the security model of vmwgfx to
meaningfully comment on those changes. On the surface they look fine,
but it really needs someone who understands whether this exposes an
attack surface.

The DRM core patch concerns me slightly (although again I'm not
completely up to speed on the security mode here). For a device which
doesn't have address space separation (and doesn't support render
nodes), is there anything stopping a process which hasn't authenticated
converting another process's handle to a prime fd? (or injecting dmabufs
into the address space used by the authenticated process - which might
cause address space exhaustion). If that's not a concern then I'm not
sure why the ioctls were originally added with DRM_AUTH...

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

Re: [PATCH 4/5] drm/panfrost: remove DRM_AUTH and respective comment

[Emil Velikov]

On Fri, 8 Nov 2019 at 15:55, Steven Price <steven.price@arm.com> wrote:
>
> On 08/11/2019 13:10, Emil Velikov wrote:
> > On Fri, 1 Nov 2019 at 13:34, Steven Price <steven.price@arm.com> wrote:
> >>
> >> On 01/11/2019 13:03, Emil Velikov wrote:
> >>> From: Emil Velikov <emil.velikov@collabora.com>
> >>>
> >>> As of earlier commit we have address space separation. Yet we forgot to
> >>> remove the respective comment and DRM_AUTH in the ioctl declaration.
> >>>
> >>> Cc: Tomeu Vizoso <tomeu.vizoso@collabora.com>
> >>> Cc: David Airlie <airlied@linux.ie>
> >>> Cc: Daniel Vetter <daniel@ffwll.ch>
> >>> Cc: Robin Murphy <robin.murphy@arm.com>
> >>> Cc: Steven Price <steven.price@arm.com>
> >>> Fixes: 7282f7645d06 ("drm/panfrost: Implement per FD address spaces")
> >>> Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
> >>
> >> Reviewed-by: Steven Price <steven.price@arm.com>
> >>
> >> I'm not sure DRM_AUTH provided us with much in the first place (because
> >> render nodes could snoop/affect the primary node), but since we have
> >> address space separation it's clearly not required now.
> >>
> > Thanks Steve. This is exactly the reason why I removed it from most
> > other drivers.
> > There are equivalent vmwgfx changes and a DRM core patch in this series.
> >
> > Do you think you'll have some time to check those over? Would be
> > amazing if I can apply the lot in one go to drm-misc.
>
> I'm afraid I don't know enough about the security model of vmwgfx to
> meaningfully comment on those changes. On the surface they look fine,
> but it really needs someone who understands whether this exposes an
> attack surface.
>
I understand, thanks for having a look.

> The DRM core patch concerns me slightly (although again I'm not
> completely up to speed on the security mode here). For a device which
> doesn't have address space separation (and doesn't support render
> nodes), is there anything stopping a process which hasn't authenticated
> converting another process's handle to a prime fd? (or injecting dmabufs
> into the address space used by the authenticated process - which might
> cause address space exhaustion). If that's not a concern then I'm not
> sure why the ioctls were originally added with DRM_AUTH...
>
Thanks for raising this up.

Let's start with the short question: Why was DRM_AUTH added?
I would expect either cargo-cult - we have DRM_AUTH even for get_param ioctl.

In order for a handle to be exported as fd, the driver must support
render nodes. Which implicitly mandates address space separation.
If that assumption is broken, then we have deeper problems.
On the other hand, V3D does expose render nodes and uses the same
quirk as panfrost before the address space separation patch.

So overall, it doesn't seem like it would cause any problems, which
don't exist already.

Thanks
Emil
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 5/5] drm: drop DRM_AUTH from PRIME_TO/FROM_HANDLE ioctls

[Emil Velikov]

From: Emil Velikov <emil.velikov@collabora.com>

As mentioned by Christian, for drivers which support only primary nodes
this changes the returned error from -EACCES into -EOPNOTSUPP/-ENOSYS.

For others, this check in particular will be a noop. So let's remove it
as suggested by Christian.

Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: amd-gfx@lists.freedesktop.org
Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: Sean Paul <sean@poorly.run>
Acked-by: Christian König <christian.koenig@amd.com>
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
---
 drivers/gpu/drm/drm_ioctl.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index fcd728d7cf72..5afb39688b55 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -652,8 +652,8 @@ static const struct drm_ioctl_desc drm_ioctls[] = {
 
 	DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETRESOURCES, drm_mode_getresources, 0),
 
-	DRM_IOCTL_DEF(DRM_IOCTL_PRIME_HANDLE_TO_FD, drm_prime_handle_to_fd_ioctl, DRM_AUTH|DRM_RENDER_ALLOW),
-	DRM_IOCTL_DEF(DRM_IOCTL_PRIME_FD_TO_HANDLE, drm_prime_fd_to_handle_ioctl, DRM_AUTH|DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF(DRM_IOCTL_PRIME_HANDLE_TO_FD, drm_prime_handle_to_fd_ioctl, DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF(DRM_IOCTL_PRIME_FD_TO_HANDLE, drm_prime_fd_to_handle_ioctl, DRM_RENDER_ALLOW),
 
 	DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETPLANERESOURCES, drm_mode_getplane_res, 0),
 	DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETCRTC, drm_mode_getcrtc, 0),
-- 
2.23.0

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

[PATCH 5/5] drm: drop DRM_AUTH from PRIME_TO/FROM_HANDLE ioctls

[Emil Velikov]

From: Emil Velikov <emil.velikov@collabora.com>

As mentioned by Christian, for drivers which support only primary nodes
this changes the returned error from -EACCES into -EOPNOTSUPP/-ENOSYS.

For others, this check in particular will be a noop. So let's remove it
as suggested by Christian.

Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: amd-gfx@lists.freedesktop.org
Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: Sean Paul <sean@poorly.run>
Acked-by: Christian König <christian.koenig@amd.com>
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
---
 drivers/gpu/drm/drm_ioctl.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index fcd728d7cf72..5afb39688b55 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -652,8 +652,8 @@ static const struct drm_ioctl_desc drm_ioctls[] = {
 
 	DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETRESOURCES, drm_mode_getresources, 0),
 
-	DRM_IOCTL_DEF(DRM_IOCTL_PRIME_HANDLE_TO_FD, drm_prime_handle_to_fd_ioctl, DRM_AUTH|DRM_RENDER_ALLOW),
-	DRM_IOCTL_DEF(DRM_IOCTL_PRIME_FD_TO_HANDLE, drm_prime_fd_to_handle_ioctl, DRM_AUTH|DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF(DRM_IOCTL_PRIME_HANDLE_TO_FD, drm_prime_handle_to_fd_ioctl, DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF(DRM_IOCTL_PRIME_FD_TO_HANDLE, drm_prime_fd_to_handle_ioctl, DRM_RENDER_ALLOW),
 
 	DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETPLANERESOURCES, drm_mode_getplane_res, 0),
 	DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETCRTC, drm_mode_getcrtc, 0),
-- 
2.23.0

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 5/5] drm: drop DRM_AUTH from PRIME_TO/FROM_HANDLE ioctls

[Emil Velikov]

On Fri, 1 Nov 2019 at 13:05, Emil Velikov <emil.l.velikov@gmail.com> wrote:
>
> From: Emil Velikov <emil.velikov@collabora.com>
>
> As mentioned by Christian, for drivers which support only primary nodes
> this changes the returned error from -EACCES into -EOPNOTSUPP/-ENOSYS.
>
> For others, this check in particular will be a noop. So let's remove it
> as suggested by Christian.
>
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Cc: amd-gfx@lists.freedesktop.org
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Cc: Sean Paul <sean@poorly.run>
> Acked-by: Christian König <christian.koenig@amd.com>
> Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
> ---
>  drivers/gpu/drm/drm_ioctl.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
> index fcd728d7cf72..5afb39688b55 100644
> --- a/drivers/gpu/drm/drm_ioctl.c
> +++ b/drivers/gpu/drm/drm_ioctl.c
> @@ -652,8 +652,8 @@ static const struct drm_ioctl_desc drm_ioctls[] = {
>
>         DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETRESOURCES, drm_mode_getresources, 0),
>
> -       DRM_IOCTL_DEF(DRM_IOCTL_PRIME_HANDLE_TO_FD, drm_prime_handle_to_fd_ioctl, DRM_AUTH|DRM_RENDER_ALLOW),
> -       DRM_IOCTL_DEF(DRM_IOCTL_PRIME_FD_TO_HANDLE, drm_prime_fd_to_handle_ioctl, DRM_AUTH|DRM_RENDER_ALLOW),
> +       DRM_IOCTL_DEF(DRM_IOCTL_PRIME_HANDLE_TO_FD, drm_prime_handle_to_fd_ioctl, DRM_RENDER_ALLOW),
> +       DRM_IOCTL_DEF(DRM_IOCTL_PRIME_FD_TO_HANDLE, drm_prime_fd_to_handle_ioctl, DRM_RENDER_ALLOW),
>
>         DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETPLANERESOURCES, drm_mode_getplane_res, 0),
>         DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETCRTC, drm_mode_getcrtc, 0),
> --
Humble poke? Sean, others?

Thanks
Emil
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 5/5] drm: drop DRM_AUTH from PRIME_TO/FROM_HANDLE ioctls

[Emil Velikov]

On Fri, 1 Nov 2019 at 13:05, Emil Velikov <emil.l.velikov@gmail.com> wrote:
>
> From: Emil Velikov <emil.velikov@collabora.com>
>
> As mentioned by Christian, for drivers which support only primary nodes
> this changes the returned error from -EACCES into -EOPNOTSUPP/-ENOSYS.
>
> For others, this check in particular will be a noop. So let's remove it
> as suggested by Christian.
>
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Cc: amd-gfx@lists.freedesktop.org
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Cc: Sean Paul <sean@poorly.run>
> Acked-by: Christian König <christian.koenig@amd.com>
> Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
> ---
>  drivers/gpu/drm/drm_ioctl.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
> index fcd728d7cf72..5afb39688b55 100644
> --- a/drivers/gpu/drm/drm_ioctl.c
> +++ b/drivers/gpu/drm/drm_ioctl.c
> @@ -652,8 +652,8 @@ static const struct drm_ioctl_desc drm_ioctls[] = {
>
>         DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETRESOURCES, drm_mode_getresources, 0),
>
> -       DRM_IOCTL_DEF(DRM_IOCTL_PRIME_HANDLE_TO_FD, drm_prime_handle_to_fd_ioctl, DRM_AUTH|DRM_RENDER_ALLOW),
> -       DRM_IOCTL_DEF(DRM_IOCTL_PRIME_FD_TO_HANDLE, drm_prime_fd_to_handle_ioctl, DRM_AUTH|DRM_RENDER_ALLOW),
> +       DRM_IOCTL_DEF(DRM_IOCTL_PRIME_HANDLE_TO_FD, drm_prime_handle_to_fd_ioctl, DRM_RENDER_ALLOW),
> +       DRM_IOCTL_DEF(DRM_IOCTL_PRIME_FD_TO_HANDLE, drm_prime_fd_to_handle_ioctl, DRM_RENDER_ALLOW),
>
>         DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETPLANERESOURCES, drm_mode_getplane_res, 0),
>         DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETCRTC, drm_mode_getcrtc, 0),
> --
Humble poke? Sean, others?

Thanks
Emil
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 5/5] drm: drop DRM_AUTH from PRIME_TO/FROM_HANDLE ioctls

[Emil Velikov]

On Fri, 1 Nov 2019 at 13:05, Emil Velikov <emil.l.velikov@gmail.com> wrote:
>
> From: Emil Velikov <emil.velikov@collabora.com>
>
> As mentioned by Christian, for drivers which support only primary nodes
> this changes the returned error from -EACCES into -EOPNOTSUPP/-ENOSYS.
>
> For others, this check in particular will be a noop. So let's remove it
> as suggested by Christian.
>
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Cc: amd-gfx@lists.freedesktop.org
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Cc: Sean Paul <sean@poorly.run>
> Acked-by: Christian König <christian.koenig@amd.com>
> Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
> ---
>  drivers/gpu/drm/drm_ioctl.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
> index fcd728d7cf72..5afb39688b55 100644
> --- a/drivers/gpu/drm/drm_ioctl.c
> +++ b/drivers/gpu/drm/drm_ioctl.c
> @@ -652,8 +652,8 @@ static const struct drm_ioctl_desc drm_ioctls[] = {
>
>         DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETRESOURCES, drm_mode_getresources, 0),
>
> -       DRM_IOCTL_DEF(DRM_IOCTL_PRIME_HANDLE_TO_FD, drm_prime_handle_to_fd_ioctl, DRM_AUTH|DRM_RENDER_ALLOW),
> -       DRM_IOCTL_DEF(DRM_IOCTL_PRIME_FD_TO_HANDLE, drm_prime_fd_to_handle_ioctl, DRM_AUTH|DRM_RENDER_ALLOW),
> +       DRM_IOCTL_DEF(DRM_IOCTL_PRIME_HANDLE_TO_FD, drm_prime_handle_to_fd_ioctl, DRM_RENDER_ALLOW),
> +       DRM_IOCTL_DEF(DRM_IOCTL_PRIME_FD_TO_HANDLE, drm_prime_fd_to_handle_ioctl, DRM_RENDER_ALLOW),
>
>         DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETPLANERESOURCES, drm_mode_getplane_res, 0),
>         DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETCRTC, drm_mode_getcrtc, 0),
> --
Humble poke? Sean, others?

Thanks
Emil
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 5/5] drm: drop DRM_AUTH from PRIME_TO/FROM_HANDLE ioctls

[Boris Brezillon]

Hi Emil,

On Fri,  1 Nov 2019 13:03:13 +0000
Emil Velikov <emil.l.velikov@gmail.com> wrote:

> From: Emil Velikov <emil.velikov@collabora.com>
> 
> As mentioned by Christian, for drivers which support only primary nodes
> this changes the returned error from -EACCES into -EOPNOTSUPP/-ENOSYS.

Are you sure this is true for MODESET-only nodes (those that do not
have the RENDER cap set) implementing ->{fd_to_handle,handle_to_fd}()?
Shouldn't the is_authenticated() check still be done in that case?

Regards,

Boris

> 
> For others, this check in particular will be a noop. So let's remove it
> as suggested by Christian.
> 
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Cc: amd-gfx@lists.freedesktop.org
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Cc: Sean Paul <sean@poorly.run>
> Acked-by: Christian König <christian.koenig@amd.com>
> Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
> ---
>  drivers/gpu/drm/drm_ioctl.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
> index fcd728d7cf72..5afb39688b55 100644
> --- a/drivers/gpu/drm/drm_ioctl.c
> +++ b/drivers/gpu/drm/drm_ioctl.c
> @@ -652,8 +652,8 @@ static const struct drm_ioctl_desc drm_ioctls[] = {
>  
>  	DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETRESOURCES, drm_mode_getresources, 0),
>  
> -	DRM_IOCTL_DEF(DRM_IOCTL_PRIME_HANDLE_TO_FD, drm_prime_handle_to_fd_ioctl, DRM_AUTH|DRM_RENDER_ALLOW),
> -	DRM_IOCTL_DEF(DRM_IOCTL_PRIME_FD_TO_HANDLE, drm_prime_fd_to_handle_ioctl, DRM_AUTH|DRM_RENDER_ALLOW),
> +	DRM_IOCTL_DEF(DRM_IOCTL_PRIME_HANDLE_TO_FD, drm_prime_handle_to_fd_ioctl, DRM_RENDER_ALLOW),
> +	DRM_IOCTL_DEF(DRM_IOCTL_PRIME_FD_TO_HANDLE, drm_prime_fd_to_handle_ioctl, DRM_RENDER_ALLOW),
>  
>  	DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETPLANERESOURCES, drm_mode_getplane_res, 0),
>  	DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETCRTC, drm_mode_getcrtc, 0),

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

Re: [PATCH 5/5] drm: drop DRM_AUTH from PRIME_TO/FROM_HANDLE ioctls

[Boris Brezillon]

Hi Emil,

On Fri,  1 Nov 2019 13:03:13 +0000
Emil Velikov <emil.l.velikov@gmail.com> wrote:

> From: Emil Velikov <emil.velikov@collabora.com>
> 
> As mentioned by Christian, for drivers which support only primary nodes
> this changes the returned error from -EACCES into -EOPNOTSUPP/-ENOSYS.

Are you sure this is true for MODESET-only nodes (those that do not
have the RENDER cap set) implementing ->{fd_to_handle,handle_to_fd}()?
Shouldn't the is_authenticated() check still be done in that case?

Regards,

Boris

> 
> For others, this check in particular will be a noop. So let's remove it
> as suggested by Christian.
> 
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Cc: amd-gfx@lists.freedesktop.org
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Cc: Sean Paul <sean@poorly.run>
> Acked-by: Christian König <christian.koenig@amd.com>
> Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
> ---
>  drivers/gpu/drm/drm_ioctl.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
> index fcd728d7cf72..5afb39688b55 100644
> --- a/drivers/gpu/drm/drm_ioctl.c
> +++ b/drivers/gpu/drm/drm_ioctl.c
> @@ -652,8 +652,8 @@ static const struct drm_ioctl_desc drm_ioctls[] = {
>  
>  	DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETRESOURCES, drm_mode_getresources, 0),
>  
> -	DRM_IOCTL_DEF(DRM_IOCTL_PRIME_HANDLE_TO_FD, drm_prime_handle_to_fd_ioctl, DRM_AUTH|DRM_RENDER_ALLOW),
> -	DRM_IOCTL_DEF(DRM_IOCTL_PRIME_FD_TO_HANDLE, drm_prime_fd_to_handle_ioctl, DRM_AUTH|DRM_RENDER_ALLOW),
> +	DRM_IOCTL_DEF(DRM_IOCTL_PRIME_HANDLE_TO_FD, drm_prime_handle_to_fd_ioctl, DRM_RENDER_ALLOW),
> +	DRM_IOCTL_DEF(DRM_IOCTL_PRIME_FD_TO_HANDLE, drm_prime_fd_to_handle_ioctl, DRM_RENDER_ALLOW),
>  
>  	DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETPLANERESOURCES, drm_mode_getplane_res, 0),
>  	DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETCRTC, drm_mode_getcrtc, 0),

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 5/5] drm: drop DRM_AUTH from PRIME_TO/FROM_HANDLE ioctls

[Emil Velikov]

On Wed, 27 Nov 2019 at 07:41, Boris Brezillon
<boris.brezillon@collabora.com> wrote:
>
> Hi Emil,
>
> On Fri,  1 Nov 2019 13:03:13 +0000
> Emil Velikov <emil.l.velikov@gmail.com> wrote:
>
> > From: Emil Velikov <emil.velikov@collabora.com>
> >
> > As mentioned by Christian, for drivers which support only primary nodes
> > this changes the returned error from -EACCES into -EOPNOTSUPP/-ENOSYS.
>
> Are you sure this is true for MODESET-only nodes (those that do not
> have the RENDER cap set) implementing ->{fd_to_handle,handle_to_fd}()?
> Shouldn't the is_authenticated() check still be done in that case?
>
Thanks for catching this. Just sent out v2, which I should address the concern.

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

Re: [PATCH 5/5] drm: drop DRM_AUTH from PRIME_TO/FROM_HANDLE ioctls

[Emil Velikov]

On Wed, 27 Nov 2019 at 07:41, Boris Brezillon
<boris.brezillon@collabora.com> wrote:
>
> Hi Emil,
>
> On Fri,  1 Nov 2019 13:03:13 +0000
> Emil Velikov <emil.l.velikov@gmail.com> wrote:
>
> > From: Emil Velikov <emil.velikov@collabora.com>
> >
> > As mentioned by Christian, for drivers which support only primary nodes
> > this changes the returned error from -EACCES into -EOPNOTSUPP/-ENOSYS.
>
> Are you sure this is true for MODESET-only nodes (those that do not
> have the RENDER cap set) implementing ->{fd_to_handle,handle_to_fd}()?
> Shouldn't the is_authenticated() check still be done in that case?
>
Thanks for catching this. Just sent out v2, which I should address the concern.

-Emil
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 5/5] drm: drop DRM_AUTH from PRIME_TO/FROM_HANDLE ioctls

[Daniel Vetter]

On Wed, Nov 27, 2019 at 04:27:29PM +0000, Emil Velikov wrote:
> On Wed, 27 Nov 2019 at 07:41, Boris Brezillon
> <boris.brezillon@collabora.com> wrote:
> >
> > Hi Emil,
> >
> > On Fri,  1 Nov 2019 13:03:13 +0000
> > Emil Velikov <emil.l.velikov@gmail.com> wrote:
> >
> > > From: Emil Velikov <emil.velikov@collabora.com>
> > >
> > > As mentioned by Christian, for drivers which support only primary nodes
> > > this changes the returned error from -EACCES into -EOPNOTSUPP/-ENOSYS.
> >
> > Are you sure this is true for MODESET-only nodes (those that do not
> > have the RENDER cap set) implementing ->{fd_to_handle,handle_to_fd}()?
> > Shouldn't the is_authenticated() check still be done in that case?
> >
> Thanks for catching this. Just sent out v2, which I should address the concern.

Why do we need this additional check in v2? What can go wrong on modeset
drivers if non-authenticated legacy things can use this? modeset-only
drivers have all their resources segregated by the drm core (drm_fb,
mmaps, buffer lists), so there's really no access limitations that can go
wrong here.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 5/5] drm: drop DRM_AUTH from PRIME_TO/FROM_HANDLE ioctls

[Daniel Vetter]

On Wed, Nov 27, 2019 at 04:27:29PM +0000, Emil Velikov wrote:
> On Wed, 27 Nov 2019 at 07:41, Boris Brezillon
> <boris.brezillon@collabora.com> wrote:
> >
> > Hi Emil,
> >
> > On Fri,  1 Nov 2019 13:03:13 +0000
> > Emil Velikov <emil.l.velikov@gmail.com> wrote:
> >
> > > From: Emil Velikov <emil.velikov@collabora.com>
> > >
> > > As mentioned by Christian, for drivers which support only primary nodes
> > > this changes the returned error from -EACCES into -EOPNOTSUPP/-ENOSYS.
> >
> > Are you sure this is true for MODESET-only nodes (those that do not
> > have the RENDER cap set) implementing ->{fd_to_handle,handle_to_fd}()?
> > Shouldn't the is_authenticated() check still be done in that case?
> >
> Thanks for catching this. Just sent out v2, which I should address the concern.

Why do we need this additional check in v2? What can go wrong on modeset
drivers if non-authenticated legacy things can use this? modeset-only
drivers have all their resources segregated by the drm core (drm_fb,
mmaps, buffer lists), so there's really no access limitations that can go
wrong here.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 5/5] drm: drop DRM_AUTH from PRIME_TO/FROM_HANDLE ioctls

[Daniel Vetter]

On Wed, Nov 27, 2019 at 04:27:29PM +0000, Emil Velikov wrote:
> On Wed, 27 Nov 2019 at 07:41, Boris Brezillon
> <boris.brezillon@collabora.com> wrote:
> >
> > Hi Emil,
> >
> > On Fri,  1 Nov 2019 13:03:13 +0000
> > Emil Velikov <emil.l.velikov@gmail.com> wrote:
> >
> > > From: Emil Velikov <emil.velikov@collabora.com>
> > >
> > > As mentioned by Christian, for drivers which support only primary nodes
> > > this changes the returned error from -EACCES into -EOPNOTSUPP/-ENOSYS.
> >
> > Are you sure this is true for MODESET-only nodes (those that do not
> > have the RENDER cap set) implementing ->{fd_to_handle,handle_to_fd}()?
> > Shouldn't the is_authenticated() check still be done in that case?
> >
> Thanks for catching this. Just sent out v2, which I should address the concern.

Why do we need this additional check in v2? What can go wrong on modeset
drivers if non-authenticated legacy things can use this? modeset-only
drivers have all their resources segregated by the drm core (drm_fb,
mmaps, buffer lists), so there's really no access limitations that can go
wrong here.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 5/5] drm: drop DRM_AUTH from PRIME_TO/FROM_HANDLE ioctls

[Emil Velikov]

On Wed, 27 Nov 2019 at 18:04, Daniel Vetter <daniel@ffwll.ch> wrote:
>
> On Wed, Nov 27, 2019 at 04:27:29PM +0000, Emil Velikov wrote:
> > On Wed, 27 Nov 2019 at 07:41, Boris Brezillon
> > <boris.brezillon@collabora.com> wrote:
> > >
> > > Hi Emil,
> > >
> > > On Fri,  1 Nov 2019 13:03:13 +0000
> > > Emil Velikov <emil.l.velikov@gmail.com> wrote:
> > >
> > > > From: Emil Velikov <emil.velikov@collabora.com>
> > > >
> > > > As mentioned by Christian, for drivers which support only primary nodes
> > > > this changes the returned error from -EACCES into -EOPNOTSUPP/-ENOSYS.
> > >
> > > Are you sure this is true for MODESET-only nodes (those that do not
> > > have the RENDER cap set) implementing ->{fd_to_handle,handle_to_fd}()?
> > > Shouldn't the is_authenticated() check still be done in that case?
> > >
> > Thanks for catching this. Just sent out v2, which I should address the concern.
>
> Why do we need this additional check in v2? What can go wrong on modeset
> drivers if non-authenticated legacy things can use this? modeset-only
> drivers have all their resources segregated by the drm core (drm_fb,
> mmaps, buffer lists), so there's really no access limitations that can go
> wrong here.

Welcome back Daniel.

I haven't audited the core drm code, so wasn't sure if there's any
issues that may arise.
Hence the conservative approach in v2.

If you think this is fine as-is a formal Reviewed-by would be highly
appreciated.

Thanks
Emil
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 5/5] drm: drop DRM_AUTH from PRIME_TO/FROM_HANDLE ioctls

[Emil Velikov]

On Wed, 27 Nov 2019 at 18:04, Daniel Vetter <daniel@ffwll.ch> wrote:
>
> On Wed, Nov 27, 2019 at 04:27:29PM +0000, Emil Velikov wrote:
> > On Wed, 27 Nov 2019 at 07:41, Boris Brezillon
> > <boris.brezillon@collabora.com> wrote:
> > >
> > > Hi Emil,
> > >
> > > On Fri,  1 Nov 2019 13:03:13 +0000
> > > Emil Velikov <emil.l.velikov@gmail.com> wrote:
> > >
> > > > From: Emil Velikov <emil.velikov@collabora.com>
> > > >
> > > > As mentioned by Christian, for drivers which support only primary nodes
> > > > this changes the returned error from -EACCES into -EOPNOTSUPP/-ENOSYS.
> > >
> > > Are you sure this is true for MODESET-only nodes (those that do not
> > > have the RENDER cap set) implementing ->{fd_to_handle,handle_to_fd}()?
> > > Shouldn't the is_authenticated() check still be done in that case?
> > >
> > Thanks for catching this. Just sent out v2, which I should address the concern.
>
> Why do we need this additional check in v2? What can go wrong on modeset
> drivers if non-authenticated legacy things can use this? modeset-only
> drivers have all their resources segregated by the drm core (drm_fb,
> mmaps, buffer lists), so there's really no access limitations that can go
> wrong here.

Welcome back Daniel.

I haven't audited the core drm code, so wasn't sure if there's any
issues that may arise.
Hence the conservative approach in v2.

If you think this is fine as-is a formal Reviewed-by would be highly
appreciated.

Thanks
Emil
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 5/5] drm: drop DRM_AUTH from PRIME_TO/FROM_HANDLE ioctls

[Emil Velikov]

On Wed, 27 Nov 2019 at 18:04, Daniel Vetter <daniel@ffwll.ch> wrote:
>
> On Wed, Nov 27, 2019 at 04:27:29PM +0000, Emil Velikov wrote:
> > On Wed, 27 Nov 2019 at 07:41, Boris Brezillon
> > <boris.brezillon@collabora.com> wrote:
> > >
> > > Hi Emil,
> > >
> > > On Fri,  1 Nov 2019 13:03:13 +0000
> > > Emil Velikov <emil.l.velikov@gmail.com> wrote:
> > >
> > > > From: Emil Velikov <emil.velikov@collabora.com>
> > > >
> > > > As mentioned by Christian, for drivers which support only primary nodes
> > > > this changes the returned error from -EACCES into -EOPNOTSUPP/-ENOSYS.
> > >
> > > Are you sure this is true for MODESET-only nodes (those that do not
> > > have the RENDER cap set) implementing ->{fd_to_handle,handle_to_fd}()?
> > > Shouldn't the is_authenticated() check still be done in that case?
> > >
> > Thanks for catching this. Just sent out v2, which I should address the concern.
>
> Why do we need this additional check in v2? What can go wrong on modeset
> drivers if non-authenticated legacy things can use this? modeset-only
> drivers have all their resources segregated by the drm core (drm_fb,
> mmaps, buffer lists), so there's really no access limitations that can go
> wrong here.

Welcome back Daniel.

I haven't audited the core drm code, so wasn't sure if there's any
issues that may arise.
Hence the conservative approach in v2.

If you think this is fine as-is a formal Reviewed-by would be highly
appreciated.

Thanks
Emil
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 5/5] drm: drop DRM_AUTH from PRIME_TO/FROM_HANDLE ioctls

[Daniel Vetter]

On Wed, Nov 27, 2019 at 06:32:56PM +0000, Emil Velikov wrote:
> On Wed, 27 Nov 2019 at 18:04, Daniel Vetter <daniel@ffwll.ch> wrote:
> >
> > On Wed, Nov 27, 2019 at 04:27:29PM +0000, Emil Velikov wrote:
> > > On Wed, 27 Nov 2019 at 07:41, Boris Brezillon
> > > <boris.brezillon@collabora.com> wrote:
> > > >
> > > > Hi Emil,
> > > >
> > > > On Fri,  1 Nov 2019 13:03:13 +0000
> > > > Emil Velikov <emil.l.velikov@gmail.com> wrote:
> > > >
> > > > > From: Emil Velikov <emil.velikov@collabora.com>
> > > > >
> > > > > As mentioned by Christian, for drivers which support only primary nodes
> > > > > this changes the returned error from -EACCES into -EOPNOTSUPP/-ENOSYS.
> > > >
> > > > Are you sure this is true for MODESET-only nodes (those that do not
> > > > have the RENDER cap set) implementing ->{fd_to_handle,handle_to_fd}()?
> > > > Shouldn't the is_authenticated() check still be done in that case?
> > > >
> > > Thanks for catching this. Just sent out v2, which I should address the concern.
> >
> > Why do we need this additional check in v2? What can go wrong on modeset
> > drivers if non-authenticated legacy things can use this? modeset-only
> > drivers have all their resources segregated by the drm core (drm_fb,
> > mmaps, buffer lists), so there's really no access limitations that can go
> > wrong here.
> 
> Welcome back Daniel.
> 
> I haven't audited the core drm code, so wasn't sure if there's any
> issues that may arise.
> Hence the conservative approach in v2.
> 
> If you think this is fine as-is a formal Reviewed-by would be highly
> appreciated.

I think there's a non-zero chance I'll have to eat a few hats on this, but
I think v1 is solid.

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> 
> Thanks
> Emil

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 5/5] drm: drop DRM_AUTH from PRIME_TO/FROM_HANDLE ioctls

[Daniel Vetter]

On Wed, Nov 27, 2019 at 06:32:56PM +0000, Emil Velikov wrote:
> On Wed, 27 Nov 2019 at 18:04, Daniel Vetter <daniel@ffwll.ch> wrote:
> >
> > On Wed, Nov 27, 2019 at 04:27:29PM +0000, Emil Velikov wrote:
> > > On Wed, 27 Nov 2019 at 07:41, Boris Brezillon
> > > <boris.brezillon@collabora.com> wrote:
> > > >
> > > > Hi Emil,
> > > >
> > > > On Fri,  1 Nov 2019 13:03:13 +0000
> > > > Emil Velikov <emil.l.velikov@gmail.com> wrote:
> > > >
> > > > > From: Emil Velikov <emil.velikov@collabora.com>
> > > > >
> > > > > As mentioned by Christian, for drivers which support only primary nodes
> > > > > this changes the returned error from -EACCES into -EOPNOTSUPP/-ENOSYS.
> > > >
> > > > Are you sure this is true for MODESET-only nodes (those that do not
> > > > have the RENDER cap set) implementing ->{fd_to_handle,handle_to_fd}()?
> > > > Shouldn't the is_authenticated() check still be done in that case?
> > > >
> > > Thanks for catching this. Just sent out v2, which I should address the concern.
> >
> > Why do we need this additional check in v2? What can go wrong on modeset
> > drivers if non-authenticated legacy things can use this? modeset-only
> > drivers have all their resources segregated by the drm core (drm_fb,
> > mmaps, buffer lists), so there's really no access limitations that can go
> > wrong here.
> 
> Welcome back Daniel.
> 
> I haven't audited the core drm code, so wasn't sure if there's any
> issues that may arise.
> Hence the conservative approach in v2.
> 
> If you think this is fine as-is a formal Reviewed-by would be highly
> appreciated.

I think there's a non-zero chance I'll have to eat a few hats on this, but
I think v1 is solid.

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> 
> Thanks
> Emil

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 5/5] drm: drop DRM_AUTH from PRIME_TO/FROM_HANDLE ioctls

[Daniel Vetter]

On Wed, Nov 27, 2019 at 06:32:56PM +0000, Emil Velikov wrote:
> On Wed, 27 Nov 2019 at 18:04, Daniel Vetter <daniel@ffwll.ch> wrote:
> >
> > On Wed, Nov 27, 2019 at 04:27:29PM +0000, Emil Velikov wrote:
> > > On Wed, 27 Nov 2019 at 07:41, Boris Brezillon
> > > <boris.brezillon@collabora.com> wrote:
> > > >
> > > > Hi Emil,
> > > >
> > > > On Fri,  1 Nov 2019 13:03:13 +0000
> > > > Emil Velikov <emil.l.velikov@gmail.com> wrote:
> > > >
> > > > > From: Emil Velikov <emil.velikov@collabora.com>
> > > > >
> > > > > As mentioned by Christian, for drivers which support only primary nodes
> > > > > this changes the returned error from -EACCES into -EOPNOTSUPP/-ENOSYS.
> > > >
> > > > Are you sure this is true for MODESET-only nodes (those that do not
> > > > have the RENDER cap set) implementing ->{fd_to_handle,handle_to_fd}()?
> > > > Shouldn't the is_authenticated() check still be done in that case?
> > > >
> > > Thanks for catching this. Just sent out v2, which I should address the concern.
> >
> > Why do we need this additional check in v2? What can go wrong on modeset
> > drivers if non-authenticated legacy things can use this? modeset-only
> > drivers have all their resources segregated by the drm core (drm_fb,
> > mmaps, buffer lists), so there's really no access limitations that can go
> > wrong here.
> 
> Welcome back Daniel.
> 
> I haven't audited the core drm code, so wasn't sure if there's any
> issues that may arise.
> Hence the conservative approach in v2.
> 
> If you think this is fine as-is a formal Reviewed-by would be highly
> appreciated.

I think there's a non-zero chance I'll have to eat a few hats on this, but
I think v1 is solid.

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> 
> Thanks
> Emil

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 5/5] drm: drop DRM_AUTH from PRIME_TO/FROM_HANDLE ioctls

[Emil Velikov]

On Wed, 27 Nov 2019 at 18:37, Daniel Vetter <daniel@ffwll.ch> wrote:
>
> On Wed, Nov 27, 2019 at 06:32:56PM +0000, Emil Velikov wrote:
> > On Wed, 27 Nov 2019 at 18:04, Daniel Vetter <daniel@ffwll.ch> wrote:
> > >
> > > On Wed, Nov 27, 2019 at 04:27:29PM +0000, Emil Velikov wrote:
> > > > On Wed, 27 Nov 2019 at 07:41, Boris Brezillon
> > > > <boris.brezillon@collabora.com> wrote:
> > > > >
> > > > > Hi Emil,
> > > > >
> > > > > On Fri,  1 Nov 2019 13:03:13 +0000
> > > > > Emil Velikov <emil.l.velikov@gmail.com> wrote:
> > > > >
> > > > > > From: Emil Velikov <emil.velikov@collabora.com>
> > > > > >
> > > > > > As mentioned by Christian, for drivers which support only primary nodes
> > > > > > this changes the returned error from -EACCES into -EOPNOTSUPP/-ENOSYS.
> > > > >
> > > > > Are you sure this is true for MODESET-only nodes (those that do not
> > > > > have the RENDER cap set) implementing ->{fd_to_handle,handle_to_fd}()?
> > > > > Shouldn't the is_authenticated() check still be done in that case?
> > > > >
> > > > Thanks for catching this. Just sent out v2, which I should address the concern.
> > >
> > > Why do we need this additional check in v2? What can go wrong on modeset
> > > drivers if non-authenticated legacy things can use this? modeset-only
> > > drivers have all their resources segregated by the drm core (drm_fb,
> > > mmaps, buffer lists), so there's really no access limitations that can go
> > > wrong here.
> >
> > Welcome back Daniel.
> >
> > I haven't audited the core drm code, so wasn't sure if there's any
> > issues that may arise.
> > Hence the conservative approach in v2.
> >
> > If you think this is fine as-is a formal Reviewed-by would be highly
> > appreciated.
>
> I think there's a non-zero chance I'll have to eat a few hats on this, but
> I think v1 is solid.
>
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>
Thanks. I've just re-read the DIM instructions and pushed this to drm-misc-next.
Fingers crossed, I did not butcher it this time around.

-Emil
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 5/5] drm: drop DRM_AUTH from PRIME_TO/FROM_HANDLE ioctls

[Emil Velikov]

On Wed, 27 Nov 2019 at 18:37, Daniel Vetter <daniel@ffwll.ch> wrote:
>
> On Wed, Nov 27, 2019 at 06:32:56PM +0000, Emil Velikov wrote:
> > On Wed, 27 Nov 2019 at 18:04, Daniel Vetter <daniel@ffwll.ch> wrote:
> > >
> > > On Wed, Nov 27, 2019 at 04:27:29PM +0000, Emil Velikov wrote:
> > > > On Wed, 27 Nov 2019 at 07:41, Boris Brezillon
> > > > <boris.brezillon@collabora.com> wrote:
> > > > >
> > > > > Hi Emil,
> > > > >
> > > > > On Fri,  1 Nov 2019 13:03:13 +0000
> > > > > Emil Velikov <emil.l.velikov@gmail.com> wrote:
> > > > >
> > > > > > From: Emil Velikov <emil.velikov@collabora.com>
> > > > > >
> > > > > > As mentioned by Christian, for drivers which support only primary nodes
> > > > > > this changes the returned error from -EACCES into -EOPNOTSUPP/-ENOSYS.
> > > > >
> > > > > Are you sure this is true for MODESET-only nodes (those that do not
> > > > > have the RENDER cap set) implementing ->{fd_to_handle,handle_to_fd}()?
> > > > > Shouldn't the is_authenticated() check still be done in that case?
> > > > >
> > > > Thanks for catching this. Just sent out v2, which I should address the concern.
> > >
> > > Why do we need this additional check in v2? What can go wrong on modeset
> > > drivers if non-authenticated legacy things can use this? modeset-only
> > > drivers have all their resources segregated by the drm core (drm_fb,
> > > mmaps, buffer lists), so there's really no access limitations that can go
> > > wrong here.
> >
> > Welcome back Daniel.
> >
> > I haven't audited the core drm code, so wasn't sure if there's any
> > issues that may arise.
> > Hence the conservative approach in v2.
> >
> > If you think this is fine as-is a formal Reviewed-by would be highly
> > appreciated.
>
> I think there's a non-zero chance I'll have to eat a few hats on this, but
> I think v1 is solid.
>
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>
Thanks. I've just re-read the DIM instructions and pushed this to drm-misc-next.
Fingers crossed, I did not butcher it this time around.

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

Re: [PATCH 5/5] drm: drop DRM_AUTH from PRIME_TO/FROM_HANDLE ioctls

[Emil Velikov]

On Wed, 27 Nov 2019 at 18:37, Daniel Vetter <daniel@ffwll.ch> wrote:
>
> On Wed, Nov 27, 2019 at 06:32:56PM +0000, Emil Velikov wrote:
> > On Wed, 27 Nov 2019 at 18:04, Daniel Vetter <daniel@ffwll.ch> wrote:
> > >
> > > On Wed, Nov 27, 2019 at 04:27:29PM +0000, Emil Velikov wrote:
> > > > On Wed, 27 Nov 2019 at 07:41, Boris Brezillon
> > > > <boris.brezillon@collabora.com> wrote:
> > > > >
> > > > > Hi Emil,
> > > > >
> > > > > On Fri,  1 Nov 2019 13:03:13 +0000
> > > > > Emil Velikov <emil.l.velikov@gmail.com> wrote:
> > > > >
> > > > > > From: Emil Velikov <emil.velikov@collabora.com>
> > > > > >
> > > > > > As mentioned by Christian, for drivers which support only primary nodes
> > > > > > this changes the returned error from -EACCES into -EOPNOTSUPP/-ENOSYS.
> > > > >
> > > > > Are you sure this is true for MODESET-only nodes (those that do not
> > > > > have the RENDER cap set) implementing ->{fd_to_handle,handle_to_fd}()?
> > > > > Shouldn't the is_authenticated() check still be done in that case?
> > > > >
> > > > Thanks for catching this. Just sent out v2, which I should address the concern.
> > >
> > > Why do we need this additional check in v2? What can go wrong on modeset
> > > drivers if non-authenticated legacy things can use this? modeset-only
> > > drivers have all their resources segregated by the drm core (drm_fb,
> > > mmaps, buffer lists), so there's really no access limitations that can go
> > > wrong here.
> >
> > Welcome back Daniel.
> >
> > I haven't audited the core drm code, so wasn't sure if there's any
> > issues that may arise.
> > Hence the conservative approach in v2.
> >
> > If you think this is fine as-is a formal Reviewed-by would be highly
> > appreciated.
>
> I think there's a non-zero chance I'll have to eat a few hats on this, but
> I think v1 is solid.
>
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>
Thanks. I've just re-read the DIM instructions and pushed this to drm-misc-next.
Fingers crossed, I did not butcher it this time around.

-Emil
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH v2] drm: drop DRM_AUTH from PRIME_TO/FROM_HANDLE ioctls

[Emil Velikov]

From: Emil Velikov <emil.velikov@collabora.com>

Current validation requires that we're authenticated, even though we can
bypass (by design) the authentication when using a render node.

Let's address the former by following the design decision.

v2: Add simpler validation in the ioctls themselves (Boris)

Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: amd-gfx@lists.freedesktop.org
Cc: Boris Brezillon <boris.brezillon@collabora.com>
Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: Sean Paul <sean@poorly.run>
Acked-by: Christian König <christian.koenig@amd.com>
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
---
 drivers/gpu/drm/drm_ioctl.c |  4 ++--
 drivers/gpu/drm/drm_prime.c | 19 +++++++++++++++++++
 2 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index fcd728d7cf72..5afb39688b55 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -652,8 +652,8 @@ static const struct drm_ioctl_desc drm_ioctls[] = {
 
 	DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETRESOURCES, drm_mode_getresources, 0),
 
-	DRM_IOCTL_DEF(DRM_IOCTL_PRIME_HANDLE_TO_FD, drm_prime_handle_to_fd_ioctl, DRM_AUTH|DRM_RENDER_ALLOW),
-	DRM_IOCTL_DEF(DRM_IOCTL_PRIME_FD_TO_HANDLE, drm_prime_fd_to_handle_ioctl, DRM_AUTH|DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF(DRM_IOCTL_PRIME_HANDLE_TO_FD, drm_prime_handle_to_fd_ioctl, DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF(DRM_IOCTL_PRIME_FD_TO_HANDLE, drm_prime_fd_to_handle_ioctl, DRM_RENDER_ALLOW),
 
 	DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETPLANERESOURCES, drm_mode_getplane_res, 0),
 	DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETCRTC, drm_mode_getcrtc, 0),
diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
index 0a2316e0e812..dab166c860ec 100644
--- a/drivers/gpu/drm/drm_prime.c
+++ b/drivers/gpu/drm/drm_prime.c
@@ -358,11 +358,27 @@ int drm_gem_prime_fd_to_handle(struct drm_device *dev,
 }
 EXPORT_SYMBOL(drm_gem_prime_fd_to_handle);
 
+static inline bool
+allowed_ioctl(struct drm_device *dev, struct drm_file *file_priv)
+{
+	/* Unauthenticated master is allowed, for render capable devices */
+	if (drm_is_primary_client(file_priv)) {
+		if (!file_priv->authenticated &&
+		    !drm_core_check_feature(dev, DRIVER_RENDER))
+		return false;
+	}
+
+	return true;
+}
+
 int drm_prime_fd_to_handle_ioctl(struct drm_device *dev, void *data,
 				 struct drm_file *file_priv)
 {
 	struct drm_prime_handle *args = data;
 
+	if (!allowed_ioctl(dev, file_priv))
+		return -EACCES;
+
 	if (!dev->driver->prime_fd_to_handle)
 		return -ENOSYS;
 
@@ -511,6 +527,9 @@ int drm_prime_handle_to_fd_ioctl(struct drm_device *dev, void *data,
 {
 	struct drm_prime_handle *args = data;
 
+	if (!allowed_ioctl(dev, file_priv))
+		return -EACCES;
+
 	if (!dev->driver->prime_handle_to_fd)
 		return -ENOSYS;
 
-- 
2.23.0

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

[PATCH v2] drm: drop DRM_AUTH from PRIME_TO/FROM_HANDLE ioctls

[Emil Velikov]

From: Emil Velikov <emil.velikov@collabora.com>

Current validation requires that we're authenticated, even though we can
bypass (by design) the authentication when using a render node.

Let's address the former by following the design decision.

v2: Add simpler validation in the ioctls themselves (Boris)

Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: amd-gfx@lists.freedesktop.org
Cc: Boris Brezillon <boris.brezillon@collabora.com>
Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: Sean Paul <sean@poorly.run>
Acked-by: Christian König <christian.koenig@amd.com>
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
---
 drivers/gpu/drm/drm_ioctl.c |  4 ++--
 drivers/gpu/drm/drm_prime.c | 19 +++++++++++++++++++
 2 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index fcd728d7cf72..5afb39688b55 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -652,8 +652,8 @@ static const struct drm_ioctl_desc drm_ioctls[] = {
 
 	DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETRESOURCES, drm_mode_getresources, 0),
 
-	DRM_IOCTL_DEF(DRM_IOCTL_PRIME_HANDLE_TO_FD, drm_prime_handle_to_fd_ioctl, DRM_AUTH|DRM_RENDER_ALLOW),
-	DRM_IOCTL_DEF(DRM_IOCTL_PRIME_FD_TO_HANDLE, drm_prime_fd_to_handle_ioctl, DRM_AUTH|DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF(DRM_IOCTL_PRIME_HANDLE_TO_FD, drm_prime_handle_to_fd_ioctl, DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF(DRM_IOCTL_PRIME_FD_TO_HANDLE, drm_prime_fd_to_handle_ioctl, DRM_RENDER_ALLOW),
 
 	DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETPLANERESOURCES, drm_mode_getplane_res, 0),
 	DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETCRTC, drm_mode_getcrtc, 0),
diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
index 0a2316e0e812..dab166c860ec 100644
--- a/drivers/gpu/drm/drm_prime.c
+++ b/drivers/gpu/drm/drm_prime.c
@@ -358,11 +358,27 @@ int drm_gem_prime_fd_to_handle(struct drm_device *dev,
 }
 EXPORT_SYMBOL(drm_gem_prime_fd_to_handle);
 
+static inline bool
+allowed_ioctl(struct drm_device *dev, struct drm_file *file_priv)
+{
+	/* Unauthenticated master is allowed, for render capable devices */
+	if (drm_is_primary_client(file_priv)) {
+		if (!file_priv->authenticated &&
+		    !drm_core_check_feature(dev, DRIVER_RENDER))
+		return false;
+	}
+
+	return true;
+}
+
 int drm_prime_fd_to_handle_ioctl(struct drm_device *dev, void *data,
 				 struct drm_file *file_priv)
 {
 	struct drm_prime_handle *args = data;
 
+	if (!allowed_ioctl(dev, file_priv))
+		return -EACCES;
+
 	if (!dev->driver->prime_fd_to_handle)
 		return -ENOSYS;
 
@@ -511,6 +527,9 @@ int drm_prime_handle_to_fd_ioctl(struct drm_device *dev, void *data,
 {
 	struct drm_prime_handle *args = data;
 
+	if (!allowed_ioctl(dev, file_priv))
+		return -EACCES;
+
 	if (!dev->driver->prime_handle_to_fd)
 		return -ENOSYS;
 
-- 
2.23.0

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 1/5] drm/vmwgfx: move the require_exist handling together

[Emil Velikov]

On Fri, 1 Nov 2019 at 13:05, Emil Velikov <emil.l.velikov@gmail.com> wrote:
>
> From: Emil Velikov <emil.velikov@collabora.com>
>
> Move the render_client hunk for require_exist alongside the rest.
> Keeping all the reasons why an existing object is needed, in a single
> place makes it easier to follow.
>
> Cc: VMware Graphics <linux-graphics-maintainer@vmware.com>
> Cc: Thomas Hellstrom <thellstrom@vmware.com>
> Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
> ---
>  drivers/gpu/drm/vmwgfx/vmwgfx_surface.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
> index 29d8794f0421..1f989f3605c8 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
> @@ -909,16 +909,12 @@ vmw_surface_handle_reference(struct vmw_private *dev_priv,
>         uint32_t handle;
>         struct ttm_base_object *base;
>         int ret;
> -       bool require_exist = false;
>
>         if (handle_type == DRM_VMW_HANDLE_PRIME) {
>                 ret = ttm_prime_fd_to_handle(tfile, u_handle, &handle);
>                 if (unlikely(ret != 0))
>                         return ret;
>         } else {
> -               if (unlikely(drm_is_render_client(file_priv)))
> -                       require_exist = true;
> -
>                 handle = u_handle;
>         }
>
> @@ -935,6 +931,8 @@ vmw_surface_handle_reference(struct vmw_private *dev_priv,
>         }
>
>         if (handle_type != DRM_VMW_HANDLE_PRIME) {
> +               bool require_exist = false;
> +
>                 user_srf = container_of(base, struct vmw_user_surface,
>                                         prime.base);
>
> @@ -946,6 +944,9 @@ vmw_surface_handle_reference(struct vmw_private *dev_priv,
>                     user_srf->master != file_priv->master)
>                         require_exist = true;
>
> +               if (unlikely(drm_is_render_client(file_priv)))
> +                       require_exist = true;
> +
>                 ret = ttm_ref_object_add(tfile, base, TTM_REF_USAGE, NULL,
>                                          require_exist);
>                 if (unlikely(ret != 0)) {
> --
Thomas, VMware devs, humble poke?
Any comments and review would be appreciated.

Thanks
Emil
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/5] drm/vmwgfx: move the require_exist handling together

[Thomas Hellstrom]

Hi, Emil!

On 11/8/19 2:14 PM, Emil Velikov wrote:
> On Fri, 1 Nov 2019 at 13:05, Emil Velikov <emil.l.velikov@gmail.com> wrote:
>> From: Emil Velikov <emil.velikov@collabora.com>
>>
>> Move the render_client hunk for require_exist alongside the rest.
>> Keeping all the reasons why an existing object is needed, in a single
>> place makes it easier to follow.
>>
>> Cc: VMware Graphics <linux-graphics-maintainer@vmware.com>
>> Cc: Thomas Hellstrom <thellstrom@vmware.com>
>> Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
>> ---
>>  drivers/gpu/drm/vmwgfx/vmwgfx_surface.c | 9 +++++----
>>  1 file changed, 5 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
>> index 29d8794f0421..1f989f3605c8 100644
>> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
>> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
>> @@ -909,16 +909,12 @@ vmw_surface_handle_reference(struct vmw_private *dev_priv,
>>         uint32_t handle;
>>         struct ttm_base_object *base;
>>         int ret;
>> -       bool require_exist = false;
>>
>>         if (handle_type == DRM_VMW_HANDLE_PRIME) {
>>                 ret = ttm_prime_fd_to_handle(tfile, u_handle, &handle);
>>                 if (unlikely(ret != 0))
>>                         return ret;
>>         } else {
>> -               if (unlikely(drm_is_render_client(file_priv)))
>> -                       require_exist = true;
>> -
>>                 handle = u_handle;
>>         }
>>
>> @@ -935,6 +931,8 @@ vmw_surface_handle_reference(struct vmw_private *dev_priv,
>>         }
>>
>>         if (handle_type != DRM_VMW_HANDLE_PRIME) {
>> +               bool require_exist = false;
>> +
>>                 user_srf = container_of(base, struct vmw_user_surface,
>>                                         prime.base);
>>
>> @@ -946,6 +944,9 @@ vmw_surface_handle_reference(struct vmw_private *dev_priv,
>>                     user_srf->master != file_priv->master)
>>                         require_exist = true;
>>
>> +               if (unlikely(drm_is_render_client(file_priv)))
>> +                       require_exist = true;
>> +
>>                 ret = ttm_ref_object_add(tfile, base, TTM_REF_USAGE, NULL,
>>                                          require_exist);
>>                 if (unlikely(ret != 0)) {
>> --
> Thomas, VMware devs, humble poke?
> Any comments and review would be appreciated.
>
> Thanks
> Emil
>
Sorry, I'll look at this early monday.

Thanks,

Thomas


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

Re: [PATCH 1/5] drm/vmwgfx: move the require_exist handling together

[Thomas Hellstrom]

On 11/1/19 2:05 PM, Emil Velikov wrote:
> From: Emil Velikov <emil.velikov@collabora.com>
>
> Move the render_client hunk for require_exist alongside the rest.
> Keeping all the reasons why an existing object is needed, in a single
> place makes it easier to follow.
>
> Cc: VMware Graphics <linux-graphics-maintainer@vmware.com>
> Cc: Thomas Hellstrom <thellstrom@vmware.com>
> Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
> ---
>  drivers/gpu/drm/vmwgfx/vmwgfx_surface.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
> index 29d8794f0421..1f989f3605c8 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
> @@ -909,16 +909,12 @@ vmw_surface_handle_reference(struct vmw_private *dev_priv,
>  	uint32_t handle;
>  	struct ttm_base_object *base;
>  	int ret;
> -	bool require_exist = false;
>  
>  	if (handle_type == DRM_VMW_HANDLE_PRIME) {
>  		ret = ttm_prime_fd_to_handle(tfile, u_handle, &handle);
>  		if (unlikely(ret != 0))
>  			return ret;
>  	} else {
> -		if (unlikely(drm_is_render_client(file_priv)))
> -			require_exist = true;
> -
>  		handle = u_handle;
>  	}
>  
> @@ -935,6 +931,8 @@ vmw_surface_handle_reference(struct vmw_private *dev_priv,
>  	}
>  
>  	if (handle_type != DRM_VMW_HANDLE_PRIME) {
> +		bool require_exist = false;
> +
>  		user_srf = container_of(base, struct vmw_user_surface,
>  					prime.base);
>  
> @@ -946,6 +944,9 @@ vmw_surface_handle_reference(struct vmw_private *dev_priv,
>  		    user_srf->master != file_priv->master)
>  			require_exist = true;
>  
> +		if (unlikely(drm_is_render_client(file_priv)))
> +			require_exist = true;
> +
>  		ret = ttm_ref_object_add(tfile, base, TTM_REF_USAGE, NULL,
>  					 require_exist);
>  		if (unlikely(ret != 0)) {

Reviewed-by: Thomas Hellstrom <thellstrom@vmware.com>


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