swr: Fixed an uncommon freed-memory access during state validation
authorBruce Cherniak <bruce.cherniak@intel.com>
Thu, 9 Nov 2017 00:39:37 +0000 (18:39 -0600)
committerGeorge Kyriazis <george.kyriazis@intel.com>
Fri, 10 Nov 2017 14:55:42 +0000 (08:55 -0600)
State validation is performed during clear and draw calls.  Validation
during clear was still accessing vertex buffer state.  When the currently
set vertex buffers are client arrays, this could lead to accessing freed
memory.  Such is the case with the VMD application.

Previously, vertex buffer validation depended on a dirty bit or the
draw info indicating an indexed draw.  This required special handling for
clears.  But, vertex buffer validation still occurred which was unnecessary
and wrong.

Now, only minimal validation is performed during clear, deferring the
remainder to the next draw.  And, by setting the dirty bit in swr_draw_vbo
for indexed draws, vertex buffer validation is only dependent upon a
single dirty bit.

This fixes a bug exposed by the VMD application when changing models.

Reviewed-By: George Kyriazis <george.kyriazis@intel.com>
src/gallium/drivers/swr/swr_draw.cpp
src/gallium/drivers/swr/swr_state.cpp

index 57660c7464d937187a0184048d6e308ebd619ba2..a94cdd6da0b89b66ee0aa64eecea22627c423255 100644 (file)
@@ -52,7 +52,12 @@ swr_draw_vbo(struct pipe_context *pipe, const struct pipe_draw_info *info)
       return;
    }
 
-   /* Update derived state, pass draw info to update function */
+   /* If indexed draw, force vertex validation since index buffer comes
+    * from draw info. */
+   if (info->index_size)
+      ctx->dirty |= SWR_NEW_VERTEX;
+
+   /* Update derived state, pass draw info to update function. */
    swr_update_derived(pipe, info);
 
    swr_update_draw_context(ctx);
index c6da4fcb8ed253858e1d0b80867d4a9155d748bb..4530d377ee904b5bf22197a3f2ccdef5a9777639 100644 (file)
@@ -1204,11 +1204,6 @@ swr_update_derived(struct pipe_context *pipe,
       ctx->api.pfnSwrSetRastState(ctx->swrContext, rastState);
    }
 
-   /* Scissor */
-   if (ctx->dirty & SWR_NEW_SCISSOR) {
-      ctx->api.pfnSwrSetScissorRects(ctx->swrContext, 1, &ctx->swr_scissor);
-   }
-
    /* Viewport */
    if (ctx->dirty & (SWR_NEW_VIEWPORT | SWR_NEW_FRAMEBUFFER
                      | SWR_NEW_RASTERIZER)) {
@@ -1249,18 +1244,26 @@ swr_update_derived(struct pipe_context *pipe,
       ctx->api.pfnSwrSetViewports(ctx->swrContext, 1, vp, vpm);
    }
 
-   /* Set vertex & index buffers
-    * (using draw info if called by swr_draw_vbo)
-    * If indexed draw, revalidate since index buffer comes from
-    * pipe_draw_info.
-    */
-   if (ctx->dirty & SWR_NEW_VERTEX ||
-      (p_draw_info && p_draw_info->index_size)) {
+   /* When called from swr_clear (p_draw_info = null), render targets,
+    * rasterState and viewports (dependent on render targets) are the only
+    * necessary validation.  Defer remaining validation by setting
+    * post_update_dirty_flags and clear all dirty flags.  BackendState is
+    * still unconditionally validated below */
+   if (!p_draw_info) {
+      post_update_dirty_flags = ctx->dirty & ~(SWR_NEW_FRAMEBUFFER |
+                                               SWR_NEW_RASTERIZER |
+                                               SWR_NEW_VIEWPORT);
+      ctx->dirty = 0;
+   }
+
+   /* Scissor */
+   if (ctx->dirty & SWR_NEW_SCISSOR) {
+      ctx->api.pfnSwrSetScissorRects(ctx->swrContext, 1, &ctx->swr_scissor);
+   }
 
-      /* If being called by swr_draw_vbo, copy draw details */
-      struct pipe_draw_info info = {0};
-      if (p_draw_info)
-         info = *p_draw_info;
+   /* Set vertex & index buffers */
+   if (ctx->dirty & SWR_NEW_VERTEX) {
+      const struct pipe_draw_info &info = *p_draw_info;
 
       /* vertex buffers */
       SWR_VERTEX_BUFFER_STATE swrVertexBuffers[PIPE_MAX_ATTRIBS];