diff mbox series

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

Message ID 20191101130313.8862-3-emil.l.velikov@gmail.com (mailing list archive)
State New, archived
Headers show
Series [1/5] drm/vmwgfx: move the require_exist handling together | expand

Commit Message

Emil Velikov Nov. 1, 2019, 1:03 p.m. UTC 
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(-)

Comments

Thomas Hellstrom Nov. 12, 2019, 12:54 p.m. UTC | #1 
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
Emil Velikov Nov. 13, 2019, 2:48 p.m. UTC | #2 
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
diff mbox series

Patch

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[] = {