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

[Ilia Mirkin]

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.

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
>