diff options
author | Rebecca N. Palmer <rebecca_palmer@zoho.com> | 2015-05-08 13:26:50 (GMT) |
---|---|---|
committer | Daniel Vetter <daniel.vetter@ffwll.ch> | 2015-05-08 15:26:01 (GMT) |
commit | c7c7372edc4ebc173ad359aeb5752e9ce09f2741 (patch) | |
tree | 7e75f95061a4bfcc4d6cf1958334e2d16f7907a3 /drivers | |
parent | ac6f2e29bb08a2313b0480c6cea94b01ab274970 (diff) | |
download | linux-c7c7372edc4ebc173ad359aeb5752e9ce09f2741.tar.xz |
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 (17cabf571e50677d980e9ab2a43c5f11213003ae):
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: Rebecca Palmer <rebecca_palmer@zoho.com>
Reviewed-by: Mika 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: Daniel Vetter <daniel.vetter@ffwll.ch>
Diffstat (limited to 'drivers')
-rw-r--r-- | drivers/gpu/drm/i915/i915_gem_execbuffer.c | 31 |
1 files changed, 21 insertions, 10 deletions
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index 7ab63d9..560c79a 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -1540,28 +1540,39 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data, } 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, eb, batch_obj, args->batch_start_offset, args->batch_len, file->is_master); - if (IS_ERR(batch_obj)) { - ret = PTR_ERR(batch_obj); + if (IS_ERR(parsed_batch_obj)) { + ret = PTR_ERR(parsed_batch_obj); goto err; } /* - * 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 when the command parser is - * enabled. + * parsed_batch_obj == batch_obj means batch not fully parsed: + * Accept, but don't promote to secure. */ - 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; |