From b6caa9556c7c1cf9206ac610fb57587ad119d3ab Mon Sep 17 00:00:00 2001 From: Eric Anholt Date: Fri, 1 Aug 2014 20:23:31 -0700 Subject: [PATCH] vc4: Validate that the same BO doesn't get reused for different purposes. We don't care if things like vertex data get smashed by render target data, but we do need to make sure that shader code doesn't get rendered to. v2: Fix overflowing read of gl_relocs[] that incorrect flagged of some VBOs as shader code. --- src/gallium/drivers/vc4/vc4_simulator.c | 7 +- .../drivers/vc4/vc4_simulator_validate.c | 89 +++++++++++++------ .../drivers/vc4/vc4_simulator_validate.h | 22 +++-- 3 files changed, 81 insertions(+), 37 deletions(-) diff --git a/src/gallium/drivers/vc4/vc4_simulator.c b/src/gallium/drivers/vc4/vc4_simulator.c index b2f2b669dc9..88eda4fe722 100644 --- a/src/gallium/drivers/vc4/vc4_simulator.c +++ b/src/gallium/drivers/vc4/vc4_simulator.c @@ -70,16 +70,15 @@ vc4_simulator_pin_bos(struct drm_device *dev, struct exec_info *exec) struct vc4_bo **bos = vc4->bo_pointers.base; exec->bo_count = args->bo_handle_count; - exec->bo = calloc(exec->bo_count, sizeof(void *)); + exec->bo = calloc(exec->bo_count, sizeof(struct vc4_bo_exec_state)); for (int i = 0; i < exec->bo_count; i++) { struct vc4_bo *bo = bos[i]; struct drm_gem_cma_object *obj = vc4_wrap_bo_with_cma(dev, bo); memcpy(obj->vaddr, bo->map, bo->size); - exec->bo[i] = obj; + exec->bo[i].bo = obj; } - return 0; } @@ -87,7 +86,7 @@ static int vc4_simulator_unpin_bos(struct exec_info *exec) { for (int i = 0; i < exec->bo_count; i++) { - struct drm_gem_cma_object *obj = exec->bo[i]; + struct drm_gem_cma_object *obj = exec->bo[i].bo; struct vc4_bo *bo = obj->bo; memcpy(bo->map, obj->vaddr, bo->size); diff --git a/src/gallium/drivers/vc4/vc4_simulator_validate.c b/src/gallium/drivers/vc4/vc4_simulator_validate.c index fafb45fa8fd..3db74a0b18b 100644 --- a/src/gallium/drivers/vc4/vc4_simulator_validate.c +++ b/src/gallium/drivers/vc4/vc4_simulator_validate.c @@ -47,6 +47,44 @@ void *validated, \ void *untrusted +static bool +vc4_use_bo(struct exec_info *exec, + uint32_t hindex, + enum vc4_bo_mode mode, + struct drm_gem_cma_object **obj) +{ + *obj = NULL; + + if (hindex >= exec->bo_count) { + DRM_ERROR("BO index %d greater than BO count %d\n", + hindex, exec->bo_count); + return false; + } + + if (exec->bo[hindex].mode != mode) { + if (exec->bo[hindex].mode == VC4_MODE_UNDECIDED) { + exec->bo[hindex].mode = mode; + } else { + DRM_ERROR("BO index %d reused with mode %d vs %d\n", + hindex, exec->bo[hindex].mode, mode); + return false; + } + } + + *obj = exec->bo[hindex].bo; + return true; +} + +static bool +vc4_use_handle(struct exec_info *exec, + uint32_t gem_handles_packet_index, + enum vc4_bo_mode mode, + struct drm_gem_cma_object **obj) +{ + return vc4_use_bo(exec, exec->bo_index[gem_handles_packet_index], + mode, obj); +} + static uint32_t gl_shader_rec_size(uint32_t pointer_bits) { @@ -66,7 +104,8 @@ validate_branch_to_sublist(VALIDATE_ARGS) /* XXX: Validate address jumped to */ - target = exec->bo[exec->bo_index[0]]; + if (!vc4_use_handle(exec, 0, VC4_MODE_TILE_ALLOC, &target)) + return -EINVAL; *(uint32_t *)(validated + 0) = *(uint32_t *)(untrusted + 0) + target->paddr; @@ -78,11 +117,14 @@ static int validate_loadstore_tile_buffer_general(VALIDATE_ARGS) { uint32_t packet_b0 = *(uint8_t *)(untrusted + 0); - struct drm_gem_cma_object *fbo = exec->bo[exec->bo_index[0]]; + struct drm_gem_cma_object *fbo; if ((packet_b0 & 0xf) == VC4_LOADSTORE_TILE_BUFFER_NONE) return 0; + if (!vc4_use_handle(exec, 0, VC4_MODE_RENDER, &fbo)) + return -EINVAL; + /* XXX: Validate address offset */ *(uint32_t *)(validated + 2) = *(uint32_t *)(untrusted + 2) + fbo->paddr; @@ -109,7 +151,9 @@ validate_indexed_prim_list(VALIDATE_ARGS) return -EINVAL; } - ib = exec->bo[exec->bo_index[0]]; + + if (!vc4_use_handle(exec, 0, VC4_MODE_RENDER, &ib)) + return -EINVAL; if (ib_access_end > ib->base.size) { DRM_ERROR("IB access out of bounds (%d/%d)\n", ib_access_end, ib->base.size); @@ -180,8 +224,9 @@ validate_tile_binning_config(VALIDATE_ARGS) struct drm_gem_cma_object *tile_allocation; struct drm_gem_cma_object *tile_state_data_array; - tile_allocation = exec->bo[exec->bo_index[0]]; - tile_state_data_array = exec->bo[exec->bo_index[1]]; + if (!vc4_use_handle(exec, 0, VC4_MODE_TILE_ALLOC, &tile_allocation) || + !vc4_use_handle(exec, 1, VC4_MODE_TSDA, &tile_state_data_array)) + return -EINVAL; /* XXX: Validate offsets */ *(uint32_t *)validated = @@ -198,7 +243,8 @@ validate_tile_rendering_mode_config(VALIDATE_ARGS) { struct drm_gem_cma_object *fbo; - fbo = exec->bo[exec->bo_index[0]]; + if (!vc4_use_handle(exec, 0, VC4_MODE_RENDER, &fbo)) + return -EINVAL; /* XXX: Validate offsets */ *(uint32_t *)validated = @@ -210,18 +256,7 @@ validate_tile_rendering_mode_config(VALIDATE_ARGS) static int validate_gem_handles(VALIDATE_ARGS) { - int i; - memcpy(exec->bo_index, untrusted, sizeof(exec->bo_index)); - - for (i = 0; i < ARRAY_SIZE(exec->bo_index); i++) { - if (exec->bo_index[i] >= exec->bo_count) { - DRM_ERROR("Validated BO index %d >= %d\n", - exec->bo_index[i], exec->bo_count); - return -EINVAL; - } - } - return 0; } @@ -379,12 +414,8 @@ reloc_tex(struct exec_info *exec, sample->p_offset[0]); uint32_t *validated_p0 = exec->uniforms_v + sample->p_offset[0]; - if (texture_handle_index >= exec->bo_count) { - DRM_ERROR("texture handle index %d >= %d\n", - texture_handle_index, exec->bo_count); + if (!vc4_use_bo(exec, texture_handle_index, VC4_MODE_RENDER, &tex)) return false; - } - tex = exec->bo[texture_handle_index]; *validated_p0 = tex->paddr + unvalidated_p0; @@ -467,12 +498,16 @@ validate_shader_rec(struct drm_device *dev, exec->shader_rec_size -= packet_size; for (i = 0; i < nr_relocs; i++) { - if (src_handles[i] >= exec->bo_count) { - DRM_ERROR("shader rec bo index %d > %d\n", - src_handles[i], exec->bo_count); - return -EINVAL; + enum vc4_bo_mode mode; + + if (i < nr_fixed_relocs && relocs[i].type == RELOC_CODE) + mode = VC4_MODE_SHADER; + else + mode = VC4_MODE_RENDER; + + if (!vc4_use_bo(exec, src_handles[i], mode, &bo[i])) { + return false; } - bo[i] = exec->bo[src_handles[i]]; } for (i = 0; i < nr_fixed_relocs; i++) { diff --git a/src/gallium/drivers/vc4/vc4_simulator_validate.h b/src/gallium/drivers/vc4/vc4_simulator_validate.h index 52f664adbc1..1f3d23f881e 100644 --- a/src/gallium/drivers/vc4/vc4_simulator_validate.h +++ b/src/gallium/drivers/vc4/vc4_simulator_validate.h @@ -69,6 +69,19 @@ struct drm_gem_cma_object { void *vaddr; }; +enum vc4_bo_mode { + VC4_MODE_UNDECIDED, + VC4_MODE_TILE_ALLOC, + VC4_MODE_TSDA, + VC4_MODE_RENDER, + VC4_MODE_SHADER, +}; + +struct vc4_bo_exec_state { + struct drm_gem_cma_object *bo; + enum vc4_bo_mode mode; +}; + struct exec_info { /* Kernel-space copy of the ioctl arguments */ struct drm_vc4_submit_cl *args; @@ -76,14 +89,11 @@ struct exec_info { /* This is the array of BOs that were looked up at the start of exec. * Command validation will use indices into this array. */ - struct drm_gem_cma_object **bo; + struct vc4_bo_exec_state *bo; uint32_t bo_count; - /* Current indices into @bo loaded by the non-hardware packet - * that passes in indices. This can be used even without - * checking that we've seen one of those packets, because - * @bo_count is always >= 1, and this struct is initialized to - * 0. + /* Current unvalidated indices into @bo loaded by the non-hardware + * VC4_PACKET_GEM_HANDLES. */ uint32_t bo_index[2]; uint32_t max_width, max_height; -- 2.30.2