[Mesa-dev] [PATCH] vbo: fix possible use-after-free segfault after a VAO is deleted

[Marek Olšák]

This like the fifth attempt to fix the issue.

Also with the new "validating" flag, we can set recalculate_inputs to FALSE
earlier in vbo_bind_arrays, because _mesa_update_state won't change it.

NOTE: This is a candidate for the stable branches.
---
 src/mesa/vbo/vbo_exec.c       |   20 ++++++++++++++++++--
 src/mesa/vbo/vbo_exec.h       |    1 +
 src/mesa/vbo/vbo_exec_array.c |    8 ++++++--
 3 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/src/mesa/vbo/vbo_exec.c b/src/mesa/vbo/vbo_exec.c
index 5827f90..fd3a052 100644
--- a/src/mesa/vbo/vbo_exec.c
+++ b/src/mesa/vbo/vbo_exec.c
@@ -79,10 +79,26 @@ void vbo_exec_destroy( struct gl_context *ctx )
  */ 
 void vbo_exec_invalidate_state( struct gl_context *ctx, GLuint new_state )
 {
-   struct vbo_exec_context *exec = &vbo_context(ctx)->exec;
+   struct vbo_context *vbo = vbo_context(ctx);
+   struct vbo_exec_context *exec = &vbo->exec;
 
-   if (new_state & (_NEW_PROGRAM|_NEW_ARRAY)) {
+   if (!exec->validating && new_state & (_NEW_PROGRAM|_NEW_ARRAY)) {
       exec->array.recalculate_inputs = GL_TRUE;
+
+      /* If we ended up here because a VAO was deleted, the _DrawArrays
+       * pointer which pointed to the VAO might be invalid now, so set it
+       * to NULL.  This prevents crashes in driver functions like Clear
+       * where driver state validation might occur, but the vbo module is
+       * still in an invalid state.
+       *
+       * Drivers should skip vertex array state validation if _DrawArrays
+       * is NULL.  It also has no affect on performance, because attrib
+       * bindings will be recalculated anyway.
+       */
+      if (vbo->last_draw_method == DRAW_ARRAYS) {
+         ctx->Array._DrawArrays = NULL;
+         vbo->last_draw_method = DRAW_NONE;
+      }
    }
 
    if (new_state & _NEW_EVAL)
diff --git a/src/mesa/vbo/vbo_exec.h b/src/mesa/vbo/vbo_exec.h
index 9fc8791..bd3ab3b 100644
--- a/src/mesa/vbo/vbo_exec.h
+++ b/src/mesa/vbo/vbo_exec.h
@@ -81,6 +81,7 @@ struct vbo_exec_context
    struct gl_context *ctx;   
    GLvertexformat vtxfmt;
    GLvertexformat vtxfmt_noop;
+   GLboolean validating; /**< if we're in the middle of state validation */
 
    struct {
       struct gl_buffer_object *bufferobj;
diff --git a/src/mesa/vbo/vbo_exec_array.c b/src/mesa/vbo/vbo_exec_array.c
index 7e61f7b..1bf3af4 100644
--- a/src/mesa/vbo/vbo_exec_array.c
+++ b/src/mesa/vbo/vbo_exec_array.c
@@ -501,6 +501,7 @@ vbo_bind_arrays(struct gl_context *ctx)
 
    if (exec->array.recalculate_inputs) {
       recalculate_input_bindings(ctx);
+      exec->array.recalculate_inputs = GL_FALSE;
 
       /* Again... because we may have changed the bitmask of per-vertex varying
        * attributes.  If we regenerate the fixed-function vertex program now
@@ -508,10 +509,13 @@ vbo_bind_arrays(struct gl_context *ctx)
        * need in the shader.
        */
       if (ctx->NewState) {
+         /* Setting "validating" to TRUE prevents _mesa_update_state from
+          * invalidating what we just did.
+          */
+         exec->validating = GL_TRUE;
          _mesa_update_state(ctx);
+         exec->validating = GL_FALSE;
       }
-
-      exec->array.recalculate_inputs = GL_FALSE;
    }
 }
 
-- 
1.7.10.4