| Message ID | 20230208215340.2103955-1-zack@kde.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/vmwgfx: Do not drop the reference to the handle too soon | expand |
On 2/8/23 13:53, Zack Rusin wrote: > From: Zack Rusin <zackr@vmware.com> > > It is possible for userspace to predict the next buffer handle and > to destroy the buffer while it's still used by the kernel. Delay > dropping the internal reference on the buffers until kernel is done > with them. > > Signed-off-by: Zack Rusin <zackr@vmware.com> > Fixes: 8afa13a0583f ("drm/vmwgfx: Implement DRIVER_GEM") > Cc: <stable@vger.kernel.org> # v5.17+ > --- > drivers/gpu/drm/vmwgfx/vmwgfx_bo.c | 3 ++- > drivers/gpu/drm/vmwgfx/vmwgfx_gem.c | 4 ++-- > drivers/gpu/drm/vmwgfx/vmwgfx_surface.c | 1 - > 3 files changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c > index 43ffa5c7acbd..65bd88c8fef9 100644 > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c > @@ -708,7 +708,8 @@ int vmw_dumb_create(struct drm_file *file_priv, > ret = vmw_gem_object_create_with_handle(dev_priv, file_priv, > args->size, &args->handle, > &vbo); > - > + /* drop reference from allocate - handle holds it now */ > + drm_gem_object_put(&vbo->tbo.base); > return ret; > } > > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c b/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c > index 51bd1f8c5cc4..d6baf73a6458 100644 > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c > @@ -133,8 +133,6 @@ int vmw_gem_object_create_with_handle(struct vmw_private *dev_priv, > (*p_vbo)->tbo.base.funcs = &vmw_gem_object_funcs; > > ret = drm_gem_handle_create(filp, &(*p_vbo)->tbo.base, handle); > - /* drop reference from allocate - handle holds it now */ > - drm_gem_object_put(&(*p_vbo)->tbo.base); > out_no_bo: > return ret; > } > @@ -161,6 +159,8 @@ int vmw_gem_object_create_ioctl(struct drm_device *dev, void *data, > rep->map_handle = drm_vma_node_offset_addr(&vbo->tbo.base.vma_node); > rep->cur_gmr_id = handle; > rep->cur_gmr_offset = 0; > + /* drop reference from allocate - handle holds it now */ > + drm_gem_object_put(&vbo->tbo.base); > out_no_bo: > return ret; > } > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c > index 9d4ae9623a00..d18fec953fa7 100644 > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c > @@ -867,7 +867,6 @@ int vmw_surface_define_ioctl(struct drm_device *dev, void *data, > goto out_unlock; > } > vmw_bo_reference(res->guest_memory_bo); > - drm_gem_object_get(&res->guest_memory_bo->tbo.base); > } > > tmp = vmw_resource_reference(&srf->res); LGTM! Reviewed-by: Maaz Mombasawala <mombasawalam@vmware.com>
From: Martin Krastev <krastevm@vmware.com> Looks good to me. Reviewed-by: Martin Krastev <krastevm@vmware.com> Regards, Martin On 8.02.23 г. 23:53 ч., Zack Rusin wrote: > From: Zack Rusin <zackr@vmware.com> > > It is possible for userspace to predict the next buffer handle and > to destroy the buffer while it's still used by the kernel. Delay > dropping the internal reference on the buffers until kernel is done > with them. > > Signed-off-by: Zack Rusin <zackr@vmware.com> > Fixes: 8afa13a0583f ("drm/vmwgfx: Implement DRIVER_GEM") > Cc: <stable@vger.kernel.org> # v5.17+ > --- > drivers/gpu/drm/vmwgfx/vmwgfx_bo.c | 3 ++- > drivers/gpu/drm/vmwgfx/vmwgfx_gem.c | 4 ++-- > drivers/gpu/drm/vmwgfx/vmwgfx_surface.c | 1 - > 3 files changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c > index 43ffa5c7acbd..65bd88c8fef9 100644 > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c > @@ -708,7 +708,8 @@ int vmw_dumb_create(struct drm_file *file_priv, > ret = vmw_gem_object_create_with_handle(dev_priv, file_priv, > args->size, &args->handle, > &vbo); > - > + /* drop reference from allocate - handle holds it now */ > + drm_gem_object_put(&vbo->tbo.base); > return ret; > } > > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c b/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c > index 51bd1f8c5cc4..d6baf73a6458 100644 > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c > @@ -133,8 +133,6 @@ int vmw_gem_object_create_with_handle(struct vmw_private *dev_priv, > (*p_vbo)->tbo.base.funcs = &vmw_gem_object_funcs; > > ret = drm_gem_handle_create(filp, &(*p_vbo)->tbo.base, handle); > - /* drop reference from allocate - handle holds it now */ > - drm_gem_object_put(&(*p_vbo)->tbo.base); > out_no_bo: > return ret; > } > @@ -161,6 +159,8 @@ int vmw_gem_object_create_ioctl(struct drm_device *dev, void *data, > rep->map_handle = drm_vma_node_offset_addr(&vbo->tbo.base.vma_node); > rep->cur_gmr_id = handle; > rep->cur_gmr_offset = 0; > + /* drop reference from allocate - handle holds it now */ > + drm_gem_object_put(&vbo->tbo.base); > out_no_bo: > return ret; > } > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c > index 9d4ae9623a00..d18fec953fa7 100644 > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c > @@ -867,7 +867,6 @@ int vmw_surface_define_ioctl(struct drm_device *dev, void *data, > goto out_unlock; > } > vmw_bo_reference(res->guest_memory_bo); > - drm_gem_object_get(&res->guest_memory_bo->tbo.base); > } > > tmp = vmw_resource_reference(&srf->res);
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c index 43ffa5c7acbd..65bd88c8fef9 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c @@ -708,7 +708,8 @@ int vmw_dumb_create(struct drm_file *file_priv, ret = vmw_gem_object_create_with_handle(dev_priv, file_priv, args->size, &args->handle, &vbo); - + /* drop reference from allocate - handle holds it now */ + drm_gem_object_put(&vbo->tbo.base); return ret; } diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c b/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c index 51bd1f8c5cc4..d6baf73a6458 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c @@ -133,8 +133,6 @@ int vmw_gem_object_create_with_handle(struct vmw_private *dev_priv, (*p_vbo)->tbo.base.funcs = &vmw_gem_object_funcs; ret = drm_gem_handle_create(filp, &(*p_vbo)->tbo.base, handle); - /* drop reference from allocate - handle holds it now */ - drm_gem_object_put(&(*p_vbo)->tbo.base); out_no_bo: return ret; } @@ -161,6 +159,8 @@ int vmw_gem_object_create_ioctl(struct drm_device *dev, void *data, rep->map_handle = drm_vma_node_offset_addr(&vbo->tbo.base.vma_node); rep->cur_gmr_id = handle; rep->cur_gmr_offset = 0; + /* drop reference from allocate - handle holds it now */ + drm_gem_object_put(&vbo->tbo.base); out_no_bo: return ret; } diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c index 9d4ae9623a00..d18fec953fa7 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c @@ -867,7 +867,6 @@ int vmw_surface_define_ioctl(struct drm_device *dev, void *data, goto out_unlock; } vmw_bo_reference(res->guest_memory_bo); - drm_gem_object_get(&res->guest_memory_bo->tbo.base); } tmp = vmw_resource_reference(&srf->res);