[Mesa-dev] [PATCH 4/4] mesa: Check for valid PBO access in gl(Compressed)Tex(Sub)Image calls
Laura Ekstrand
laura at jlekstrand.net
Mon Mar 9 17:53:40 PDT 2015
Looks good to me.
Reviewed-by: Laura Ekstrand <laura at jlekstrand.net>
On Thu, Mar 5, 2015 at 12:20 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 | 51
> +++++++++++++++++++++++++++++++++++++++---------
> 1 file changed, 42 insertions(+), 9 deletions(-)
>
> diff --git a/src/mesa/main/teximage.c b/src/mesa/main/teximage.c
> index abcafde..f239e39 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"
>
>
> /**
> @@ -2110,7 +2111,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;
>
> @@ -2195,6 +2197,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,
> @@ -2291,7 +2300,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;
> @@ -2319,6 +2328,12 @@ 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:
> @@ -2451,7 +2466,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;
> @@ -2503,6 +2519,13 @@ texsubimage_error_check(struct gl_context *ctx,
> GLuint dimensions,
> 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, "glTexSubImage")) {
> + return GL_TRUE;
> + }
> +
> texImage = _mesa_select_tex_image(texObj, target, level);
> if (!texImage) {
> /* non-existant texture level */
> @@ -3215,12 +3238,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;
> }
>
> @@ -3570,7 +3594,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 */
> }
>
> @@ -3624,7 +3649,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 */
> }
>
> @@ -4633,7 +4659,8 @@ 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, bool dsa)
> {
> struct gl_texture_image *texImage;
> GLint expectedSize;
> @@ -4653,6 +4680,12 @@ compressed_subtexture_error_check(struct gl_context
> *ctx, GLint dims,
> return GL_TRUE;
> }
>
> + /* validate the bound PBO, if any */
> + if (!_mesa_validate_pbo_source_compressed(ctx, dims, &ctx->Unpack,
> + imageSize, data,
> "glCompressedTexImage")) {
> + return GL_TRUE;
> + }
> +
> /* Check for invalid pixel storage modes */
> if (!_mesa_compressed_pixel_storage_error_check(ctx, dims,
> &ctx->Unpack,
> @@ -4758,7 +4791,7 @@ _mesa_compressed_texture_sub_image(struct gl_context
> *ctx, GLuint dims,
> if (compressed_subtexture_error_check(ctx, dims, texObj, target,
> level, xoffset, yoffset, zoffset,
> width, height, depth,
> - format, imageSize, dsa)) {
> + format, imageSize, data, dsa)) {
> return;
> }
>
> --
> 2.1.3
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150309/259243bb/attachment.html>
More information about the mesa-dev
mailing list