[Mesa-dev] [wip 6/9] glsl: ir_deserializer class for the binary shader cache

[Paul Berry]

On 14 January 2014 03:35, Tapani Pälli <tapani.palli at intel.com> wrote:

>  On 01/14/2014 01:24 AM, Paul Berry wrote:
>
> On 2 January 2014 03:58, Tapani Pälli <tapani.palli at intel.com> wrote:
>
>   +   var->state_slots = NULL;
>> +
>> +   if (var->num_state_slots > 0) {
>> +      var->state_slots = ralloc_array(var, ir_state_slot,
>> +         var->num_state_slots);
>>
>
>  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.
>
>
> What is the expected reasonable range for state_slots?
>

I believe the uniform with the most state slots is gl_LightSource, which
has 96.

What I'd suggest doing (in a separate patch) is: add a

#define MAX_NUM_STATE_SLOTS 96

somewhere, and then in builtin_variable_generator::add_uniform(), right
after setting uni->num_state_slots, add:

assert(uni->num_state_slots <= MAX_NUM_STATE_SLOTS);

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.


>
>
>    +   /* fill parse state from shader header information */
>> +   switch (shader->Type) {
>> +   case GL_VERTEX_SHADER:
>> +      state->target = MESA_SHADER_VERTEX;
>> +      break;
>> +   case GL_FRAGMENT_SHADER:
>> +      state->target = MESA_SHADER_FRAGMENT;
>> +      break;
>> +   case GL_GEOMETRY_SHADER_ARB:
>> +      state->target = MESA_SHADER_GEOMETRY;
>> +      break;
>>
>
>  There should be a default case here to handle corrupted blobs with an
> illegal type.
>
>
> I'm now doing _mesa_shader_enum_to_shader_stage(shader->Type) here which
> will assert if type is illegal.
>

An assertion isn't good enough, because assertions don't get checked in
release builds.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20140117/441719f8/attachment.html>