i915_parse_cmds returns -EACCES on chained batches, which "tells the
caller to abort and dispatch the workload as a non-secure batch",
but the mechanism implementing that was broken when
flags |= I915_DISPATCH_SECURE was moved from i915_gem_execbuffer_parse
to i915_gem_do_execbuffer (17cabf571e50677d980e9ab2a43c5f11213003ae):
i915_gem_execbuffer_parse returns the original batch_obj in this case,
and i915_gem_do_execbuffer doesn't check for that.

Is this being made secure some other way (in which case the obsolete
comments should probably be removed), or is this a security hole?

Warning: this is my first kernel patch, and has not been tested yet.
Signed-off-by: Rebecca Palmer <rebecca_pal...@zoho.com> 

--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -1398,7 +1398,7 @@ i915_gem_do_execbuffer(struct drm_device
 {
        struct drm_i915_private *dev_priv = dev->dev_private;
        struct eb_vmas *eb;
-       struct drm_i915_gem_object *batch_obj;
+       struct drm_i915_gem_object *batch_obj, *orig_batch_obj;
        struct drm_i915_gem_exec_object2 shadow_exec_entry;
        struct intel_engine_cs *ring;
        struct intel_context *ctx;
@@ -1511,7 +1511,7 @@ i915_gem_do_execbuffer(struct drm_device
                goto err;
 
        /* take note of the batch buffer before we might reorder the lists */
-       batch_obj = eb_get_batch(eb);
+       orig_batch_obj = eb_get_batch(eb);
 
        /* Move the objects en-masse into the GTT, evicting if necessary. */
        need_relocs = (args->flags & I915_EXEC_NO_RELOC) == 0;
@@ -1533,7 +1533,7 @@ i915_gem_do_execbuffer(struct drm_device
        }
 
        /* Set the pending read domains for the batch buffer to COMMAND */
-       if (batch_obj->base.pending_write_domain) {
+       if (orig_batch_obj->base.pending_write_domain) {
                DRM_DEBUG("Attempting to use self-modifying batch buffer\n");
                ret = -EINVAL;
                goto err;
@@ -1543,7 +1543,7 @@ i915_gem_do_execbuffer(struct drm_device
                batch_obj = i915_gem_execbuffer_parse(ring,
                                                      &shadow_exec_entry,
                                                      eb,
-                                                     batch_obj,
+                                                     orig_batch_obj,
                                                      args->batch_start_offset,
                                                      args->batch_len,
                                                      file->is_master);
@@ -1559,7 +1559,7 @@ i915_gem_do_execbuffer(struct drm_device
                 * don't want that set when the command parser is
                 * enabled.
                 */
-               if (USES_PPGTT(dev))
+               if (USES_PPGTT(dev) && batch_obj!=orig_batch_obj)
                        dispatch_flags |= I915_DISPATCH_SECURE;
 
                exec_start = 0;

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to