[Mesa-dev] [PATCH v2] texgetimage: Add check for the target argument to GetTextureSubImage

[Eduardo Lima Mitev]

On 02/02/2017 09:24 PM, Ilia Mirkin wrote:
> On Thu, Feb 2, 2017 at 2:50 PM, Eduardo Lima Mitev <elima at igalia.com> wrote:
>> This is currently not being checked and e.g a multisample texture image is passed
>> down to the driver hook. On i965, it is crashing the driver with an assertion:
>>
>> intel_mipmap_tree.c:3125: intel_miptree_map: Assertion `mt->num_samples <= 1' failed.
>>
>> Similar check exists for GetTextureSubImage.
>>
>> v2: (Ilia Mirkin) Use legal_getteximage_target() instead.
>> ---
>>  src/mesa/main/texgetimage.c | 11 +++++++++++
>>  1 file changed, 11 insertions(+)
>>
>> diff --git a/src/mesa/main/texgetimage.c b/src/mesa/main/texgetimage.c
>> index d5cb1636605..c9a0ef0b8ac 100644
>> --- a/src/mesa/main/texgetimage.c
>> +++ b/src/mesa/main/texgetimage.c
>> @@ -1465,6 +1465,17 @@ _mesa_GetTextureSubImage(GLuint texture, GLint level,
>>        return;
>>     }
>>
>> +   if (!legal_getteximage_target(ctx, texObj->Target, true)) {
> 
> Sorry, perhaps I wasn't clear enough. Actually I don't think this
> needs to be called here at all - it doesn't really add anything, since
> a target isn't passed in here, and the extension checking isn't really
> important since the texture object can't just appear out of nowhere
> with an illegal target. [And the same argument applies to
> GetTextureImage as well...] Just the if statement below without the
> else clause. I believe the rest is covered by getteximage_error_check.
> 

Sorry, Ilia. It seems my head is in Fosdem already :). I understand what
you meant now. Calling legal_gettextimage_target() there is silly.
I will send a new patch ASAP.

Thanks again for the patience.

Eduardo

>> +      if (texObj->Target == GL_TEXTURE_BUFFER ||
>> +          texObj->Target == GL_TEXTURE_2D_MULTISAMPLE) {
> 
> GL_TEXTURE_2D_MULTISAMPLE_ARRAY as well presumably?
> 
>> +         _mesa_error(ctx, GL_INVALID_OPERATION,
>> +                     "%s(buffer/multisample texture)", caller);
>> +      } else {
>> +         _mesa_error(ctx, GL_INVALID_ENUM, "%s(target)", caller);
>> +      }
>> +      return;
>> +   }
>> +
>>     if (getteximage_error_check(ctx, texObj, texObj->Target, level,
>>                                 xoffset, yoffset, zoffset, width, height, depth,
>>                                 format, type, bufSize, pixels, caller)) {
>> --
>> 2.11.0
>>
>