i915_gem_execbuffer_parse returns the original batch_obj on batches it can't check (currently, chained batches). Don't clear offset or set I915_DISPATCH_SECURE in this case.
Fixes 17cabf571e50677d980e9ab2a43c5f11213003ae. Signed-off-by: Rebecca Palmer <[email protected]> --- > > This version also brings exec_start = 0 inside this check, as it > > appears to be there because the copying (i915_cmd_parser.c:1054) > > removes any offset the original might have had. > [pushed without comment on this] That makes this a bug in mainline as well, though I don't know of any actual problems it causes. (The security hole exists there too, but only with the declared-unsafe parameter i915.enable_ppgtt=2, hence the change of title) This fix was tested on 3e0283a (tip of Linus' tree), in the same way as before (libva tests, vlc video, glxgears, beignet tests). diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index a3190e79..5ff8a64 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -1548,33 +1548,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. - * - * FIXME: with aliasing ppgtt, buffers that should only - * be in ggtt still end up in the aliasing ppgtt. remove - * this check when that is fixed. - */ - if (USES_FULL_PPGTT(dev)) - 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. + * + * FIXME: with aliasing ppgtt, buffers that should only + * be in ggtt still end up in the aliasing ppgtt. + * remove USES_FULL_PPGTT check when that is fixed. + */ + if (USES_FULL_PPGTT(dev)) + dispatch_flags |= I915_DISPATCH_SECURE; + exec_start = 0; + batch_obj = parsed_batch_obj; + } } batch_obj->base.pending_read_domains |= I915_GEM_DOMAIN_COMMAND; _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
