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

[David Riley]

On Wed, Jul 18, 2018 at 1:10 PM Po-Hsien Wang <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.


>
> On Wed, Jul 18, 2018 at 1:04 AM Erik Faye-Lund <
> 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>
>> ---
>> 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
> 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/2ab8a8e7/attachment.html>