integer overflow check

[jes at posteo.de]

Hello,

the recent xserver security patches included this patch:
http://cgit.freedesktop.org/xorg/xserver/commit/?id=6692670fde081bbfe9313f17d84037ae9116702a

I wonder why there is no integer overflow check in line 300 of
http://cgit.freedesktop.org/xorg/xserver/tree/hw/xfree86/dri2/dri2ext.c

I'm asking myself why the overflow check is made after the 
REQUEST_FIXED_SIZE call.
I read the macro and things should not explode when an overflow will 
happen.
The undefined behavior could be avoided by moving the check before the 
call.

I add some code, please let me know what you think.


Jan

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;
@@ -297,6 +298,9 @@ ProcDRI2GetBuffersWithFormat(ClientPtr client)
      int status, width, height, count;
      unsigned int *attachments;

+    if (stuff->count > (INT_MAX / (2 * 4)))
+        return BadLength;
+
      REQUEST_FIXED_SIZE(xDRI2GetBuffersReq, stuff->count * (2 * 4));
      if (!validDrawable(client, stuff->drawable, DixReadAccess | 
DixWriteAccess,
                         &pDrawable, &status))