From 250605c57d8eb01c818cf639e412ca2f7cf4b00a Mon Sep 17 00:00:00 2001 From: Jose Fonseca Date: Fri, 14 May 2021 16:25:32 +0100 Subject: [PATCH] draw: Allocate extra padding for extra shader outputs. This prevents read buffer overflows in dup_vertex(), when draw stages allocate extra shader outputs after the vertex buffers are allocated. The original issue can be exercised with upcoming piglit/tests/general/vertex-fallbacks.c test. Reviewed-by: Roland Scheidegger Cc: 21.0 21.1 Part-of: --- src/gallium/auxiliary/draw/draw_gs.c | 3 +- src/gallium/auxiliary/draw/draw_pipe_util.c | 4 +- .../auxiliary/draw/draw_prim_assembler.c | 2 +- src/gallium/auxiliary/draw/draw_private.h | 37 +++++++++++++++++-- .../draw/draw_pt_fetch_shade_pipeline.c | 6 ++- .../draw/draw_pt_fetch_shade_pipeline_llvm.c | 3 +- src/gallium/auxiliary/draw/draw_vs_variant.c | 6 ++- 7 files changed, 49 insertions(+), 12 deletions(-) diff --git a/src/gallium/auxiliary/draw/draw_gs.c b/src/gallium/auxiliary/draw/draw_gs.c index 952e540337c..ed698e920d8 100644 --- a/src/gallium/auxiliary/draw/draw_gs.c +++ b/src/gallium/auxiliary/draw/draw_gs.c @@ -589,7 +589,8 @@ int draw_geometry_shader_run(struct draw_geometry_shader *shader, output_verts[i].stride = output_verts[i].vertex_size; output_verts[i].verts = (struct vertex_header *)MALLOC(output_verts[i].vertex_size * - total_verts_per_buffer * shader->num_invocations); + total_verts_per_buffer * shader->num_invocations + + DRAW_EXTRA_VERTICES_PADDING); debug_assert(output_verts[i].verts); } diff --git a/src/gallium/auxiliary/draw/draw_pipe_util.c b/src/gallium/auxiliary/draw/draw_pipe_util.c index 53a42a6a0e4..c2cb156a5ee 100644 --- a/src/gallium/auxiliary/draw/draw_pipe_util.c +++ b/src/gallium/auxiliary/draw/draw_pipe_util.c @@ -76,8 +76,8 @@ boolean draw_alloc_temp_verts( struct draw_stage *stage, unsigned nr ) if (nr != 0) { unsigned i; - ubyte *store = (ubyte *) MALLOC( MAX_VERTEX_SIZE * nr ); - + ubyte *store = (ubyte *) MALLOC( MAX_VERTEX_SIZE * nr + + DRAW_EXTRA_VERTICES_PADDING ); if (!store) return FALSE; diff --git a/src/gallium/auxiliary/draw/draw_prim_assembler.c b/src/gallium/auxiliary/draw/draw_prim_assembler.c index f7750bfde34..e628a143d44 100644 --- a/src/gallium/auxiliary/draw/draw_prim_assembler.c +++ b/src/gallium/auxiliary/draw/draw_prim_assembler.c @@ -268,7 +268,7 @@ draw_prim_assembler_run(struct draw_context *draw, output_verts->vertex_size = input_verts->vertex_size; output_verts->stride = input_verts->stride; output_verts->verts = (struct vertex_header*)MALLOC( - input_verts->vertex_size * max_verts); + input_verts->vertex_size * max_verts + DRAW_EXTRA_VERTICES_PADDING); output_verts->count = 0; diff --git a/src/gallium/auxiliary/draw/draw_private.h b/src/gallium/auxiliary/draw/draw_private.h index d2a6ea56363..7335e0cb24a 100644 --- a/src/gallium/auxiliary/draw/draw_private.h +++ b/src/gallium/auxiliary/draw/draw_private.h @@ -59,6 +59,37 @@ struct gallivm_state; */ #define DRAW_MAX_FETCH_IDX 0xffffffff +/** + * Maximum number of extra shader outputs. These are allocated by: + * - draw_pipe_aaline.c (1) + * - draw_pipe_aapoint.c (1) + * - draw_pipe_unfilled.c (1) + * - draw_pipe_wide_point.c (up to 32) + * - draw_prim_assembler.c (1) + */ +#define DRAW_MAX_EXTRA_SHADER_OUTPUTS 32 + +/** + * Despite some efforts to determine the number of extra shader outputs ahead + * of time, the matter of fact is that this number will vary as primitives + * flow through the draw pipeline. In particular, aaline/aapoint stages + * only allocate their extra shader outputs on the first line/point. + * + * Consequently dup_vert() ends up copying vertices larger than those + * allocated. + * + * Ideally we'd keep track of incoming/outgoing vertex sizes (and strides) + * throughout the draw pipeline, but unfortunately we recompute these all over + * the place, so preemptively expanding the vertex stride/size does not work + * as mismatches ensue. + * + * As stopgap to prevent buffer read overflows, we allocate an extra bit of + * padding at the end of temporary vertex buffers, allowing dup_vert() to copy + * more vertex attributes than allocated. + */ +#define DRAW_EXTRA_VERTICES_PADDING \ + (DRAW_MAX_EXTRA_SHADER_OUTPUTS * sizeof(float[4])) + struct pipe_context; struct draw_vertex_shader; struct draw_context; @@ -361,9 +392,9 @@ struct draw_context */ struct { uint num; - uint semantic_name[10]; - uint semantic_index[10]; - uint slot[10]; + uint semantic_name[DRAW_MAX_EXTRA_SHADER_OUTPUTS]; + uint semantic_index[DRAW_MAX_EXTRA_SHADER_OUTPUTS]; + uint slot[DRAW_MAX_EXTRA_SHADER_OUTPUTS]; } extra_shader_outputs; unsigned instance_id; diff --git a/src/gallium/auxiliary/draw/draw_pt_fetch_shade_pipeline.c b/src/gallium/auxiliary/draw/draw_pt_fetch_shade_pipeline.c index 07838fb7eda..2b5cb7dc0d8 100644 --- a/src/gallium/auxiliary/draw/draw_pt_fetch_shade_pipeline.c +++ b/src/gallium/auxiliary/draw/draw_pt_fetch_shade_pipeline.c @@ -214,7 +214,8 @@ draw_vertex_shader_run(struct draw_vertex_shader *vshader, output_verts->count = input_verts->count; output_verts->verts = (struct vertex_header *)MALLOC(output_verts->vertex_size * - align(output_verts->count, 4)); + align(output_verts->count, 4) + + DRAW_EXTRA_VERTICES_PADDING); vshader->run_linear(vshader, (const float (*)[4])input_verts->verts->data, @@ -254,7 +255,8 @@ fetch_pipeline_generic(struct draw_pt_middle_end *middle, fetched_vert_info.stride = fpme->vertex_size; fetched_vert_info.verts = (struct vertex_header *)MALLOC(fpme->vertex_size * - align(fetch_info->count, 4)); + align(fetch_info->count, 4) + + DRAW_EXTRA_VERTICES_PADDING); if (!fetched_vert_info.verts) { assert(0); return; diff --git a/src/gallium/auxiliary/draw/draw_pt_fetch_shade_pipeline_llvm.c b/src/gallium/auxiliary/draw/draw_pt_fetch_shade_pipeline_llvm.c index cca0bcc627c..fe1734a9a37 100644 --- a/src/gallium/auxiliary/draw/draw_pt_fetch_shade_pipeline_llvm.c +++ b/src/gallium/auxiliary/draw/draw_pt_fetch_shade_pipeline_llvm.c @@ -595,7 +595,8 @@ llvm_pipeline_generic(struct draw_pt_middle_end *middle, llvm_vert_info.stride = fpme->vertex_size; llvm_vert_info.verts = (struct vertex_header *) MALLOC(fpme->vertex_size * - align(fetch_info->count, lp_native_vector_width / 32)); + align(fetch_info->count, lp_native_vector_width / 32) + + DRAW_EXTRA_VERTICES_PADDING); if (!llvm_vert_info.verts) { assert(0); return; diff --git a/src/gallium/auxiliary/draw/draw_vs_variant.c b/src/gallium/auxiliary/draw/draw_vs_variant.c index 44cf29b8e47..bc320cb1cd8 100644 --- a/src/gallium/auxiliary/draw/draw_vs_variant.c +++ b/src/gallium/auxiliary/draw/draw_vs_variant.c @@ -158,7 +158,8 @@ static void PIPE_CDECL vsvg_run_elts( struct draw_vs_variant *variant, { struct draw_vs_variant_generic *vsvg = (struct draw_vs_variant_generic *)variant; unsigned temp_vertex_stride = vsvg->temp_vertex_stride; - void *temp_buffer = MALLOC( align(count,4) * temp_vertex_stride ); + void *temp_buffer = MALLOC( align(count,4) * temp_vertex_stride + + DRAW_EXTRA_VERTICES_PADDING ); if (0) debug_printf("%s %d \n", __FUNCTION__, count); @@ -227,7 +228,8 @@ static void PIPE_CDECL vsvg_run_linear( struct draw_vs_variant *variant, { struct draw_vs_variant_generic *vsvg = (struct draw_vs_variant_generic *)variant; unsigned temp_vertex_stride = vsvg->temp_vertex_stride; - void *temp_buffer = MALLOC( align(count,4) * temp_vertex_stride ); + void *temp_buffer = MALLOC( align(count,4) * temp_vertex_stride + + DRAW_EXTRA_VERTICES_PADDING ); if (0) debug_printf("%s %d %d (sz %d, %d)\n", __FUNCTION__, start, count, vsvg->base.key.output_stride,