integer overflow check

[Alan Coopersmith]

On 12/15/14 02:49 PM, Julien Cristau wrote:
> On Mon, Dec 15, 2014 at 17:56:56 +0100, jes at posteo.de wrote:
>
>> diff --git a/hw/xfree86/dri2/dri2ext.c b/hw/xfree86/dri2/dri2ext.c
>> index 221ec53..63191dc 100644
>> --- a/hw/xfree86/dri2/dri2ext.c
>> +++ b/hw/xfree86/dri2/dri2ext.c
>> @@ -269,10 +269,11 @@ ProcDRI2GetBuffers(ClientPtr client)
>>       int status, width, height, count;
>>       unsigned int *attachments;
>>
>> -    REQUEST_FIXED_SIZE(xDRI2GetBuffersReq, stuff->count * 4);
>>       if (stuff->count > (INT_MAX / 4))
>>           return BadLength;
>>
>> +    REQUEST_FIXED_SIZE(xDRI2GetBuffersReq, stuff->count * 4);
>> +
>>       if (!validDrawable(client, stuff->drawable, DixReadAccess |
>> DixWriteAccess,
>>                          &pDrawable, &status))
>>           return status;
>
> As far as I can tell you're now reading stuff->count before checking
> whether it's inside the request buffer?

Oh, right.  I was thinking of "length" (which is always guaranteed to be
present) when I looked at the code to check "count" there.

That's why the checks were in that order.

If you wanted to be extremely pedantic, you'd need to do:

	REQUEST_AT_LEAST_SIZE(xDRI2GetBuffersReq);

	if (stuff->count > (INT_MAX / 4))
		return BadLength;

	REQUEST_FIXED_SIZE(xDRI2GetBuffersReq, stuff->count * 4);

but since as Julien pointed out in another message, C does formally define
overflow handling on unsigned ints such as stuff->count, that shouldn't be
necessary here.

-- 
	-Alan Coopersmith-              alan.coopersmith at oracle.com
	 Oracle Solaris Engineering - http://blogs.oracle.com/alanc