[Mesa-dev] [PATCH 3/3] mesa: generate GL_INVALID_VALUE when bufSize < 0

[nobled]

On Tue, Apr 24, 2012 at 9:29 AM, Ian Romanick <idr at freedesktop.org> wrote:
> 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.
I'm still waiting for someone who can use the proprietary drivers to
test the piglit patch I sent out a week ago. Till then, how about I
just commit a minimal fix for the "accidentally no error" bug:

[PATCH] mesa: fix potential buffer overflow

This 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.

There's some ambiguity with the GL_ARB_robustness spec whether
the error should be GL_INVALID_VALUE or GL_INVALID_OPERATION.
While waiting for someone to run the piglit test on the proprietary
drivers, just give GL_INVALID_OPERATION for now.
---
 src/mesa/main/pbo.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/src/mesa/main/pbo.c b/src/mesa/main/pbo.c
index d8fa919..c6b7ede 100644
--- a/src/mesa/main/pbo.c
+++ b/src/mesa/main/pbo.c
@@ -77,6 +77,8 @@ _mesa_validate_pbo_access(GLuint dimensions,
       In that case 'clientMemSize' is ignored: we just use the PBO's size.
     */
    if (!_mesa_is_bufferobj(pack->BufferObj)) {
+      if (clientMemSize < 0)
+         return GL_FALSE;
       offset = 0;
       size = clientMemSize;
    } else {
-- 
1.7.4.1