[Intel-gfx] [PATCH] drm/i915: Always use kref tracking for contexts.

[Chris Wilson]

If we always initialize kref for the context, even if we are using fake
contexts for hangstats when there is no hw support, we can forgo the
dance to dereference the ctx->obj and inspect whether we are permitted
to use kref inside i915_gem_context_reference() and _unreference().

My ulterior motive here is to improve the debugging of a use-after-free
of ctx->obj. This patch avoids the dereference here and instead forces
the assertion checks associated with kref.

Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_drv.h         |  6 ++----
 drivers/gpu/drm/i915/i915_gem_context.c | 37 +++++++++++++++++++++------------
 2 files changed, 26 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 8af8e0dd3943..17a37578053c 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2380,14 +2380,12 @@ i915_gem_context_get(struct drm_i915_file_private *file_priv, u32 id);
 void i915_gem_context_free(struct kref *ctx_ref);
 static inline void i915_gem_context_reference(struct i915_hw_context *ctx)
 {
-	if (ctx->obj && HAS_HW_CONTEXTS(ctx->obj->base.dev))
-		kref_get(&ctx->ref);
+	kref_get(&ctx->ref);
 }
 
 static inline void i915_gem_context_unreference(struct i915_hw_context *ctx)
 {
-	if (ctx->obj && HAS_HW_CONTEXTS(ctx->obj->base.dev))
-		kref_put(&ctx->ref, i915_gem_context_free);
+	kref_put(&ctx->ref, i915_gem_context_free);
 }
 
 static inline bool i915_gem_context_is_default(const struct i915_hw_context *c)
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index e9f888ea67d6..48dc5f8f21bf 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -179,12 +179,9 @@ create_vm_for_ctx(struct drm_device *dev, struct i915_hw_context *ctx)
 }
 
 static struct i915_hw_context *
-__create_hw_context(struct drm_device *dev,
-		  struct drm_i915_file_private *file_priv)
+__ctx_alloc(void)
 {
-	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct i915_hw_context *ctx;
-	int ret;
 
 	ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
 	if (ctx == NULL)
@@ -193,6 +190,21 @@ __create_hw_context(struct drm_device *dev,
 	kref_init(&ctx->ref);
 	INIT_LIST_HEAD(&ctx->link);
 
+	return ctx;
+}
+
+static struct i915_hw_context *
+__create_hw_context(struct drm_device *dev,
+		  struct drm_i915_file_private *file_priv)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct i915_hw_context *ctx;
+	int ret;
+
+	ctx = __ctx_alloc();
+	if (IS_ERR(ctx))
+		return ctx;
+
 	ctx->obj = i915_gem_object_create_stolen(dev, dev_priv->hw_context_size);
 	if (ctx->obj == NULL)
 		ctx->obj = i915_gem_alloc_object(dev, dev_priv->hw_context_size);
@@ -492,11 +504,9 @@ int i915_gem_context_open(struct drm_device *dev, struct drm_file *file)
 
 	if (!HAS_HW_CONTEXTS(dev)) {
 		/* Cheat for hang stats */
-		file_priv->private_default_ctx =
-			kzalloc(sizeof(struct i915_hw_context), GFP_KERNEL);
-
-		if (file_priv->private_default_ctx == NULL)
-			return -ENOMEM;
+		file_priv->private_default_ctx = __ctx_alloc();
+		if (IS_ERR(file_priv->private_default_ctx))
+			return PTR_ERR(file_priv->private_default_ctx);
 
 		file_priv->private_default_ctx->vm = &dev_priv->gtt.base;
 		return 0;
@@ -521,14 +531,15 @@ void i915_gem_context_close(struct drm_device *dev, struct drm_file *file)
 {
 	struct drm_i915_file_private *file_priv = file->driver_priv;
 
-	if (!HAS_HW_CONTEXTS(dev)) {
-		kfree(file_priv->private_default_ctx);
+	if (IS_ERR_OR_NULL(file_priv->private_default_ctx))
 		return;
+
+	if (HAS_HW_CONTEXTS(dev)) {
+		idr_for_each(&file_priv->context_idr, context_idr_cleanup, NULL);
+		idr_destroy(&file_priv->context_idr);
 	}
 
-	idr_for_each(&file_priv->context_idr, context_idr_cleanup, NULL);
 	i915_gem_context_unreference(file_priv->private_default_ctx);
-	idr_destroy(&file_priv->context_idr);
 }
 
 struct i915_hw_context *
-- 
1.9.0