On 04/22/2012 09:44 AM, nobled wrote:
Though not explicit in the GL_ARB_robustness extension,
the GL standard says:

   If a negative number is provided where an argument of type sizei or
   sizeiptr is specified, the error INVALID_VALUE is generated.

While the extension says that GL_INVALID_OPERATION is given
when the required space is "greater than<bufSize>", and this

Which error do other implementations generate? If there's a clear trend, we should follow that. If there's not a clear trend, I can file a bug against the spec for being ambiguous.

would be technically true for all negative values, defer to the
above language. This error check seems to apply even when a PBO
is bound, while insufficient but non-negative values of bufSize
are ignored.

This also fixes a bug in _mesa_validate_pbo_access() where the
signed value clientMemSize was blindly cast to an unsigned value.
When bufSize/clientMemSize were negative, it would be cast to
a value greater than INT_MAX and no error would have been given,
not even the incorrect GL_INVALID_OPERATION. Oops.

NOTE: This is a candidate for the 8.0 branch.
---
  src/mesa/main/eval.c            |   18 ++++++++++++++++++
  src/mesa/main/pbo.c             |   15 +++++++++++++++
  src/mesa/main/pixel.c           |    6 ++++++
  src/mesa/main/readpix.c         |    6 ++++++
  src/mesa/main/texgetimage.c     |   16 +++++++++++++++-
  src/mesa/main/uniform_query.cpp |    9 ++++++++-
  6 files changed, 68 insertions(+), 2 deletions(-)

diff --git a/src/mesa/main/eval.c b/src/mesa/main/eval.c
index e651715..8c25295 100644
--- a/src/mesa/main/eval.c
+++ b/src/mesa/main/eval.c
@@ -563,6 +563,12 @@ _mesa_GetnMapdvARB( GLenum target, GLenum query,
GLsizei bufSize, GLdouble *v )
        return;
     }

+   if (bufSize<  0) {
+      _mesa_error( ctx, GL_INVALID_VALUE,
+                  "glGetnMapdvARB(bufSize = %d)", bufSize );
+      return;
+   }
+
     map1d = get_1d_map(ctx, target);
     map2d = get_2d_map(ctx, target);
     ASSERT(map1d || map2d);
@@ -655,6 +661,12 @@ _mesa_GetnMapfvARB( GLenum target, GLenum query,
GLsizei bufSize, GLfloat *v )
        return;
     }

+   if (bufSize<  0) {
+      _mesa_error( ctx, GL_INVALID_VALUE,
+                  "glGetnMapfvARB(bufSize = %d)", bufSize );
+      return;
+   }
+
     map1d = get_1d_map(ctx, target);
     map2d = get_2d_map(ctx, target);
     ASSERT(map1d || map2d);
@@ -749,6 +761,12 @@ _mesa_GetnMapivARB( GLenum target, GLenum query,
GLsizei bufSize, GLint *v )
        return;
     }

+   if (bufSize<  0) {
+      _mesa_error( ctx, GL_INVALID_VALUE,
+                  "glGetnMapivARB(bufSize = %d)", bufSize );
+      return;
+   }
+
     map1d = get_1d_map(ctx, target);
     map2d = get_2d_map(ctx, target);
     ASSERT(map1d || map2d);
