[virglrenderer-devel] [PATCH 2/2] protect gl{BeginEnd}ConditionalRendering calls

[Erik Faye-Lund]

On 18. juli 2018 22:12, David Riley wrote:
> On Wed, Jul 18, 2018 at 1:10 PM Po-Hsien Wang <pwang at chromium.org 
> <mailto:pwang at chromium.org>> wrote:
>
>     Shouldn't have_gl_conditional_render deps on !gles?
>
>
> To expand on this, the change as posted still fails for me with the 
> same fuzzing input, but adding a !gles check to the 
> have_gl_conditional_render assignment fixed the failure in question.

Oh, are you saying gl_ver is the OpenGL ES version in the GLES case? If 
so, you are of course right, and I think we have a lot more code-paths 
to update...

>
>     On Wed, Jul 18, 2018 at 1:04 AM Erik Faye-Lund
>     <erik.faye-lund at collabora.com
>     <mailto:erik.faye-lund at collabora.com>> wrote:
>
>         Otherwise, it'd be possible to generate evil commands from a rouge
>         guest-driver that can crash the VM.
>
>         This is a bit trickier than the previous one, because we were
>         already
>         mixing calls to the OpenGL 3.0 version and the
>         GL_NV_conditional_render
>         version, which indicates that this was previously only safe if
>         *both*
>         were supported, that is OpenGL 3.0 *with*
>         GL_NV_conditional_render. Now
>         this code should match the caps we generate, which shouldn't
>         give any
>         percieved feature-regressions or gl-versions supported.
>
>         Signed-off-by: Erik Faye-Lund <erik.faye-lund at collabora.com
>         <mailto:erik.faye-lund at collabora.com>>
>         ---
>         So this is a little bit tric
>
>          src/vrend_renderer.c | 30 ++++++++++++++++++++++++------
>          1 file changed, 24 insertions(+), 6 deletions(-)
>
>         diff --git a/src/vrend_renderer.c b/src/vrend_renderer.c
>         index 564807a..f34a58f 100644
>         --- a/src/vrend_renderer.c
>         +++ b/src/vrend_renderer.c
>         @@ -109,6 +109,8 @@ struct global_renderer_state {
>             bool have_ms_scaled_blit;
>             bool have_nv_prim_restart;
>             bool have_gl_prim_restart;
>         +   bool have_gl_conditional_render;
>         +   bool have_nv_conditional_render;
>             bool have_bit_encoding;
>             bool have_gles31_vertex_attrib_binding;
>             bool have_tf2;
>         @@ -4523,6 +4525,10 @@ int vrend_renderer_init(struct
>         vrend_if_cbs *cbs, uint32_t flags)
>                vrend_state.have_gl_prim_restart = true;
>             else if (epoxy_has_gl_extension("GL_NV_primitive_restart"))
>                vrend_state.have_nv_prim_restart = true;
>         +   if (gl_ver >= 30)
>         +      vrend_state.have_gl_conditional_render = true;
>         +   else if (epoxy_has_gl_extension("GL_NV_conditional_render"))
>         +      vrend_state.have_nv_conditional_render = true;
>             if (gl_ver >= 40 || (gles && gl_ver >= 30) ||
>         epoxy_has_gl_extension("GL_ARB_transform_feedback2")) {
>                vrend_state.have_tf2 = true;
>         @@ -7008,11 +7014,18 @@ static void
>         vrend_pause_render_condition(struct vrend_context *ctx, bool
>         pause)
>          {
>             if (pause) {
>                if (ctx->sub->cond_render_q_id)
>         -         glEndConditionalRenderNV();
>         +         if (vrend_state.have_gl_conditional_render)
>         +            glEndConditionalRender();
>         +         else if (vrend_state.have_nv_conditional_render)
>         +            glEndConditionalRenderNV();
>             } else {
>                if (ctx->sub->cond_render_q_id)
>         -  glBeginConditionalRender(ctx->sub->cond_render_q_id,
>         - ctx->sub->cond_render_gl_mode);
>         +         if (vrend_state.have_gl_conditional_render)
>         + glBeginConditionalRender(ctx->sub->cond_render_q_id,
>         +  ctx->sub->cond_render_gl_mode);
>         +         else if (vrend_state.have_nv_conditional_render)
>         + glBeginConditionalRenderNV(ctx->sub->cond_render_q_id,
>         +  ctx->sub->cond_render_gl_mode);
>             }
>          }
>
>         @@ -7025,7 +7038,10 @@ void vrend_render_condition(struct
>         vrend_context *ctx,
>             GLenum glmode = 0;
>
>             if (handle == 0) {
>         -      glEndConditionalRenderNV();
>         +      if (vrend_state.have_gl_conditional_render)
>         +         glEndConditionalRender();
>         +      else if (vrend_state.have_nv_conditional_render)
>         +         glEndConditionalRenderNV();
>                ctx->sub->cond_render_q_id = 0;
>                ctx->sub->cond_render_gl_mode = 0;
>                return;
>         @@ -7054,8 +7070,10 @@ void vrend_render_condition(struct
>         vrend_context *ctx,
>
>             ctx->sub->cond_render_q_id = q->id;
>             ctx->sub->cond_render_gl_mode = glmode;
>         -   glBeginConditionalRender(q->id, glmode);
>         -
>         +   if (vrend_state.have_gl_conditional_render)
>         +      glBeginConditionalRender(q->id, glmode);
>         +   if (vrend_state.have_nv_conditional_render)
>         +      glBeginConditionalRenderNV(q->id, glmode);
>          }
>
>          int vrend_create_so_target(struct vrend_context *ctx,
>         -- 
>         2.18.0.rc2
>
>     _______________________________________________
>     virglrenderer-devel mailing list
>     virglrenderer-devel at lists.freedesktop.org
>     <mailto:virglrenderer-devel at lists.freedesktop.org>
>     https://lists.freedesktop.org/mailman/listinfo/virglrenderer-devel
>
>
>
> _______________________________________________
> virglrenderer-devel mailing list
> virglrenderer-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/virglrenderer-devel

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/virglrenderer-devel/attachments/20180718/ab04f585/attachment.html>