drm/i915: Fix potential context UAFs
authorjsg <jsg@openbsd.org>
Wed, 18 Jan 2023 23:58:45 +0000 (23:58 +0000)
committerjsg <jsg@openbsd.org>
Wed, 18 Jan 2023 23:58:45 +0000 (23:58 +0000)
From Rob Clark
b696c627b3f56e173f7f70b8487d66da8ff22506 in linux-6.1.y/6.1.7
afce71ff6daa9c0f852df0727fe32c6fb107f0fa in mainline linux

sys/dev/pci/drm/i915/gem/i915_gem_context.c

index ee85b9b..2a49ede 100644 (file)
@@ -1709,6 +1709,10 @@ void i915_gem_init__contexts(struct drm_i915_private *i915)
        init_contexts(&i915->gem.contexts);
 }
 
+/*
+ * Note that this implicitly consumes the ctx reference, by placing
+ * the ctx in the context_xa.
+ */
 static void gem_context_register(struct i915_gem_context *ctx,
                                 struct drm_i915_file_private *fpriv,
                                 u32 id)
@@ -1732,10 +1736,6 @@ static void gem_context_register(struct i915_gem_context *ctx,
                 curproc->p_p->ps_comm, ctx->pid);
 #endif
 
-       /* And finally expose ourselves to userspace via the idr */
-       old = xa_store(&fpriv->context_xa, id, ctx, GFP_KERNEL);
-       WARN_ON(old);
-
        spin_lock(&ctx->client->ctx_lock);
        list_add_tail_rcu(&ctx->client_link, &ctx->client->ctx_list);
        spin_unlock(&ctx->client->ctx_lock);
@@ -1743,6 +1743,10 @@ static void gem_context_register(struct i915_gem_context *ctx,
        spin_lock(&i915->gem.contexts.lock);
        list_add_tail(&ctx->link, &i915->gem.contexts.list);
        spin_unlock(&i915->gem.contexts.lock);
+
+       /* And finally expose ourselves to userspace via the idr */
+       old = xa_store(&fpriv->context_xa, id, ctx, GFP_KERNEL);
+       WARN_ON(old);
 }
 
 int i915_gem_context_open(struct drm_i915_private *i915,
@@ -2228,14 +2232,22 @@ finalize_create_context_locked(struct drm_i915_file_private *file_priv,
        if (IS_ERR(ctx))
                return ctx;
 
+       /*
+        * One for the xarray and one for the caller.  We need to grab
+        * the reference *prior* to making the ctx visble to userspace
+        * in gem_context_register(), as at any point after that
+        * userspace can try to race us with another thread destroying
+        * the context under our feet.
+        */
+       i915_gem_context_get(ctx);
+
        gem_context_register(ctx, file_priv, id);
 
        old = xa_erase(&file_priv->proto_context_xa, id);
        GEM_BUG_ON(old != pc);
        proto_context_close(file_priv->dev_priv, pc);
 
-       /* One for the xarray and one for the caller */
-       return i915_gem_context_get(ctx);
+       return ctx;
 }
 
 struct i915_gem_context *