diff --git a/src/mesa/main/pbo.c b/src/mesa/main/pbo.c
index d8fa919..0fe5394 100644
--- a/src/mesa/main/pbo.c
+++ b/src/mesa/main/pbo.c
@@ -71,6 +71,9 @@ _mesa_validate_pbo_access(GLuint dimensions,
     /* unsigned, to detect overflow/wrap-around */
     uintptr_t start, end, offset, size;

+   /* This should be error-checked by all callers beforehand. */
+   assert(clientMemSize>= 0);
+
     /* If no PBO is bound, 'ptr' is a pointer to client memory containing
        'clientMemSize' bytes.
        If a PBO is bound, 'ptr' is an offset into the bound PBO.
@@ -180,6 +183,12 @@ _mesa_map_validate_pbo_source(struct gl_context *ctx,
  {
     ASSERT(dimensions == 1 || dimensions == 2 || dimensions == 3);

+   if (clientMemSize<  0) {
+      _mesa_error( ctx, GL_INVALID_VALUE,
+                  "%s(bufSize = %d)", where, clientMemSize );
+      return NULL;
+   }
+
     if (!_mesa_validate_pbo_access(dimensions, unpack, width, height, depth,
                                    format, type, clientMemSize, ptr)) {
        if (_mesa_is_bufferobj(unpack->BufferObj)) {
@@ -276,6 +285,12 @@ _mesa_map_validate_pbo_dest(struct gl_context *ctx,
  {
     ASSERT(dimensions == 1 || dimensions == 2 || dimensions == 3);

+   if (clientMemSize<  0) {
+      _mesa_error( ctx, GL_INVALID_VALUE,
+                  "%s(bufSize = %d)", where, clientMemSize );
+      return NULL;
+   }
+
     if (!_mesa_validate_pbo_access(dimensions, unpack, width, height, depth,
                                    format, type, clientMemSize, ptr)) {
        if (_mesa_is_bufferobj(unpack->BufferObj)) {
diff --git a/src/mesa/main/pixel.c b/src/mesa/main/pixel.c
index 450c936..be6a19d 100644
--- a/src/mesa/main/pixel.c
+++ b/src/mesa/main/pixel.c
@@ -153,6 +153,12 @@ validate_pbo_access(struct gl_context *ctx,
  {
     GLboolean ok;

+   if (clientMemSize<  0) {
+      _mesa_error(ctx, GL_INVALID_VALUE,
+                  "glGetnPixelMap*vARB(bufSize = %d)", clientMemSize);
+      return GL_FALSE;
+   }
+
     /* Note, need to use DefaultPacking and Unpack's buffer object */
     _mesa_reference_buffer_object(ctx,
                                   &ctx->DefaultPacking.BufferObj,
diff --git a/src/mesa/main/readpix.c b/src/mesa/main/readpix.c
index 31acfcb..d4fcc56 100644
--- a/src/mesa/main/readpix.c
+++ b/src/mesa/main/readpix.c
@@ -690,6 +690,12 @@ _mesa_ReadnPixelsARB( GLint x, GLint y, GLsizei
width, GLsizei height,
        return;
     }

+   if (bufSize<  0) {
+      _mesa_error( ctx, GL_INVALID_VALUE,
+                  "glReadnPixelsARB(bufSize = %d)", bufSize );
+      return;
+   }
+
     if (ctx->NewState)
        _mesa_update_state(ctx);

diff --git a/src/mesa/main/texgetimage.c b/src/mesa/main/texgetimage.c
index 05b052a..ff03d6d 100644
--- a/src/mesa/main/texgetimage.c
+++ b/src/mesa/main/texgetimage.c
@@ -659,6 +659,12 @@ getteximage_error_check(struct gl_context *ctx,
GLenum target, GLint level,
        return GL_TRUE;
     }

+   if (clientMemSize<  0) {
+      _mesa_error( ctx, GL_INVALID_VALUE,
+                  "glGetnTexImageARB(bufSize = %d)", clientMemSize );
+      return GL_TRUE;
+   }
+
     err = _mesa_error_check_format_and_type(ctx, format, type);
     if (err != GL_NO_ERROR) {
        _mesa_error(ctx, err, "glGetTexImage(format/type)");
@@ -717,6 +723,7 @@ getteximage_error_check(struct gl_context *ctx,
GLenum target, GLint level,
           _mesa_error(ctx, GL_INVALID_OPERATION,
                       "glGetTexImage(out of bounds PBO access)");
        } else {
+         assert(bufSize>= 0);
           _mesa_error(ctx, GL_INVALID_OPERATION,
                       "glGetnTexImageARB(out of bounds access:"
                       " bufSize (%d) is too small)", clientMemSize);
@@ -822,6 +829,12 @@ getcompressedteximage_error_check(struct
gl_context *ctx, GLenum target,
                    "glGetCompressedTexImageARB(bad level = %d)", level);
        return GL_TRUE;
     }
+   if (clientMemSize<  0) {
+      _mesa_error(ctx, GL_INVALID_VALUE,
+                  "glGetnCompressedTexImageARB"
+                  "(bufSize = %d)", clientMemSize);
+      return GL_TRUE;
+   }

     if (_mesa_is_proxy_texture(target)) {
        _mesa_error(ctx, GL_INVALID_ENUM,
@@ -857,8 +870,9 @@ getcompressedteximage_error_check(struct
gl_context *ctx, GLenum target,
                                              texImage->Depth);

     if (!_mesa_is_bufferobj(ctx->Pack.BufferObj)) {
+      assert(clientMemSize>= 0);
        /* do bounds checking on writing to client memory */
-      if (clientMemSize<  compressedSize) {
+      if ((GLuint)clientMemSize<  compressedSize) {
           _mesa_error(ctx, GL_INVALID_OPERATION,
                       "glGetnCompressedTexImageARB(out of bounds access:"
                       " bufSize (%d) is too small)", clientMemSize);
diff --git a/src/mesa/main/uniform_query.cpp b/src/mesa/main/uniform_query.cpp
index da41ee8..96fe70b 100644
--- a/src/mesa/main/uniform_query.cpp
+++ b/src/mesa/main/uniform_query.cpp
@@ -187,6 +187,12 @@ _mesa_get_uniform(struct gl_context *ctx, GLuint
program, GLint location,
     struct gl_uniform_storage *uni;
     unsigned loc, offset;

+   if (bufSize<  0) {
+      _mesa_error( ctx, GL_INVALID_VALUE,
+                  "glGetnUniform*vARB(bufSize = %d)", bufSize );
+      return;
+   }
+
     if (!validate_uniform_parameters(ctx, shProg, location, 1,
                                &loc,&offset, "glGetUniform", true))
        return;
@@ -210,7 +216,8 @@ _mesa_get_uniform(struct gl_context *ctx, GLuint
program, GLint location,
         * with glGetUniformdv()...
         */
        unsigned bytes = sizeof(src[0]) * elements;
-      if (bufSize<  0 || bytes>  (unsigned) bufSize) {
+      assert(bufSize>= 0);
+      if (bytes>  (unsigned) bufSize) {
         _mesa_error( ctx, GL_INVALID_OPERATION,
                     "glGetnUniform*vARB(out of bounds: bufSize is %d,"
                     " but %u bytes are required)", bufSize, bytes );

_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to