<div dir="ltr">On 14 January 2014 03:35, Tapani Pälli <span dir="ltr"><<a href="mailto:tapani.palli@intel.com" target="_blank">tapani.palli@intel.com</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
  
    
  
  <div bgcolor="#FFFFFF" text="#000000"><div class="im">
    <div>On 01/14/2014 01:24 AM, Paul Berry
      wrote:<br>
    </div>
    <blockquote type="cite">
      
      <div dir="ltr">On 2 January 2014 03:58, Tapani Pälli <span dir="ltr"><<a href="mailto:tapani.palli@intel.com" target="_blank">tapani.palli@intel.com</a>></span>
        wrote:<br>
        </div></blockquote></div><div class="im"><blockquote type="cite">
      <div dir="ltr">
        <div class="gmail_extra">
          <div class="gmail_quote">
            <blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
              +   var->state_slots = NULL;<br>
              +<br>
              +   if (var->num_state_slots > 0) {<br>
              +      var->state_slots = ralloc_array(var,
              ir_state_slot,<br>
              +         var->num_state_slots);<br>
            </blockquote>
            <div><br>
            </div>
            <div>Security issue: if the shader blob is corrupted,
              num_state_slots could be outrageously large, causing this
              memory allocation to fail (which would then cause the loop
              below to perform illegal memory accesses).  We should
              validate that num_state_slots is within a reasonable
              range, and if it isn't, abort reading the binary blob.<br>
            </div>
          </div>
        </div>
      </div>
    </blockquote>
    <br></div>
    What is the expected reasonable range for state_slots?</div></blockquote><div><br></div><div>I believe the uniform with the most state slots is gl_LightSource, which has 96.<br><br></div><div>What I'd suggest doing (in a separate patch) is: add a<br>
<br></div><div>#define MAX_NUM_STATE_SLOTS 96<br><br>somewhere, and then in builtin_variable_generator::add_uniform(), right after setting uni->num_state_slots, add:<br><br>assert(uni->num_state_slots <= MAX_NUM_STATE_SLOTS);<br>
<br>That way, in the unlikely event that we ever add a GL feature that requires more than this number of state slots, the assertion failure will alert us that we need to increase MAX_NUM_STATE_SLOTS.<br></div><div> </div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div bgcolor="#FFFFFF" text="#000000"><div class="im"><br>
    <br>
    <blockquote type="cite">
      <div dir="ltr">
        <div class="gmail_extra">
          <div class="gmail_quote">
            <blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
              +   /* fill parse state from shader header information */<br>
              +   switch (shader->Type) {<br>
              +   case GL_VERTEX_SHADER:<br>
              +      state->target = MESA_SHADER_VERTEX;<br>
              +      break;<br>
              +   case GL_FRAGMENT_SHADER:<br>
              +      state->target = MESA_SHADER_FRAGMENT;<br>
              +      break;<br>
              +   case GL_GEOMETRY_SHADER_ARB:<br>
              +      state->target = MESA_SHADER_GEOMETRY;<br>
              +      break;<br>
            </blockquote>
            <div><br>
            </div>
            <div>There should be a default case here to handle corrupted
              blobs with an illegal type.<br>
            </div>
          </div>
        </div>
      </div>
    </blockquote>
    <br></div>
    I'm now doing _mesa_shader_enum_to_shader_stage(shader->Type)
    here which will assert if type is illegal.<br></div></blockquote><div><br></div><div>An assertion isn't good enough, because assertions don't get checked in release builds.<br></div></div></div></div>