[Mesa-dev] [PATCH v3] texgetimage: Add check for the effective target to GetTextureSubImage

[Eduardo Lima Mitev]

On 02/02/2017 10:05 PM, Ilia Mirkin wrote:
> Reviewed-by: Ilia Mirkin <imirkin at alum.mit.edu>
> 
> Although come to think of it, perhaps this *would* be better as
> 
> if (!legal_getteximage_target(ctx, texObj->Target, true)) {
>    _mesa_error(ctx, GL_INVALID_OPERATION,
>                "%s(invalid texture)", caller);
>    return;
> }
> 
> or something. [No need for the INVALID_ENUM case though since the enum
> can't be *invalid* if it's in texobj->Target.]
> 
> Sorry to be flip-flopping on this, but I keep confusing myself too :)
> I'd be happy with either patch, so feel free to slap my R-b on
> whichever you find clearer.
> 

Ok, I finally pushed it based on your suggestion above. It seems clearer
and more robust.

Thanks a lot!

Eduardo

> Cheers,
> 
>   -ilia
> 
> On Thu, Feb 2, 2017 at 3:57 PM, Eduardo Lima Mitev <elima at igalia.com> wrote:
>> OpenGL 4.5 spec, section "8.11.4 Texture Image Queries", page 233 of
>> the PDF states:
>>
>>     "An INVALID_OPERATION error is generated if texture is the name of a buffer
>>      or multisample texture."
>>
>> This is currently not being checked and e.g a multisample texture image can
>> be passed down to the driver hook, crashing the driver on i965.
>>
>> v2: (Ilia Mirkin) Move the check from gettextimage_error_check() to
>>     GetTextureSubImage() and use the texObj target.
>>
>> v3: (Ilia Mirkin) Calling legal_getteximage_target() is not necessary, and
>>     add GL_TEXTURE_2D_MULTISAMPLE_ARRAY too.
>> ---
>>  src/mesa/main/texgetimage.c | 8 ++++++++
>>  1 file changed, 8 insertions(+)
>>
>> diff --git a/src/mesa/main/texgetimage.c b/src/mesa/main/texgetimage.c
>> index d5cb1636605..7202c7f99ab 100644
>> --- a/src/mesa/main/texgetimage.c
>> +++ b/src/mesa/main/texgetimage.c
>> @@ -1465,6 +1465,14 @@ _mesa_GetTextureSubImage(GLuint texture, GLint level,
>>        return;
>>     }
>>
>> +   if (texObj->Target == GL_TEXTURE_BUFFER ||
>> +       texObj->Target == GL_TEXTURE_2D_MULTISAMPLE ||
>> +       texObj->Target == GL_TEXTURE_2D_MULTISAMPLE_ARRAY) {
>> +      _mesa_error(ctx, GL_INVALID_OPERATION,
>> +                  "%s(buffer/multisample texture)", 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
>>
>