Commit c7c7372e authored by Rebecca N. Palmer's avatar Rebecca N. Palmer Committed by Daniel Vetter

drm/i915: Fix possible security hole in command parsing

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 (17cabf57):
i915_gem_execbuffer_parse returns the original batch_obj in this case,
and i915_gem_do_execbuffer doesn't check for that.

Don't set the secure bit in this case to make sure such batches don't
run with elevated priviledges.
Signed-off-by: default avatarRebecca Palmer <rebecca_palmer@zoho.com>
Reviewed-by: default avatarMika Kuoppala <mika.kuoppala@intel.com>
[danvet: Stitch together commit message. Also remove a comment as
suggested by Mika. And style-align the comment while at it.]
Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
parent ac6f2e29
...@@ -1540,28 +1540,39 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data, ...@@ -1540,28 +1540,39 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
} }
if (i915_needs_cmd_parser(ring) && args->batch_len) { if (i915_needs_cmd_parser(ring) && args->batch_len) {
batch_obj = i915_gem_execbuffer_parse(ring, struct drm_i915_gem_object *parsed_batch_obj;
parsed_batch_obj = i915_gem_execbuffer_parse(ring,
&shadow_exec_entry, &shadow_exec_entry,
eb, eb,
batch_obj, batch_obj,
args->batch_start_offset, args->batch_start_offset,
args->batch_len, args->batch_len,
file->is_master); file->is_master);
if (IS_ERR(batch_obj)) { if (IS_ERR(parsed_batch_obj)) {
ret = PTR_ERR(batch_obj); ret = PTR_ERR(parsed_batch_obj);
goto err; goto err;
} }
/* /*
* Set the DISPATCH_SECURE bit to remove the NON_SECURE * parsed_batch_obj == batch_obj means batch not fully parsed:
* bit from MI_BATCH_BUFFER_START commands issued in the * Accept, but don't promote to secure.
* dispatch_execbuffer implementations. We specifically
* don't want that set when the command parser is
* enabled.
*/ */
dispatch_flags |= I915_DISPATCH_SECURE;
exec_start = 0; if (parsed_batch_obj != batch_obj) {
/*
* Batch parsed and accepted:
*
* Set the DISPATCH_SECURE bit to remove the NON_SECURE
* bit from MI_BATCH_BUFFER_START commands issued in
* the dispatch_execbuffer implementations. We
* specifically don't want that set on batches the
* command parser has accepted.
*/
dispatch_flags |= I915_DISPATCH_SECURE;
exec_start = 0;
batch_obj = parsed_batch_obj;
}
} }
batch_obj->base.pending_read_domains |= I915_GEM_DOMAIN_COMMAND; batch_obj->base.pending_read_domains |= I915_GEM_DOMAIN_COMMAND;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment