Module: Mesa
Branch: main
Commit: 250605c57d8eb01c818cf639e412ca2f7cf4b00a
URL:    
http://cgit.freedesktop.org/mesa/mesa/commit/?id=250605c57d8eb01c818cf639e412ca2f7cf4b00a

Author: Jose Fonseca <[email protected]>
Date:   Fri May 14 16:25:32 2021 +0100

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 <[email protected]>
Cc: 21.0 21.1 <[email protected]>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/10836>

---

 src/gallium/auxiliary/draw/draw_gs.c               |  3 +-
 src/gallium/auxiliary/draw/draw_pipe_util.c        |  4 +--
 src/gallium/auxiliary/draw/draw_prim_assembler.c   |  2 +-
 src/gallium/auxiliary/draw/draw_private.h          | 37 ++++++++++++++++++++--
 .../auxiliary/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,

_______________________________________________
mesa-commit mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/mesa-commit

Reply via email to