Good practice dictates that we do not leak stale information to our callers, and should avoid overwriting an outparam on an error path.
Reported-by: Ville Syrjälä ville.syrjala@linux.intel.com Signed-off-by: Chris Wilson chris@chris-wilson.co.uk Cc: Ville Syrjälä ville.syrjala@linux.intel.com --- drivers/gpu/drm/drm_gem.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index 1b0c2c127072..eeee320e406b 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -331,6 +331,7 @@ drm_gem_handle_create_tail(struct drm_file *file_priv, u32 *handlep) { struct drm_device *dev = obj->dev; + u32 handle; int ret;
WARN_ON(!mutex_is_locked(&dev->object_name_lock)); @@ -353,7 +354,7 @@ drm_gem_handle_create_tail(struct drm_file *file_priv, if (ret < 0) goto err_unref;
- *handlep = ret; + handle = ret;
ret = drm_vma_node_allow(&obj->vma_node, file_priv->filp); if (ret) @@ -365,13 +366,14 @@ drm_gem_handle_create_tail(struct drm_file *file_priv, goto err_revoke; }
+ *handlep = handle; return 0;
err_revoke: drm_vma_node_revoke(&obj->vma_node, file_priv->filp); err_remove: spin_lock(&file_priv->table_lock); - idr_remove(&file_priv->object_idr, *handlep); + idr_remove(&file_priv->object_idr, handle); spin_unlock(&file_priv->table_lock); err_unref: drm_gem_object_handle_unreference_unlocked(obj);
drm_gem_handle_delete() contains its own version of drm_gem_object_release_handle(), so lets just call the release method instead.
Reported-by: Ville Syrjälä ville.syrjala@linux.intel.com Signed-off-by: Chris Wilson chris@chris-wilson.co.uk Cc: Ville Syrjälä ville.syrjala@linux.intel.com --- drivers/gpu/drm/drm_gem.c | 55 +++++++++++++++++++++-------------------------- 1 file changed, 24 insertions(+), 31 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index eeee320e406b..2e8c77e71e1f 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -244,6 +244,29 @@ drm_gem_object_handle_unreference_unlocked(struct drm_gem_object *obj) drm_gem_object_unreference_unlocked(obj); }
+/* + * Called at device or object close to release the file's + * handle references on objects. + */ +static int +drm_gem_object_release_handle(int id, void *ptr, void *data) +{ + struct drm_file *file_priv = data; + struct drm_gem_object *obj = ptr; + struct drm_device *dev = obj->dev; + + if (drm_core_check_feature(dev, DRIVER_PRIME)) + drm_gem_remove_prime_handles(obj, file_priv); + drm_vma_node_revoke(&obj->vma_node, file_priv->filp); + + if (dev->driver->gem_close_object) + dev->driver->gem_close_object(obj, file_priv); + + drm_gem_object_handle_unreference_unlocked(obj); + + return 0; +} + /** * drm_gem_handle_delete - deletes the given file-private handle * @filp: drm file-private structure to use for the handle look up @@ -282,14 +305,7 @@ drm_gem_handle_delete(struct drm_file *filp, u32 handle) idr_remove(&filp->object_idr, handle); spin_unlock(&filp->table_lock);
- if (drm_core_check_feature(dev, DRIVER_PRIME)) - drm_gem_remove_prime_handles(obj, filp); - drm_vma_node_revoke(&obj->vma_node, filp->filp); - - if (dev->driver->gem_close_object) - dev->driver->gem_close_object(obj, filp); - drm_gem_object_handle_unreference_unlocked(obj); - + drm_gem_object_release_handle(handle, obj, filp); return 0; } EXPORT_SYMBOL(drm_gem_handle_delete); @@ -726,29 +742,6 @@ drm_gem_open(struct drm_device *dev, struct drm_file *file_private) spin_lock_init(&file_private->table_lock); }
-/* - * Called at device close to release the file's - * handle references on objects. - */ -static int -drm_gem_object_release_handle(int id, void *ptr, void *data) -{ - struct drm_file *file_priv = data; - struct drm_gem_object *obj = ptr; - struct drm_device *dev = obj->dev; - - if (drm_core_check_feature(dev, DRIVER_PRIME)) - drm_gem_remove_prime_handles(obj, file_priv); - drm_vma_node_revoke(&obj->vma_node, file_priv->filp); - - if (dev->driver->gem_close_object) - dev->driver->gem_close_object(obj, file_priv); - - drm_gem_object_handle_unreference_unlocked(obj); - - return 0; -} - /** * drm_gem_release - release file-private GEM resources * @dev: drm_device which is being closed by userspace
On Tue, Jan 05, 2016 at 09:42:31AM +0000, Chris Wilson wrote:
drm_gem_handle_delete() contains its own version of drm_gem_object_release_handle(), so lets just call the release method instead.
Reported-by: Ville Syrjälä ville.syrjala@linux.intel.com Signed-off-by: Chris Wilson chris@chris-wilson.co.uk Cc: Ville Syrjälä ville.syrjala@linux.intel.com
drivers/gpu/drm/drm_gem.c | 55 +++++++++++++++++++++-------------------------- 1 file changed, 24 insertions(+), 31 deletions(-)
Reviewed-by: Thierry Reding treding@nvidia.com
On Tue, Jan 05, 2016 at 04:15:46PM +0100, Thierry Reding wrote:
On Tue, Jan 05, 2016 at 09:42:31AM +0000, Chris Wilson wrote:
drm_gem_handle_delete() contains its own version of drm_gem_object_release_handle(), so lets just call the release method instead.
Reported-by: Ville Syrjälä ville.syrjala@linux.intel.com Signed-off-by: Chris Wilson chris@chris-wilson.co.uk Cc: Ville Syrjälä ville.syrjala@linux.intel.com
drivers/gpu/drm/drm_gem.c | 55 +++++++++++++++++++++-------------------------- 1 file changed, 24 insertions(+), 31 deletions(-)
Reviewed-by: Thierry Reding treding@nvidia.com
Both applied to drm-misc, thanks for patches&review. -Daniel
On Tue, Jan 05, 2016 at 09:42:30AM +0000, Chris Wilson wrote:
Good practice dictates that we do not leak stale information to our callers, and should avoid overwriting an outparam on an error path.
Reported-by: Ville Syrjälä ville.syrjala@linux.intel.com Signed-off-by: Chris Wilson chris@chris-wilson.co.uk Cc: Ville Syrjälä ville.syrjala@linux.intel.com
drivers/gpu/drm/drm_gem.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
Reviewed-by: Thierry Reding treding@nvidia.com
dri-devel@lists.freedesktop.org
-
Chris Wilson -
Daniel Vetter -
Thierry Reding