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@xxxxxxxxxxxxxxxxxx>
---
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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
