From 4e86f725cebc8164e5f6601707379dd51440269d Mon Sep 17 00:00:00 2001 From: Francisco Jerez Date: Fri, 29 May 2015 16:44:14 +0300 Subject: [PATCH] drm/i915: Extend the parser to check register writes against a mask/value pair. In some cases it might be unnecessary or dangerous to give userspace the right to write arbitrary values to some register, even though it might be desirable to give it control of some of its bits. This patch extends the register whitelist entries to contain a mask/value pair in addition to the register offset. For registers with non-zero mask, any LRM writes and LRI writes where the bits of the immediate given by the mask don't match the specified value will be rejected. This will be used in my next patch to grant userspace partial write access to some sensitive registers. Signed-off-by: Francisco Jerez Reviewed-by: Zhigang Gong Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/i915_cmd_parser.c | 138 ++++++++++++++++-------- drivers/gpu/drm/i915/intel_ringbuffer.h | 5 +- 2 files changed, 96 insertions(+), 47 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c b/drivers/gpu/drm/i915/i915_cmd_parser.c index 5fc49bbcdb9d..cafa3e2f16fc 100644 --- a/drivers/gpu/drm/i915/i915_cmd_parser.c +++ b/drivers/gpu/drm/i915/i915_cmd_parser.c @@ -395,16 +395,38 @@ static const struct drm_i915_cmd_table hsw_blt_ring_cmds[] = { /* * Register whitelists, sorted by increasing register offset. + */ + +/* + * An individual whitelist entry granting access to register addr. If + * mask is non-zero the argument of immediate register writes will be + * AND-ed with mask, and the command will be rejected if the result + * doesn't match value. + * + * Registers with non-zero mask are only allowed to be written using + * LRI. + */ +struct drm_i915_reg_descriptor { + u32 addr; + u32 mask; + u32 value; +}; + +/* Convenience macro for adding 32-bit registers. */ +#define REG32(address, ...) \ + { .addr = address, __VA_ARGS__ } + +/* + * Convenience macro for adding 64-bit registers. * * Some registers that userspace accesses are 64 bits. The register * access commands only allow 32-bit accesses. Hence, we have to include * entries for both halves of the 64-bit registers. */ +#define REG64(addr) \ + REG32(addr), REG32(addr + sizeof(u32)) -/* Convenience macro for adding 64-bit registers */ -#define REG64(addr) (addr), (addr + sizeof(u32)) - -static const u32 gen7_render_regs[] = { +static const struct drm_i915_reg_descriptor gen7_render_regs[] = { REG64(GPGPU_THREADS_DISPATCHED), REG64(HS_INVOCATION_COUNT), REG64(DS_INVOCATION_COUNT), @@ -417,15 +439,15 @@ static const u32 gen7_render_regs[] = { REG64(CL_PRIMITIVES_COUNT), REG64(PS_INVOCATION_COUNT), REG64(PS_DEPTH_COUNT), - OACONTROL, /* Only allowed for LRI and SRM. See below. */ + REG32(OACONTROL), /* Only allowed for LRI and SRM. See below. */ REG64(MI_PREDICATE_SRC0), REG64(MI_PREDICATE_SRC1), - GEN7_3DPRIM_END_OFFSET, - GEN7_3DPRIM_START_VERTEX, - GEN7_3DPRIM_VERTEX_COUNT, - GEN7_3DPRIM_INSTANCE_COUNT, - GEN7_3DPRIM_START_INSTANCE, - GEN7_3DPRIM_BASE_VERTEX, + REG32(GEN7_3DPRIM_END_OFFSET), + REG32(GEN7_3DPRIM_START_VERTEX), + REG32(GEN7_3DPRIM_VERTEX_COUNT), + REG32(GEN7_3DPRIM_INSTANCE_COUNT), + REG32(GEN7_3DPRIM_START_INSTANCE), + REG32(GEN7_3DPRIM_BASE_VERTEX), REG64(GEN7_SO_NUM_PRIMS_WRITTEN(0)), REG64(GEN7_SO_NUM_PRIMS_WRITTEN(1)), REG64(GEN7_SO_NUM_PRIMS_WRITTEN(2)), @@ -434,33 +456,34 @@ static const u32 gen7_render_regs[] = { REG64(GEN7_SO_PRIM_STORAGE_NEEDED(1)), REG64(GEN7_SO_PRIM_STORAGE_NEEDED(2)), REG64(GEN7_SO_PRIM_STORAGE_NEEDED(3)), - GEN7_SO_WRITE_OFFSET(0), - GEN7_SO_WRITE_OFFSET(1), - GEN7_SO_WRITE_OFFSET(2), - GEN7_SO_WRITE_OFFSET(3), - GEN7_L3SQCREG1, - GEN7_L3CNTLREG2, - GEN7_L3CNTLREG3, + REG32(GEN7_SO_WRITE_OFFSET(0)), + REG32(GEN7_SO_WRITE_OFFSET(1)), + REG32(GEN7_SO_WRITE_OFFSET(2)), + REG32(GEN7_SO_WRITE_OFFSET(3)), + REG32(GEN7_L3SQCREG1), + REG32(GEN7_L3CNTLREG2), + REG32(GEN7_L3CNTLREG3), }; -static const u32 gen7_blt_regs[] = { - BCS_SWCTRL, +static const struct drm_i915_reg_descriptor gen7_blt_regs[] = { + REG32(BCS_SWCTRL), }; -static const u32 ivb_master_regs[] = { - FORCEWAKE_MT, - DERRMR, - GEN7_PIPE_DE_LOAD_SL(PIPE_A), - GEN7_PIPE_DE_LOAD_SL(PIPE_B), - GEN7_PIPE_DE_LOAD_SL(PIPE_C), +static const struct drm_i915_reg_descriptor ivb_master_regs[] = { + REG32(FORCEWAKE_MT), + REG32(DERRMR), + REG32(GEN7_PIPE_DE_LOAD_SL(PIPE_A)), + REG32(GEN7_PIPE_DE_LOAD_SL(PIPE_B)), + REG32(GEN7_PIPE_DE_LOAD_SL(PIPE_C)), }; -static const u32 hsw_master_regs[] = { - FORCEWAKE_MT, - DERRMR, +static const struct drm_i915_reg_descriptor hsw_master_regs[] = { + REG32(FORCEWAKE_MT), + REG32(DERRMR), }; #undef REG64 +#undef REG32 static u32 gen7_render_get_cmd_length_mask(u32 cmd_header) { @@ -550,14 +573,16 @@ static bool validate_cmds_sorted(struct intel_engine_cs *ring, return ret; } -static bool check_sorted(int ring_id, const u32 *reg_table, int reg_count) +static bool check_sorted(int ring_id, + const struct drm_i915_reg_descriptor *reg_table, + int reg_count) { int i; u32 previous = 0; bool ret = true; for (i = 0; i < reg_count; i++) { - u32 curr = reg_table[i]; + u32 curr = reg_table[i].addr; if (curr < previous) { DRM_ERROR("CMD: table not sorted ring=%d entry=%d reg=0x%08X prev=0x%08X\n", @@ -804,18 +829,20 @@ find_cmd(struct intel_engine_cs *ring, return default_desc; } -static bool valid_reg(const u32 *table, int count, u32 addr) +static const struct drm_i915_reg_descriptor * +find_reg(const struct drm_i915_reg_descriptor *table, + int count, u32 addr) { - if (table && count != 0) { + if (table) { int i; for (i = 0; i < count; i++) { - if (table[i] == addr) - return true; + if (table[i].addr == addr) + return &table[i]; } } - return false; + return NULL; } static u32 *vmap_batch(struct drm_i915_gem_object *obj, @@ -961,6 +988,20 @@ static bool check_cmd(const struct intel_engine_cs *ring, for (offset = desc->reg.offset; offset < length; offset += step) { const u32 reg_addr = cmd[offset] & desc->reg.mask; + const struct drm_i915_reg_descriptor *reg = + find_reg(ring->reg_table, ring->reg_count, + reg_addr); + + if (!reg && is_master) + reg = find_reg(ring->master_reg_table, + ring->master_reg_count, + reg_addr); + + if (!reg) { + DRM_DEBUG_DRIVER("CMD: Rejected register 0x%08X in command: 0x%08X (ring=%d)\n", + reg_addr, *cmd, ring->id); + return false; + } /* * OACONTROL requires some special handling for @@ -982,15 +1023,22 @@ static bool check_cmd(const struct intel_engine_cs *ring, *oacontrol_set = (cmd[offset + 1] != 0); } - if (!valid_reg(ring->reg_table, - ring->reg_count, reg_addr)) { - if (!is_master || - !valid_reg(ring->master_reg_table, - ring->master_reg_count, - reg_addr)) { - DRM_DEBUG_DRIVER("CMD: Rejected register 0x%08X in command: 0x%08X (ring=%d)\n", - reg_addr, *cmd, - ring->id); + /* + * Check the value written to the register against the + * allowed mask/value pair given in the whitelist entry. + */ + if (reg->mask) { + if (desc->cmd.value == MI_LOAD_REGISTER_MEM) { + DRM_DEBUG_DRIVER("CMD: Rejected LRM to masked register 0x%08X\n", + reg_addr); + return false; + } + + if (desc->cmd.value == MI_LOAD_REGISTER_IMM(1) && + (offset + 2 > length || + (cmd[offset + 1] & reg->mask) != reg->value)) { + DRM_DEBUG_DRIVER("CMD: Rejected LRI to masked register 0x%08X\n", + reg_addr); return false; } } diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h index 39f6dfc0ee54..e539314ae87e 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.h +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h @@ -118,6 +118,7 @@ struct intel_ringbuffer { }; struct intel_context; +struct drm_i915_reg_descriptor; struct intel_engine_cs { const char *name; @@ -300,14 +301,14 @@ struct intel_engine_cs { /* * Table of registers allowed in commands that read/write registers. */ - const u32 *reg_table; + const struct drm_i915_reg_descriptor *reg_table; int reg_count; /* * Table of registers allowed in commands that read/write registers, but * only from the DRM master. */ - const u32 *master_reg_table; + const struct drm_i915_reg_descriptor *master_reg_table; int master_reg_count; /* -- 2.20.1