[Intel-gfx] [PATCH] drm: Release driver references to handle before making it available again

[Chris Wilson]

When userspace closes a handle, we remove it from the file->object_idr
and then tell the driver to drop its references to that file/handle.
However, as the file/handle is already available again for reuse, it may
be reallocated back to userspace and active on a new object before the
driver has had a chance to drop the old file/handle references.

Whilst calling back into the driver, we have to drop the
file->table_lock spinlock and so to prevent reusing the closed handle we
mark that handle as stale in the idr, perform the callback and then
remove the handle. It is then possible for a lookup on that handle to
return an error object and so all callers of idr_find(file->object_idr)
need to check against IS_ERR_OR_NULL() instead.

Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
Cc: dri-devel at lists.freedesktop.org
Cc: David Airlie <airlied at linux.ie>
Cc: Lucas Stach <l.stach at pengutronix.de>
Cc: Russell King <linux+etnaviv at arm.linux.org.uk>
Cc: Christian Gmeiner <christian.gmeiner at gmail.com>
Cc: Daniel Vetter <daniel.vetter at intel.com>
Cc: Jani Nikula <jani.nikula at linux.intel.com>
Cc: Rob Clark <robdclark at gmail.com>
Cc: Eric Anholt <eric at anholt.net>
Cc: Dan Carpenter <dan.carpenter at oracle.com>
Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
---
 drivers/gpu/drm/drm_gem.c                    | 11 +++++++----
 drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c |  2 +-
 drivers/gpu/drm/i915/i915_gem_execbuffer.c   |  2 +-
 drivers/gpu/drm/msm/msm_gem_submit.c         |  2 +-
 drivers/gpu/drm/vc4/vc4_gem.c                |  2 +-
 5 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index 2e8c77e71e1f..c56a0b49d829 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -294,18 +294,21 @@ drm_gem_handle_delete(struct drm_file *filp, u32 handle)
 	spin_lock(&filp->table_lock);
 
 	/* Check if we currently have a reference on the object */
-	obj = idr_find(&filp->object_idr, handle);
-	if (obj == NULL) {
+	obj = idr_replace(&filp->object_idr, ERR_PTR(-ENOENT), handle);
+	if (IS_ERR(obj)) {
 		spin_unlock(&filp->table_lock);
 		return -EINVAL;
 	}
 	dev = obj->dev;
+	spin_unlock(&filp->table_lock);
 
 	/* Release reference and decrement refcount. */
+	drm_gem_object_release_handle(handle, obj, filp);
+
+	spin_lock(&filp->table_lock);
 	idr_remove(&filp->object_idr, handle);
 	spin_unlock(&filp->table_lock);
 
-	drm_gem_object_release_handle(handle, obj, filp);
 	return 0;
 }
 EXPORT_SYMBOL(drm_gem_handle_delete);
@@ -597,7 +600,7 @@ drm_gem_object_lookup(struct drm_device *dev, struct drm_file *filp,
 
 	/* Check if we currently have a reference on the object */
 	obj = idr_find(&filp->object_idr, handle);
-	if (obj == NULL) {
+	if (IS_ERR_OR_NULL(obj)) {
 		spin_unlock(&filp->table_lock);
 		return NULL;
 	}
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
index 1aba01a999df..77b549cb1301 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
@@ -78,7 +78,7 @@ static int submit_lookup_objects(struct etnaviv_gem_submit *submit,
 		 * all under single table_lock just hit object_idr directly:
 		 */
 		obj = idr_find(&file->object_idr, bo->handle);
-		if (!obj) {
+		if (IS_ERR_OR_NULL(obj)) {
 			DRM_ERROR("invalid handle %u at index %u\n",
 				  bo->handle, i);
 			ret = -EINVAL;
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index dccb517361b3..500bcff9ad48 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -106,7 +106,7 @@ eb_lookup_vmas(struct eb_vmas *eb,
 	 * or create the VMA without using GFP_ATOMIC */
 	for (i = 0; i < args->buffer_count; i++) {
 		obj = to_intel_bo(idr_find(&file->object_idr, exec[i].handle));
-		if (obj == NULL) {
+		if (IS_ERR_OR_NULL(obj)) {
 			spin_unlock(&file->table_lock);
 			DRM_DEBUG("Invalid object handle %d at index %d\n",
 				   exec[i].handle, i);
diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c b/drivers/gpu/drm/msm/msm_gem_submit.c
index 6d7cd3fe21e7..c6d7abd53d20 100644
--- a/drivers/gpu/drm/msm/msm_gem_submit.c
+++ b/drivers/gpu/drm/msm/msm_gem_submit.c
@@ -90,7 +90,7 @@ static int submit_lookup_objects(struct msm_gem_submit *submit,
 		 * all under single table_lock just hit object_idr directly:
 		 */
 		obj = idr_find(&file->object_idr, submit_bo.handle);
-		if (!obj) {
+		if (IS_ERR_OR_NULL(obj)) {
 			DRM_ERROR("invalid handle %u at index %u\n", submit_bo.handle, i);
 			ret = -EINVAL;
 			goto out_unlock;
diff --git a/drivers/gpu/drm/vc4/vc4_gem.c b/drivers/gpu/drm/vc4/vc4_gem.c
index 48ce30a6f4b5..9138702a1c67 100644
--- a/drivers/gpu/drm/vc4/vc4_gem.c
+++ b/drivers/gpu/drm/vc4/vc4_gem.c
@@ -494,7 +494,7 @@ vc4_cl_lookup_bos(struct drm_device *dev,
 	for (i = 0; i < exec->bo_count; i++) {
 		struct drm_gem_object *bo = idr_find(&file_priv->object_idr,
 						     handles[i]);
-		if (!bo) {
+		if (IS_ERR_OR_NULL(bo)) {
 			DRM_ERROR("Failed to look up GEM BO %d: %d\n",
 				  i, handles[i]);
 			ret = -EINVAL;
-- 
2.7.0.rc3