diff options
| -rw-r--r-- | drivers/gpu/drm/i915/i915_cmd_parser.c | 151 | ||||
| -rw-r--r-- | drivers/gpu/drm/i915/i915_drv.h | 16 | ||||
| -rw-r--r-- | drivers/gpu/drm/i915/i915_gem_context.c | 5 | ||||
| -rw-r--r-- | drivers/gpu/drm/i915/i915_gem_execbuffer.c | 43 |
4 files changed, 181 insertions, 34 deletions
diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c b/drivers/gpu/drm/i915/i915_cmd_parser.c index 21451e8ca985..8885c03f16c1 100644 --- a/drivers/gpu/drm/i915/i915_cmd_parser.c +++ b/drivers/gpu/drm/i915/i915_cmd_parser.c @@ -481,6 +481,19 @@ static const struct drm_i915_cmd_descriptor gen9_blt_cmds[] = { .reg = { .offset = 1, .mask = 0x007FFFFC } ), CMD( MI_LOAD_REGISTER_REG, SMI, !F, 0xFF, W, .reg = { .offset = 1, .mask = 0x007FFFFC, .step = 1 } ), + + /* + * We allow BB_START but apply further checks. We just sanitize the + * basic fields here. + */ +#define MI_BB_START_OPERAND_MASK GENMASK(SMI-1, 0) +#define MI_BB_START_OPERAND_EXPECT (MI_BATCH_PPGTT_HSW | 1) + CMD( MI_BATCH_BUFFER_START_GEN8, SMI, !F, 0xFF, B, + .bits = {{ + .offset = 0, + .mask = MI_BB_START_OPERAND_MASK, + .expected = MI_BB_START_OPERAND_EXPECT, + }}, ), }; static const struct drm_i915_cmd_descriptor noop_desc = @@ -1292,15 +1305,113 @@ static bool check_cmd(const struct intel_engine_cs *engine, return true; } +static int check_bbstart(const struct i915_gem_context *ctx, + u32 *cmd, u32 offset, u32 length, + u32 batch_len, + u64 batch_start, + u64 shadow_batch_start) +{ + u64 jump_offset, jump_target; + u32 target_cmd_offset, target_cmd_index; + + /* For igt compatibility on older platforms */ + if (CMDPARSER_USES_GGTT(ctx->i915)) { + DRM_DEBUG("CMD: Rejecting BB_START for ggtt based submission\n"); + return -EACCES; + } + + if (length != 3) { + DRM_DEBUG("CMD: Recursive BB_START with bad length(%u)\n", + length); + return -EINVAL; + } + + jump_target = *(u64*)(cmd+1); + jump_offset = jump_target - batch_start; + + /* + * Any underflow of jump_target is guaranteed to be outside the range + * of a u32, so >= test catches both too large and too small + */ + if (jump_offset >= batch_len) { + DRM_DEBUG("CMD: BB_START to 0x%llx jumps out of BB\n", + jump_target); + return -EINVAL; + } + + /* + * This cannot overflow a u32 because we already checked jump_offset + * is within the BB, and the batch_len is a u32 + */ + target_cmd_offset = lower_32_bits(jump_offset); + target_cmd_index = target_cmd_offset / sizeof(u32); + + *(u64*)(cmd + 1) = shadow_batch_start + target_cmd_offset; + + if (target_cmd_index == offset) + return 0; + + if (ctx->jump_whitelist_cmds <= target_cmd_index) { + DRM_DEBUG("CMD: Rejecting BB_START - truncated whitelist array\n"); + return -EINVAL; + } else if (!test_bit(target_cmd_index, ctx->jump_whitelist)) { + DRM_DEBUG("CMD: BB_START to 0x%llx not a previously executed cmd\n", + jump_target); + return -EINVAL; + } + + return 0; +} + +static void init_whitelist(struct i915_gem_context *ctx, u32 batch_len) +{ + const u32 batch_cmds = DIV_ROUND_UP(batch_len, sizeof(u32)); + const u32 exact_size = BITS_TO_LONGS(batch_cmds); + u32 next_size = BITS_TO_LONGS(roundup_pow_of_two(batch_cmds)); + unsigned long *next_whitelist; + + if (CMDPARSER_USES_GGTT(ctx->i915)) + return; + + if (batch_cmds <= ctx->jump_whitelist_cmds) { + memset(ctx->jump_whitelist, 0, exact_size * sizeof(u32)); + return; + } + +again: + next_whitelist = kcalloc(next_size, sizeof(long), GFP_KERNEL); + if (next_whitelist) { + kfree(ctx->jump_whitelist); + ctx->jump_whitelist = next_whitelist; + ctx->jump_whitelist_cmds = + next_size * BITS_PER_BYTE * sizeof(long); + return; + } + + if (next_size > exact_size) { + next_size = exact_size; + goto again; + } + + DRM_DEBUG("CMD: Failed to extend whitelist. BB_START may be disallowed\n"); + memset(ctx->jump_whitelist, 0, + BITS_TO_LONGS(ctx->jump_whitelist_cmds) * sizeof(u32)); + + return; +} + #define LENGTH_BIAS 2 /** * i915_parse_cmds() - parse a submitted batch buffer for privilege violations + * @ctx: the context in which the batch is to execute * @engine: the engine on which the batch is to execute * @batch_obj: the batch buffer in question - * @shadow_batch_obj: copy of the batch buffer in question + * @batch_start: Canonical base address of batch * @batch_start_offset: byte offset in the batch at which execution starts * @batch_len: length of the commands in batch_obj + * @shadow_batch_obj: copy of the batch buffer in question + * @shadow_batch_start: Canonical base address of shadow_batch_obj * * Parses the specified batch buffer looking for privilege violations as * described in the overview. @@ -1308,13 +1419,17 @@ static bool check_cmd(const struct intel_engine_cs *engine, * Return: non-zero if the parser finds violations or otherwise fails; -EACCES * if the batch appears legal but should use hardware parsing */ -int intel_engine_cmd_parser(struct intel_engine_cs *engine, + +int intel_engine_cmd_parser(struct i915_gem_context *ctx, + struct intel_engine_cs *engine, struct drm_i915_gem_object *batch_obj, - struct drm_i915_gem_object *shadow_batch_obj, + u64 batch_start, u32 batch_start_offset, - u32 batch_len) + u32 batch_len, + struct drm_i915_gem_object *shadow_batch_obj, + u64 shadow_batch_start) { - u32 *cmd, *batch_end; + u32 *cmd, *batch_end, offset = 0; struct drm_i915_cmd_descriptor default_desc = noop_desc; const struct drm_i915_cmd_descriptor *desc = &default_desc; bool needs_clflush_after = false; @@ -1328,6 +1443,8 @@ int intel_engine_cmd_parser(struct intel_engine_cs *engine, return PTR_ERR(cmd); } + init_whitelist(ctx, batch_len); + /* * We use the batch length as size because the shadow object is as * large or larger and copy_batch() will write MI_NOPs to the extra @@ -1348,16 +1465,6 @@ int intel_engine_cmd_parser(struct intel_engine_cs *engine, goto err; } - /* - * We don't try to handle BATCH_BUFFER_START because it adds - * non-trivial complexity. Instead we abort the scan and return - * and error to indicate that the batch is unsafe. - */ - if (desc->cmd.value == MI_BATCH_BUFFER_START) { - ret = -EACCES; - goto err; - } - if (desc->flags & CMD_DESC_FIXED) length = desc->length.fixed; else @@ -1377,7 +1484,21 @@ int intel_engine_cmd_parser(struct intel_engine_cs *engine, goto err; } + if (desc->cmd.value == MI_BATCH_BUFFER_START) { + ret = check_bbstart(ctx, cmd, offset, length, + batch_len, batch_start, + shadow_batch_start); + + if (ret) + goto err; + break; + } + + if (ctx->jump_whitelist_cmds > offset) + set_bit(offset, ctx->jump_whitelist); + cmd += length; + offset += length; if (cmd >= batch_end) { DRM_DEBUG_DRIVER("CMD: Got to the end of the buffer w/o a BBE cmd!\n"); ret = -EINVAL; diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 88f4d270d656..6193369fdbc1 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -943,6 +943,13 @@ struct i915_gem_context { struct list_head link; u8 remap_slice; + + /** jump_whitelist: Bit array for tracking cmds during cmdparsing */ + unsigned long *jump_whitelist; + + /** jump_whitelist_cmds: No of cmd slots available */ + u32 jump_whitelist_cmds; + bool closed:1; }; @@ -3483,11 +3490,14 @@ const char *i915_cache_level_str(struct drm_i915_private *i915, int type); int i915_cmd_parser_get_version(struct drm_i915_private *dev_priv); void intel_engine_init_cmd_parser(struct intel_engine_cs *engine); void intel_engine_cleanup_cmd_parser(struct intel_engine_cs *engine); -int intel_engine_cmd_parser(struct intel_engine_cs *engine, +int intel_engine_cmd_parser(struct i915_gem_context *cxt, + struct intel_engine_cs *engine, struct drm_i915_gem_object *batch_obj, - struct drm_i915_gem_object *shadow_batch_obj, + u64 user_batch_start, u32 batch_start_offset, - u32 batch_len); + u32 batch_len, + struct drm_i915_gem_object *shadow_batch_obj, + u64 shadow_batch_start); /* i915_suspend.c */ extern int i915_save_state(struct drm_device *dev); diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c index df10f4e95736..5d55cd159e89 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.c +++ b/drivers/gpu/drm/i915/i915_gem_context.c @@ -158,6 +158,8 @@ void i915_gem_context_free(struct kref *ctx_ref) i915_vma_put(ce->state); } + kfree(ctx->jump_whitelist); + put_pid(ctx->pid); list_del(&ctx->link); @@ -327,6 +329,9 @@ __create_hw_context(struct drm_device *dev, GEN8_CTX_ADDRESSING_MODE_SHIFT; ATOMIC_INIT_NOTIFIER_HEAD(&ctx->status_notifier); + ctx->jump_whitelist = NULL; + ctx->jump_whitelist_cmds = 0; + return ctx; err_out: diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index 2240aba5f7ab..4548d89abcdc 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -1429,27 +1429,44 @@ shadow_batch_pin(struct drm_i915_gem_object *obj, struct i915_address_space *vm) static struct i915_vma * i915_gem_execbuffer_parse(struct intel_engine_cs *engine, struct drm_i915_gem_exec_object2 *shadow_exec_entry, - struct drm_i915_gem_object *batch_obj, + struct i915_execbuffer_params *params, struct eb_vmas *eb, - struct i915_address_space *vm, - u32 batch_start_offset, - u32 batch_len) + struct i915_address_space *vm) { + struct drm_i915_gem_object *batch_obj = params->batch->obj; struct drm_i915_gem_object *shadow_batch_obj; struct i915_vma *vma; + u64 batch_start; + u32 batch_start_offset = params->args_batch_start_offset; + u32 batch_len = params->args_batch_len; + u64 shadow_batch_start; int ret; + shadow_batch_obj = i915_gem_batch_pool_get(&engine->batch_pool, PAGE_ALIGN(batch_len)); if (IS_ERR(shadow_batch_obj)) return ERR_CAST(shadow_batch_obj); - ret = intel_engine_cmd_parser(engine, + vma = shadow_batch_pin(shadow_batch_obj, vm); + if (IS_ERR(vma)) + goto out; + + batch_start = gen8_canonical_addr(params->batch->node.start) + + batch_start_offset; + shadow_batch_start = gen8_canonical_addr(vma->node.start); + + ret = intel_engine_cmd_parser(params->ctx, + engine, batch_obj, - shadow_batch_obj, + batch_start, batch_start_offset, - batch_len); + batch_len, + shadow_batch_obj, + shadow_batch_start); if (ret) { + i915_vma_unpin(vma); + /* * Unsafe GGTT-backed buffers can still be submitted safely * as non-secure. @@ -1461,12 +1478,9 @@ i915_gem_execbuffer_parse(struct intel_engine_cs *engine, vma = NULL; else vma = ERR_PTR(ret); - goto out; - } - vma = shadow_batch_pin(shadow_batch_obj, vm); - if (IS_ERR(vma)) goto out; + } memset(shadow_exec_entry, 0, sizeof(*shadow_exec_entry)); @@ -1745,6 +1759,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data, goto err; } + params->ctx = ctx; params->args_batch_start_offset = args->batch_start_offset; params->args_batch_len = args->batch_len; if (args->batch_len == 0) @@ -1755,10 +1770,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data, struct i915_vma *vma; vma = i915_gem_execbuffer_parse(engine, &shadow_exec_entry, - params->batch->obj, - eb, vm, - params->args_batch_start_offset, - params->args_batch_len); + params, eb, vm); if (IS_ERR(vma)) { ret = PTR_ERR(vma); goto err; @@ -1829,7 +1841,6 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data, params->file = file; params->engine = engine; params->dispatch_flags = dispatch_flags; - params->ctx = ctx; ret = execbuf_submit(params, args, &eb->vmas); err_request: |