[Mesa-dev] [PATCH 1/2] intel: Prevent huge lastlevel values from being computed.

[Ian Romanick]

On 09/04/2012 03:19 PM, Stéphane Marchesin wrote:
> On Tue, Sep 4, 2012 at 2:59 PM, Eric Anholt <eric at anholt.net> wrote:
>> Stéphane Marchesin <marcheu at chromium.org> writes:
>>
>>> The current computation for the lastlevel is based on the level size and can
>>> lead to writing past the end of the texture array. Instead we clamp it by
>>> MAX_TEXTURE_LEVELS.
>>> ---
>>>   src/mesa/drivers/dri/intel/intel_tex_image.c |    5 +++++
>>>   1 files changed, 5 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/src/mesa/drivers/dri/intel/intel_tex_image.c b/src/mesa/drivers/dri/intel/intel_tex_image.c
>>> index fe9040c..7ef258b 100644
>>> --- a/src/mesa/drivers/dri/intel/intel_tex_image.c
>>> +++ b/src/mesa/drivers/dri/intel/intel_tex_image.c
>>> @@ -88,6 +88,11 @@ intel_miptree_create_for_teximage(struct intel_context *intel,
>>>         lastLevel = firstLevel;
>>>         } else {
>>>         lastLevel = firstLevel + _mesa_logbase2(MAX2(MAX2(width, height), depth));
>>> +      /* We tried to guess the last level based on the texture size, make
>>> +       * sure we don't go past MAX_TEXTURE_LEVELS since it's hardcoded
>>> +       * in many places.
>>> +       */
>>> +      lastLevel = MIN2(lastLevel, MAX_TEXTURE_LEVELS - 1);
>>>         }
>>>      }
>>
>> I'm confused.  MAX_TEXTURE_LEVELS should set such that it covers
>> something from the texture size limits down to 1x1.  Does it not?
>
> Let's say the app calls glTexImage for level 6, and width = 2048. That

Is that valid?  Since the base level of this texture would be huge, it 
seems like glTexImage should reject it.  I'm not sure there's any 
language in the spec either way, but that's my gut feeling.

> goes through the other layers just fine. Then run that bit of code
> (which is kindof funny in how it assumes that if you have a 1 for a
> texture dimension, that dimension is unused, so it's also broken but
> not the point here):
>
>        /* Figure out image dimensions at start level. */
>        for (i = intelImage->base.Base.Level; i > firstLevel; i--) {
>           width <<= 1;
>           if (height != 1)
>              height <<= 1;
>           if (depth != 1)
>              depth <<= 1;
>        }
>
> You get something insane like width = 2048 * 2^6 so 2^17
>
> Then run that code:
>
>           lastLevel = firstLevel + _mesa_logbase2(MAX2(MAX2(width,
> height), depth));
>
> Width is the biggest dimension so at that point log2(width) is 17.
> That max level will get passed down the stack.
>
> Stéphane
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>