[Mesa-dev] [PATCH 4/4 v2] mesa: Check for valid PBO access in gl(Compressed)Tex(Sub)Image calls
Laura Ekstrand
laura at jlekstrand.net
Wed Mar 11 16:43:35 PDT 2015
On Tue, Mar 10, 2015 at 11:36 AM, Eduardo Lima Mitev <elima at igalia.com>
wrote:
> This patch adds two types of checks to the gl(Compressed)Tex(Sub)Imgage
> family
> of functions when a pixel buffer object is bound to GL_PIXEL_UNPACK_BUFFER:
>
> - That the buffer is not mapped.
> - The total data size is within the boundaries of the buffer size.
>
> It does so by calling auxiliary validations functions from PBO API:
> _mesa_validate_pbo_source() for non-compressed texture calls, and
> _mesa_validate_pbo_source_compressed() for compressed texture calls.
>
> The first check is defined in Section 6.3.2 'Effects of Mapping Buffers
> on Other GL Commands' of the GLES 3.1 spec, page 57:
>
> "Any GL command which attempts to read from, write to, or change the
> state of a buffer object may generate an INVALID_OPERATION error if
> all
> or part of the buffer object is mapped. However, only commands which
> explicitly describe this error are required to do so. If an error is
> not
> generated, using such commands to perform invalid reads, writes, or
> state changes will have undefined results and may result in GL
> interruption or termination."
>
> Similar wording exists in GL 4.5 spec, page 76.
>
> In the case of gl(Compressed)Tex(Sub)Image(2,3)D, the specification
> doesn't force
> implemtations to throw an error. However since Mesa don't currently
> implement
> checks to determine when it is safe to read/write from/to a mapped PBO, we
> should always return the error if all or parts of it are mapped.
>
> The 2nd check is defined in Section 8.5 'Texture Image Specification' of
> the
> OpenGL 4.5 spec, page 203:
>
> "An INVALID_OPERATION error is generated if a pixel unpack buffer
> object
> is bound and storing texture data would access memory beyond the end
> of
> the pixel unpack buffer."
>
> Fixes 4 dEQP tests:
> *
> dEQP-GLES3.functional.negative_api.texture.compressedteximage2d_invalid_buffer_target
> *
> dEQP-GLES3.functional.negative_api.texture.compressedtexsubimage2d_invalid_buffer_target
> *
> dEQP-GLES3.functional.negative_api.texture.compressedteximage3d_invalid_buffer_target
> *
> dEQP-GLES3.functional.negative_api.texture.compressedtexsubimage3d_invalid_buffer_target
> ---
> src/mesa/main/teximage.c | 97
> +++++++++++++++++++++++++++++++++---------------
> 1 file changed, 67 insertions(+), 30 deletions(-)
>
> diff --git a/src/mesa/main/teximage.c b/src/mesa/main/teximage.c
> index 7b1a0e6..aae7c00 100644
> --- a/src/mesa/main/teximage.c
> +++ b/src/mesa/main/teximage.c
> @@ -53,6 +53,7 @@
> #include "mtypes.h"
> #include "glformats.h"
> #include "texstore.h"
> +#include "pbo.h"
>
>
> /**
> @@ -2113,7 +2114,8 @@ texture_error_check( struct gl_context *ctx,
> GLint level, GLint internalFormat,
> GLenum format, GLenum type,
> GLint width, GLint height,
> - GLint depth, GLint border )
> + GLint depth, GLint border,
> + const GLvoid *pixels )
> {
> GLenum err;
>
> @@ -2198,6 +2200,13 @@ texture_error_check( struct gl_context *ctx,
> return GL_TRUE;
> }
> + /* validate the bound PBO, if any */
> + if (!_mesa_validate_pbo_source(ctx, dimensions, &ctx->Unpack,
> + width, height, depth, format, type,
> + INT_MAX, pixels, "glTexImage")) {
> + return GL_TRUE;
> + }
> +
> /* make sure internal format and format basically agree */
> if (!texture_formats_agree(internalFormat, format)) {
> _mesa_error(ctx, GL_INVALID_OPERATION,
> @@ -2294,7 +2303,7 @@ compressed_texture_error_check(struct gl_context
> *ctx, GLint dimensions,
> GLenum target, GLint level,
> GLenum internalFormat, GLsizei width,
> GLsizei height, GLsizei depth, GLint
> border,
> - GLsizei imageSize)
> + GLsizei imageSize, const GLvoid *data)
> {
> const GLint maxLevels = _mesa_max_texture_levels(ctx, target);
> GLint expectedSize;
> @@ -2322,6 +2331,13 @@ compressed_texture_error_check(struct gl_context
> *ctx, GLint dimensions,
> return GL_TRUE;
> }
>
> + /* validate the bound PBO, if any */
> + if (!_mesa_validate_pbo_source_compressed(ctx, dimensions,
> &ctx->Unpack,
> + imageSize, data,
> + "glCompressedTexImage")) {
> + return GL_TRUE;
> + }
> +
> switch (internalFormat) {
> case GL_PALETTE4_RGB8_OES:
> case GL_PALETTE4_RGBA8_OES:
> @@ -2454,7 +2470,8 @@ texsubimage_error_check(struct gl_context *ctx,
> GLuint dimensions,
> GLenum target, GLint level,
> GLint xoffset, GLint yoffset, GLint zoffset,
> GLint width, GLint height, GLint depth,
> - GLenum format, GLenum type, bool dsa)
> + GLenum format, GLenum type, const GLvoid *pixels,
> + bool dsa)
> {
> struct gl_texture_image *texImage;
> GLenum err;
> @@ -2506,6 +2523,13 @@ texsubimage_error_check(struct gl_context *ctx,
> GLuint dimensions,
> return GL_TRUE;
> }
>
It would be more correct to plumb the calling function name through this
function (like for compressed_subtexture_error_check) since
texsubimage_error_check is called by glTex[ture]SubImage*D.
> + /* validate the bound PBO, if any */
> + if (!_mesa_validate_pbo_source(ctx, dimensions, &ctx->Unpack,
> + width, height, depth, format, type,
> + INT_MAX, pixels, "glTexSubImage")) {
> + return GL_TRUE;
> + }
> +
> texImage = _mesa_select_tex_image(texObj, target, level);
> if (!texImage) {
> /* non-existant texture level */
> @@ -3218,12 +3242,13 @@ teximage(struct gl_context *ctx, GLboolean
> compressed, GLuint dims,
> if (compressed_texture_error_check(ctx, dims, target, level,
> internalFormat,
> width, height, depth,
> - border, imageSize))
> + border, imageSize, pixels))
> return;
> }
> else {
> if (texture_error_check(ctx, dims, target, level, internalFormat,
> - format, type, width, height, depth, border))
> + format, type, width, height, depth, border,
> + pixels))
> return;
> }
>
> @@ -3573,7 +3598,8 @@ texsubimage(struct gl_context *ctx, GLuint dims,
> GLenum target, GLint level,
>
> if (texsubimage_error_check(ctx, dims, texObj, target, level,
> xoffset, yoffset, zoffset,
> - width, height, depth, format, type,
> false)) {
> + width, height, depth, format, type,
> + pixels, false)) {
> return; /* error was detected */
> }
>
> @@ -3627,7 +3653,8 @@ texturesubimage(struct gl_context *ctx, GLuint dims,
>
> if (texsubimage_error_check(ctx, dims, texObj, texObj->Target, level,
> xoffset, yoffset, zoffset,
> - width, height, depth, format, type, true))
> {
> + width, height, depth, format, type,
> + pixels, true)) {
> return; /* error was detected */
> }
>
> @@ -4623,68 +4650,72 @@ compressed_subtexture_error_check(struct
> gl_context *ctx, GLint dims,
> GLenum target, GLint level,
> GLint xoffset, GLint yoffset, GLint
> zoffset,
> GLsizei width, GLsizei height, GLsizei
> depth,
> - GLenum format, GLsizei imageSize, bool
> dsa)
> + GLenum format, GLsizei imageSize,
> + const GLvoid *data, const char
> *callerName)
> {
> struct gl_texture_image *texImage;
> GLint expectedSize;
> - const char *suffix = dsa ? "ture" : "";
>
> /* this will catch any invalid compressed format token */
> if (!_mesa_is_compressed_format(ctx, format)) {
> _mesa_error(ctx, GL_INVALID_ENUM,
> - "glCompressedTex%sSubImage%uD(format)", suffix, dims);
> + "%s(format)", callerName);
> return GL_TRUE;
> }
>
> if (level < 0 || level >= _mesa_max_texture_levels(ctx, target)) {
> _mesa_error(ctx, GL_INVALID_VALUE,
> - "glCompressedTex%sSubImage%uD(level=%d)",
> - suffix, dims, level);
> + "%s(level=%d)",
> + callerName, level);
> + return GL_TRUE;
> + }
> +
> + /* validate the bound PBO, if any */
> + if (!_mesa_validate_pbo_source_compressed(ctx, dims, &ctx->Unpack,
> + imageSize, data, callerName)) {
> return GL_TRUE;
> }
>
> /* Check for invalid pixel storage modes */
> if (!_mesa_compressed_pixel_storage_error_check(ctx, dims,
> - &ctx->Unpack,
> - dsa ? "glCompressedTextureSubImage" :
> - "glCompressedTexSubImage")) {
> + &ctx->Unpack,
> callerName)) {
> return GL_TRUE;
> }
>
> expectedSize = compressed_tex_size(width, height, depth, format);
> if (expectedSize != imageSize) {
> _mesa_error(ctx, GL_INVALID_VALUE,
> - "glCompressedTex%sSubImage%uD(size=%d)",
> - suffix, dims, imageSize);
> + "%s(size=%d)",
> + callerName, imageSize);
> return GL_TRUE;
> }
>
> texImage = _mesa_select_tex_image(texObj, target, level);
> if (!texImage) {
> _mesa_error(ctx, GL_INVALID_OPERATION,
> - "glCompressedTex%sSubImage%uD(invalid texture image)",
> - suffix, dims);
> + "%s(invalid texture image)",
> + callerName);
> return GL_TRUE;
> }
>
> if ((GLint) format != texImage->InternalFormat) {
> _mesa_error(ctx, GL_INVALID_OPERATION,
> - "glCompressedTex%sSubImage%uD(format=0x%x)",
> - suffix, dims, format);
> + "%s(format=0x%x)",
> + callerName, format);
> return GL_TRUE;
> }
>
> if (compressedteximage_only_format(ctx, format)) {
> _mesa_error(ctx, GL_INVALID_OPERATION,
> - "glCompressedTex%sSubImage%uD(format=0x%x cannot be
> updated)",
> - suffix, dims, format);
> + "%s(format=0x%x cannot be updated)",
> + callerName, format);
> return GL_TRUE;
> }
>
> if (error_check_subtexture_dimensions(ctx, dims,
> texImage, xoffset, yoffset,
> zoffset,
> width, height, depth,
> - "glCompressedTexSubImage")) {
> + callerName)) {
> return GL_TRUE;
> }
>
> @@ -4787,7 +4818,8 @@ _mesa_CompressedTexSubImage1D(GLenum target, GLint
> level, GLint xoffset,
> if (compressed_subtexture_error_check(ctx, 1, texObj, target,
> level, xoffset, 0, 0,
> width, 1, 1,
> - format, imageSize, false)) {
> + format, imageSize, data,
> + "glCompressedTexSubImage1D")) {
> return;
> }
>
> @@ -4823,7 +4855,8 @@ _mesa_CompressedTextureSubImage1D(GLuint texture,
> GLint level, GLint xoffset,
> if (compressed_subtexture_error_check(ctx, 1, texObj, texObj->Target,
> level, xoffset, 0, 0,
> width, 1, 1,
> - format, imageSize, true)) {
> + format, imageSize, data,
> +
> "glCompressedTextureSubImage1D")) {
> return;
> }
>
> @@ -4860,7 +4893,8 @@ _mesa_CompressedTexSubImage2D(GLenum target, GLint
> level, GLint xoffset,
> if (compressed_subtexture_error_check(ctx, 2, texObj, target,
> level, xoffset, yoffset, 0,
> width, height, 1,
> - format, imageSize, false)) {
> + format, imageSize, data,
> + "glCompressedTexSubImage2D")) {
> return;
> }
>
> @@ -4899,7 +4933,8 @@ _mesa_CompressedTextureSubImage2D(GLuint texture,
> GLint level, GLint xoffset,
> if (compressed_subtexture_error_check(ctx, 2, texObj, texObj->Target,
> level, xoffset, yoffset, 0,
> width, height, 1,
> - format, imageSize, true)) {
> + format, imageSize, data,
> +
> "glCompressedTextureSubImage2D")) {
> return;
> }
>
> @@ -4935,7 +4970,8 @@ _mesa_CompressedTexSubImage3D(GLenum target, GLint
> level, GLint xoffset,
> if (compressed_subtexture_error_check(ctx, 3, texObj, target,
> level, xoffset, yoffset, zoffset,
> width, height, depth,
> - format, imageSize, false)) {
> + format, imageSize, data,
> + "glCompressedTexSubImage3D")) {
> return;
> }
>
> @@ -4975,7 +5011,8 @@ _mesa_CompressedTextureSubImage3D(GLuint texture,
> GLint level, GLint xoffset,
> if (compressed_subtexture_error_check(ctx, 3, texObj, texObj->Target,
> level, xoffset, yoffset, zoffset,
> width, height, depth,
> - format, imageSize, true)) {
> + format, imageSize, data,
> +
> "glCompressedTextureSubImage3D")) {
> return;
> }
>
> --
> 2.1.3
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150311/a8186062/attachment-0001.html>
More information about the mesa-dev
mailing list