Re: [Mesa-dev] [PATCH] draw: Prevent out-of-bounds vertex buffer access.

[Keith Whitwell]

Looks good to me, Jose.

Keith

On Thu, 2011-03-31 at 14:45 +0100, jfons...@vmware.com wrote:
> From: José Fonseca 
> 
> Based on some code and ideas from Keith Whitwell.
> ---
>  src/gallium/auxiliary/Makefile |1 +
>  src/gallium/auxiliary/SConscript   |1 +
>  src/gallium/auxiliary/draw/draw_private.h  |8 ++
>  src/gallium/auxiliary/draw/draw_pt.c   |   11 +++
>  src/gallium/auxiliary/draw/draw_pt_fetch.c |2 +-
>  src/gallium/auxiliary/draw/draw_pt_fetch_emit.c|2 +-
>  .../auxiliary/draw/draw_pt_fetch_shade_emit.c  |2 +-
>  src/gallium/auxiliary/draw/draw_pt_vsplit.c|7 ++-
>  src/gallium/auxiliary/draw/draw_pt_vsplit_tmp.h|   12 ++-
>  src/gallium/auxiliary/util/u_draw.c|   94 
> 
>  src/gallium/auxiliary/util/u_draw.h|   19 
>  11 files changed, 152 insertions(+), 7 deletions(-)
>  create mode 100644 src/gallium/auxiliary/util/u_draw.c
> 
> diff --git a/src/gallium/auxiliary/Makefile b/src/gallium/auxiliary/Makefile
> index c765404..2be4509 100644
> --- a/src/gallium/auxiliary/Makefile
> +++ b/src/gallium/auxiliary/Makefile
> @@ -107,6 +107,7 @@ C_SOURCES = \
>   util/u_caps.c \
>   util/u_cpu_detect.c \
>   util/u_dl.c \
> + util/u_draw.c \
>   util/u_draw_quad.c \
>   util/u_format.c \
>   util/u_format_other.c \
> diff --git a/src/gallium/auxiliary/SConscript 
> b/src/gallium/auxiliary/SConscript
> index 8e422b2..96ca566 100644
> --- a/src/gallium/auxiliary/SConscript
> +++ b/src/gallium/auxiliary/SConscript
> @@ -154,6 +154,7 @@ source = [
>  'util/u_dump_defines.c',
>  'util/u_dump_state.c',
>  'util/u_dl.c',
> +'util/u_draw.c',
>  'util/u_draw_quad.c',
>  'util/u_format.c',
>  'util/u_format_other.c',
> diff --git a/src/gallium/auxiliary/draw/draw_private.h 
> b/src/gallium/auxiliary/draw/draw_private.h
> index db2e3c5..b7d693f 100644
> --- a/src/gallium/auxiliary/draw/draw_private.h
> +++ b/src/gallium/auxiliary/draw/draw_private.h
> @@ -146,6 +146,14 @@ struct draw_context
>struct pipe_vertex_buffer vertex_buffer[PIPE_MAX_ATTRIBS];
>unsigned nr_vertex_buffers;
>  
> +  /*
> +   * This is the largest legal index value for the current set of
> +   * bound vertex buffers.  Regardless of any other consideration,
> +   * all vertex lookups need to be clamped to 0..max_index to
> +   * prevent out-of-bound access.
> +   */
> +  unsigned max_index;
> +
>struct pipe_vertex_element vertex_element[PIPE_MAX_ATTRIBS];
>unsigned nr_vertex_elements;
>  
> diff --git a/src/gallium/auxiliary/draw/draw_pt.c 
> b/src/gallium/auxiliary/draw/draw_pt.c
> index c3d7e87..e0eda67 100644
> --- a/src/gallium/auxiliary/draw/draw_pt.c
> +++ b/src/gallium/auxiliary/draw/draw_pt.c
> @@ -470,6 +470,17 @@ draw_vbo(struct draw_context *draw,
> if (0)
>draw_print_arrays(draw, info->mode, info->start, MIN2(info->count, 
> 20));
>  
> +   draw->pt.max_index = util_draw_max_index(draw->pt.vertex_buffer,
> +draw->pt.nr_vertex_buffers,
> +draw->pt.vertex_element,
> +draw->pt.nr_vertex_elements,
> +info);
> +
> +   /*
> +* TODO: We could use draw->pt.max_index to further narrow
> +* the min_index/max_index hints given by the state tracker.
> +*/
> +
> for (instance = 0; instance < info->instance_count; instance++) {
>draw->instance_id = instance + info->start_instance;
>  
> diff --git a/src/gallium/auxiliary/draw/draw_pt_fetch.c 
> b/src/gallium/auxiliary/draw/draw_pt_fetch.c
> index 4fa3b26..5589a82 100644
> --- a/src/gallium/auxiliary/draw/draw_pt_fetch.c
> +++ b/src/gallium/auxiliary/draw/draw_pt_fetch.c
> @@ -139,7 +139,7 @@ void draw_pt_fetch_run( struct pt_fetch *fetch,
>   ((char *)draw->pt.user.vbuffer[i] + 
>draw->pt.vertex_buffer[i].buffer_offset),
>   draw->pt.vertex_buffer[i].stride,
> - draw->pt.user.max_index);
> + draw->pt.max_index);
> }
>  
> translate->run_elts( translate,
> diff --git a/src/gallium/auxiliary/draw/draw_pt_fetch_emit.c 
> b/src/gallium/auxiliary/draw/draw_pt_fetch_emit.c
> index 5104310..0ab11d0 100644
> --- a/src/gallium/auxiliary/draw/draw_pt_fetch_emit.c
> +++ b/src/gallium/auxiliary/draw/draw_pt_fetch_emit.c
> @@ -186,7 +186,7 @@ static void fetch_emit_prepare( struct draw_pt_middle_end 
> *middle,
>((char *)draw->pt.user.vbuffer[i] + 
> draw->pt.vertex_buffer[i].buffer_offset),
>draw->pt.vertex_buffer[i].stride,
> -  draw->pt.user.max_index);

[Mesa-dev] [PATCH] draw: Prevent out-of-bounds vertex buffer access.

[jfonseca]

From: José Fonseca 

Based on some code and ideas from Keith Whitwell.
---
 src/gallium/auxiliary/Makefile |1 +
 src/gallium/auxiliary/SConscript   |1 +
 src/gallium/auxiliary/draw/draw_private.h  |8 ++
 src/gallium/auxiliary/draw/draw_pt.c   |   11 +++
 src/gallium/auxiliary/draw/draw_pt_fetch.c |2 +-
 src/gallium/auxiliary/draw/draw_pt_fetch_emit.c|2 +-
 .../auxiliary/draw/draw_pt_fetch_shade_emit.c  |2 +-
 src/gallium/auxiliary/draw/draw_pt_vsplit.c|7 ++-
 src/gallium/auxiliary/draw/draw_pt_vsplit_tmp.h|   12 ++-
 src/gallium/auxiliary/util/u_draw.c|   94 
 src/gallium/auxiliary/util/u_draw.h|   19 
 11 files changed, 152 insertions(+), 7 deletions(-)
 create mode 100644 src/gallium/auxiliary/util/u_draw.c

diff --git a/src/gallium/auxiliary/Makefile b/src/gallium/auxiliary/Makefile
index c765404..2be4509 100644
--- a/src/gallium/auxiliary/Makefile
+++ b/src/gallium/auxiliary/Makefile
@@ -107,6 +107,7 @@ C_SOURCES = \
util/u_caps.c \
util/u_cpu_detect.c \
util/u_dl.c \
+   util/u_draw.c \
util/u_draw_quad.c \
util/u_format.c \
util/u_format_other.c \
diff --git a/src/gallium/auxiliary/SConscript b/src/gallium/auxiliary/SConscript
index 8e422b2..96ca566 100644
--- a/src/gallium/auxiliary/SConscript
+++ b/src/gallium/auxiliary/SConscript
@@ -154,6 +154,7 @@ source = [
 'util/u_dump_defines.c',
 'util/u_dump_state.c',
 'util/u_dl.c',
+'util/u_draw.c',
 'util/u_draw_quad.c',
 'util/u_format.c',
 'util/u_format_other.c',
diff --git a/src/gallium/auxiliary/draw/draw_private.h 
b/src/gallium/auxiliary/draw/draw_private.h
index db2e3c5..b7d693f 100644
--- a/src/gallium/auxiliary/draw/draw_private.h
+++ b/src/gallium/auxiliary/draw/draw_private.h
@@ -146,6 +146,14 @@ struct draw_context
   struct pipe_vertex_buffer vertex_buffer[PIPE_MAX_ATTRIBS];
   unsigned nr_vertex_buffers;
 
+  /*
+   * This is the largest legal index value for the current set of
+   * bound vertex buffers.  Regardless of any other consideration,
+   * all vertex lookups need to be clamped to 0..max_index to
+   * prevent out-of-bound access.
+   */
+  unsigned max_index;
+
   struct pipe_vertex_element vertex_element[PIPE_MAX_ATTRIBS];
   unsigned nr_vertex_elements;
 
diff --git a/src/gallium/auxiliary/draw/draw_pt.c 
b/src/gallium/auxiliary/draw/draw_pt.c
index c3d7e87..e0eda67 100644
--- a/src/gallium/auxiliary/draw/draw_pt.c
+++ b/src/gallium/auxiliary/draw/draw_pt.c
@@ -470,6 +470,17 @@ draw_vbo(struct draw_context *draw,
if (0)
   draw_print_arrays(draw, info->mode, info->start, MIN2(info->count, 20));
 
+   draw->pt.max_index = util_draw_max_index(draw->pt.vertex_buffer,
+draw->pt.nr_vertex_buffers,
+draw->pt.vertex_element,
+draw->pt.nr_vertex_elements,
+info);
+
+   /*
+* TODO: We could use draw->pt.max_index to further narrow
+* the min_index/max_index hints given by the state tracker.
+*/
+
for (instance = 0; instance < info->instance_count; instance++) {
   draw->instance_id = instance + info->start_instance;
 
diff --git a/src/gallium/auxiliary/draw/draw_pt_fetch.c 
b/src/gallium/auxiliary/draw/draw_pt_fetch.c
index 4fa3b26..5589a82 100644
--- a/src/gallium/auxiliary/draw/draw_pt_fetch.c
+++ b/src/gallium/auxiliary/draw/draw_pt_fetch.c
@@ -139,7 +139,7 @@ void draw_pt_fetch_run( struct pt_fetch *fetch,
((char *)draw->pt.user.vbuffer[i] + 
 draw->pt.vertex_buffer[i].buffer_offset),
draw->pt.vertex_buffer[i].stride,
-   draw->pt.user.max_index);
+   draw->pt.max_index);
}
 
translate->run_elts( translate,
diff --git a/src/gallium/auxiliary/draw/draw_pt_fetch_emit.c 
b/src/gallium/auxiliary/draw/draw_pt_fetch_emit.c
index 5104310..0ab11d0 100644
--- a/src/gallium/auxiliary/draw/draw_pt_fetch_emit.c
+++ b/src/gallium/auxiliary/draw/draw_pt_fetch_emit.c
@@ -186,7 +186,7 @@ static void fetch_emit_prepare( struct draw_pt_middle_end 
*middle,
   ((char *)draw->pt.user.vbuffer[i] + 
draw->pt.vertex_buffer[i].buffer_offset),
   draw->pt.vertex_buffer[i].stride,
-  draw->pt.user.max_index);
+  draw->pt.max_index);
}
 
*max_vertices = (draw->render->max_vertex_buffer_bytes / 
diff --git a/src/gallium/auxiliary/draw/draw_pt_fetch_shade_emit.c 
b/src/gallium/auxiliary/draw/draw_pt_fetch_shade_emit.c
index 1e926fb..0dbbfe2 100644
--- a/src/gallium/a