hw/xfree86: unvalidated lengths

This addresses:
CVE-2017-12180 in XFree86-VidModeExtension
CVE-2017-12181 in XFree86-DGA
CVE-2017-12182 in XFree86-DRI

Reviewed-by: Jeremy Huddleston Sequoia <jeremyhu@apple.com>
Reviewed-by: Julien Cristau <jcristau@debian.org>
Signed-off-by: Nathan Kidd <nkidd@opentext.com>
Signed-off-by: Julien Cristau <jcristau@debian.org>

3 files changed, 117 insertions, 94 deletions

diff --git a/Xext/vidmode.c b/Xext/vidmode.c
index 8ba919a3c..6e4a7c709 100644
--- a/Xext/vidmode.c
+++ b/Xext/vidmode.c
@@ -454,6 +454,20 @@ ProcVidModeAddModeLine(ClientPtr client)
     DEBUG_P("XF86VidModeAddModeline");
 
     ver = ClientMajorVersion(client);
+
+    if (ver < 2) {
+        REQUEST_AT_LEAST_SIZE(xXF86OldVidModeAddModeLineReq);
+        len =
+            client->req_len -
+            bytes_to_int32(sizeof(xXF86OldVidModeAddModeLineReq));
+    }
+    else {
+        REQUEST_AT_LEAST_SIZE(xXF86VidModeAddModeLineReq);
+        len =
+            client->req_len -
+            bytes_to_int32(sizeof(xXF86VidModeAddModeLineReq));
+    }
+
     if (ver < 2) {
         /* convert from old format */
         stuff = &newstuff;
@@ -501,18 +515,6 @@ ProcVidModeAddModeLine(ClientPtr client)
            stuff->after_vsyncend, stuff->after_vtotal,
            (unsigned long) stuff->after_flags);
 
-    if (ver < 2) {
-        REQUEST_AT_LEAST_SIZE(xXF86OldVidModeAddModeLineReq);
-        len =
-            client->req_len -
-            bytes_to_int32(sizeof(xXF86OldVidModeAddModeLineReq));
-    }
-    else {
-        REQUEST_AT_LEAST_SIZE(xXF86VidModeAddModeLineReq);
-        len =
-            client->req_len -
-            bytes_to_int32(sizeof(xXF86VidModeAddModeLineReq));
-    }
     if (len != stuff->privsize)
         return BadLength;
 
@@ -622,6 +624,20 @@ ProcVidModeDeleteModeLine(ClientPtr client)
     DEBUG_P("XF86VidModeDeleteModeline");
 
     ver = ClientMajorVersion(client);
+
+    if (ver < 2) {
+        REQUEST_AT_LEAST_SIZE(xXF86OldVidModeDeleteModeLineReq);
+        len =
+            client->req_len -
+            bytes_to_int32(sizeof(xXF86OldVidModeDeleteModeLineReq));
+    }
+    else {
+        REQUEST_AT_LEAST_SIZE(xXF86VidModeDeleteModeLineReq);
+        len =
+            client->req_len -
+            bytes_to_int32(sizeof(xXF86VidModeDeleteModeLineReq));
+    }
+
     if (ver < 2) {
         /* convert from old format */
         stuff = &newstuff;
@@ -649,18 +665,6 @@ ProcVidModeDeleteModeLine(ClientPtr client)
            stuff->vdisplay, stuff->vsyncstart, stuff->vsyncend, stuff->vtotal,
            (unsigned long) stuff->flags);
 
-    if (ver < 2) {
-        REQUEST_AT_LEAST_SIZE(xXF86OldVidModeDeleteModeLineReq);
-        len =
-            client->req_len -
-            bytes_to_int32(sizeof(xXF86OldVidModeDeleteModeLineReq));
-    }
-    else {
-        REQUEST_AT_LEAST_SIZE(xXF86VidModeDeleteModeLineReq);
-        len =
-            client->req_len -
-            bytes_to_int32(sizeof(xXF86VidModeDeleteModeLineReq));
-    }
     if (len != stuff->privsize) {
         DebugF("req_len = %ld, sizeof(Req) = %d, privsize = %ld, "
                "len = %d, length = %d\n",
@@ -744,6 +748,20 @@ ProcVidModeModModeLine(ClientPtr client)
     DEBUG_P("XF86VidModeModModeline");
 
     ver = ClientMajorVersion(client);
+
+    if (ver < 2) {
+        REQUEST_AT_LEAST_SIZE(xXF86OldVidModeModModeLineReq);
+        len =
+            client->req_len -
+            bytes_to_int32(sizeof(xXF86OldVidModeModModeLineReq));
+    }
+    else {
+        REQUEST_AT_LEAST_SIZE(xXF86VidModeModModeLineReq);
+        len =
+            client->req_len -
+            bytes_to_int32(sizeof(xXF86VidModeModModeLineReq));
+    }
+
     if (ver < 2) {
         /* convert from old format */
         stuff = &newstuff;
@@ -768,18 +786,6 @@ ProcVidModeModModeLine(ClientPtr client)
            stuff->vdisplay, stuff->vsyncstart, stuff->vsyncend,
            stuff->vtotal, (unsigned long) stuff->flags);
 
-    if (ver < 2) {
-        REQUEST_AT_LEAST_SIZE(xXF86OldVidModeModModeLineReq);
-        len =
-            client->req_len -
-            bytes_to_int32(sizeof(xXF86OldVidModeModModeLineReq));
-    }
-    else {
-        REQUEST_AT_LEAST_SIZE(xXF86VidModeModModeLineReq);
-        len =
-            client->req_len -
-            bytes_to_int32(sizeof(xXF86VidModeModModeLineReq));
-    }
     if (len != stuff->privsize)
         return BadLength;
 
@@ -877,6 +883,19 @@ ProcVidModeValidateModeLine(ClientPtr client)
     DEBUG_P("XF86VidModeValidateModeline");
 
     ver = ClientMajorVersion(client);
+
+    if (ver < 2) {
+        REQUEST_AT_LEAST_SIZE(xXF86OldVidModeValidateModeLineReq);
+        len = client->req_len -
+            bytes_to_int32(sizeof(xXF86OldVidModeValidateModeLineReq));
+    }
+    else {
+        REQUEST_AT_LEAST_SIZE(xXF86VidModeValidateModeLineReq);
+        len =
+            client->req_len -
+            bytes_to_int32(sizeof(xXF86VidModeValidateModeLineReq));
+    }
+
     if (ver < 2) {
         /* convert from old format */
         stuff = &newstuff;
@@ -905,17 +924,6 @@ ProcVidModeValidateModeLine(ClientPtr client)
            stuff->vdisplay, stuff->vsyncstart, stuff->vsyncend, stuff->vtotal,
            (unsigned long) stuff->flags);
 
-    if (ver < 2) {
-        REQUEST_AT_LEAST_SIZE(xXF86OldVidModeValidateModeLineReq);
-        len = client->req_len -
-            bytes_to_int32(sizeof(xXF86OldVidModeValidateModeLineReq));
-    }
-    else {
-        REQUEST_AT_LEAST_SIZE(xXF86VidModeValidateModeLineReq);
-        len =
-            client->req_len -
-            bytes_to_int32(sizeof(xXF86VidModeValidateModeLineReq));
-    }
     if (len != stuff->privsize)
         return BadLength;
 
@@ -1027,6 +1035,20 @@ ProcVidModeSwitchToMode(ClientPtr client)
     DEBUG_P("XF86VidModeSwitchToMode");
 
     ver = ClientMajorVersion(client);
+
+    if (ver < 2) {
+        REQUEST_AT_LEAST_SIZE(xXF86OldVidModeSwitchToModeReq);
+        len =
+            client->req_len -
+            bytes_to_int32(sizeof(xXF86OldVidModeSwitchToModeReq));
+    }
+    else {
+        REQUEST_AT_LEAST_SIZE(xXF86VidModeSwitchToModeReq);
+        len =
+            client->req_len -
+            bytes_to_int32(sizeof(xXF86VidModeSwitchToModeReq));
+    }
+
     if (ver < 2) {
         /* convert from old format */
         stuff = &newstuff;
@@ -1055,18 +1077,6 @@ ProcVidModeSwitchToMode(ClientPtr client)
            stuff->vdisplay, stuff->vsyncstart, stuff->vsyncend, stuff->vtotal,
            (unsigned long) stuff->flags);
 
-    if (ver < 2) {
-        REQUEST_AT_LEAST_SIZE(xXF86OldVidModeSwitchToModeReq);
-        len =
-            client->req_len -
-            bytes_to_int32(sizeof(xXF86OldVidModeSwitchToModeReq));
-    }
-    else {
-        REQUEST_AT_LEAST_SIZE(xXF86VidModeSwitchToModeReq);
-        len =
-            client->req_len -
-            bytes_to_int32(sizeof(xXF86VidModeSwitchToModeReq));
-    }
     if (len != stuff->privsize)
         return BadLength;
 
@@ -1457,6 +1467,7 @@ ProcVidModeSetGammaRamp(ClientPtr client)
     VidModePtr pVidMode;
 
     REQUEST(xXF86VidModeSetGammaRampReq);
+    REQUEST_AT_LEAST_SIZE(xXF86VidModeSetGammaRampReq);
 
     if (stuff->screen >= screenInfo.numScreens)
         return BadValue;
diff --git a/hw/xfree86/common/xf86DGA.c b/hw/xfree86/common/xf86DGA.c
index 95434e882..505b01917 100644
--- a/hw/xfree86/common/xf86DGA.c
+++ b/hw/xfree86/common/xf86DGA.c
@@ -1272,13 +1272,14 @@ ProcXDGAOpenFramebuffer(ClientPtr client)
     char *deviceName;
     int nameSize;
 
+    REQUEST_SIZE_MATCH(xXDGAOpenFramebufferReq);
+
     if (stuff->screen >= screenInfo.numScreens)
         return BadValue;
 
     if (!DGAAvailable(stuff->screen))
         return DGAErrorBase + XF86DGANoDirectVideoMode;
 
-    REQUEST_SIZE_MATCH(xXDGAOpenFramebufferReq);
     rep.type = X_Reply;
     rep.length = 0;
     rep.sequenceNumber = client->sequence;
@@ -1305,14 +1306,14 @@ ProcXDGACloseFramebuffer(ClientPtr client)
 {
     REQUEST(xXDGACloseFramebufferReq);
 
+    REQUEST_SIZE_MATCH(xXDGACloseFramebufferReq);
+
     if (stuff->screen >= screenInfo.numScreens)
         return BadValue;
 
     if (!DGAAvailable(stuff->screen))
         return DGAErrorBase + XF86DGANoDirectVideoMode;
 
-    REQUEST_SIZE_MATCH(xXDGACloseFramebufferReq);
-
     DGACloseFramebuffer(stuff->screen);
 
     return Success;
@@ -1328,10 +1329,11 @@ ProcXDGAQueryModes(ClientPtr client)
     xXDGAModeInfo info;
     XDGAModePtr mode;
 
+    REQUEST_SIZE_MATCH(xXDGAQueryModesReq);
+
     if (stuff->screen >= screenInfo.numScreens)
         return BadValue;
 
-    REQUEST_SIZE_MATCH(xXDGAQueryModesReq);
     rep.type = X_Reply;
     rep.length = 0;
     rep.number = 0;
@@ -1443,11 +1445,12 @@ ProcXDGASetMode(ClientPtr client)
     ClientPtr owner;
     int size;
 
+    REQUEST_SIZE_MATCH(xXDGASetModeReq);
+
     if (stuff->screen >= screenInfo.numScreens)
         return BadValue;
     owner = DGA_GETCLIENT(stuff->screen);
 
-    REQUEST_SIZE_MATCH(xXDGASetModeReq);
     rep.type = X_Reply;
     rep.length = 0;
     rep.offset = 0;
@@ -1533,14 +1536,14 @@ ProcXDGASetViewport(ClientPtr client)
 {
     REQUEST(xXDGASetViewportReq);
 
+    REQUEST_SIZE_MATCH(xXDGASetViewportReq);
+
     if (stuff->screen >= screenInfo.numScreens)
         return BadValue;
 
     if (DGA_GETCLIENT(stuff->screen) != client)
         return DGAErrorBase + XF86DGADirectNotActivated;
 
-    REQUEST_SIZE_MATCH(xXDGASetViewportReq);
-
     DGASetViewport(stuff->screen, stuff->x, stuff->y, stuff->flags);
 
     return Success;
@@ -1554,14 +1557,14 @@ ProcXDGAInstallColormap(ClientPtr client)
 
     REQUEST(xXDGAInstallColormapReq);
 
+    REQUEST_SIZE_MATCH(xXDGAInstallColormapReq);
+
     if (stuff->screen >= screenInfo.numScreens)
         return BadValue;
 
     if (DGA_GETCLIENT(stuff->screen) != client)
         return DGAErrorBase + XF86DGADirectNotActivated;
 
-    REQUEST_SIZE_MATCH(xXDGAInstallColormapReq);
-
     rc = dixLookupResourceByType((void **) &cmap, stuff->cmap, RT_COLORMAP,
                                  client, DixInstallAccess);
     if (rc != Success)
@@ -1575,14 +1578,14 @@ ProcXDGASelectInput(ClientPtr client)
 {
     REQUEST(xXDGASelectInputReq);
 
+    REQUEST_SIZE_MATCH(xXDGASelectInputReq);
+
     if (stuff->screen >= screenInfo.numScreens)
         return BadValue;
 
     if (DGA_GETCLIENT(stuff->screen) != client)
         return DGAErrorBase + XF86DGADirectNotActivated;
 
-    REQUEST_SIZE_MATCH(xXDGASelectInputReq);
-
     if (DGA_GETCLIENT(stuff->screen) == client)
         DGASelectInput(stuff->screen, client, stuff->mask);
 
@@ -1594,14 +1597,14 @@ ProcXDGAFillRectangle(ClientPtr client)
 {
     REQUEST(xXDGAFillRectangleReq);
 
+    REQUEST_SIZE_MATCH(xXDGAFillRectangleReq);
+
     if (stuff->screen >= screenInfo.numScreens)
         return BadValue;
 
     if (DGA_GETCLIENT(stuff->screen) != client)
         return DGAErrorBase + XF86DGADirectNotActivated;
 
-    REQUEST_SIZE_MATCH(xXDGAFillRectangleReq);
-
     if (Success != DGAFillRect(stuff->screen, stuff->x, stuff->y,
                                stuff->width, stuff->height, stuff->color))
         return BadMatch;
@@ -1614,14 +1617,14 @@ ProcXDGACopyArea(ClientPtr client)
 {
     REQUEST(xXDGACopyAreaReq);
 
+    REQUEST_SIZE_MATCH(xXDGACopyAreaReq);
+
     if (stuff->screen >= screenInfo.numScreens)
         return BadValue;
 
     if (DGA_GETCLIENT(stuff->screen) != client)
         return DGAErrorBase + XF86DGADirectNotActivated;
 
-    REQUEST_SIZE_MATCH(xXDGACopyAreaReq);
-
     if (Success != DGABlitRect(stuff->screen, stuff->srcx, stuff->srcy,
                                stuff->width, stuff->height, stuff->dstx,
                                stuff->dsty))
@@ -1635,14 +1638,14 @@ ProcXDGACopyTransparentArea(ClientPtr client)
 {
     REQUEST(xXDGACopyTransparentAreaReq);
 
+    REQUEST_SIZE_MATCH(xXDGACopyTransparentAreaReq);
+
     if (stuff->screen >= screenInfo.numScreens)
         return BadValue;
 
     if (DGA_GETCLIENT(stuff->screen) != client)
         return DGAErrorBase + XF86DGADirectNotActivated;
 
-    REQUEST_SIZE_MATCH(xXDGACopyTransparentAreaReq);
-
     if (Success != DGABlitTransRect(stuff->screen, stuff->srcx, stuff->srcy,
                                     stuff->width, stuff->height, stuff->dstx,
                                     stuff->dsty, stuff->key))
@@ -1657,13 +1660,14 @@ ProcXDGAGetViewportStatus(ClientPtr client)
     REQUEST(xXDGAGetViewportStatusReq);
     xXDGAGetViewportStatusReply rep;
 
+    REQUEST_SIZE_MATCH(xXDGAGetViewportStatusReq);
+
     if (stuff->screen >= screenInfo.numScreens)
         return BadValue;
 
     if (DGA_GETCLIENT(stuff->screen) != client)
         return DGAErrorBase + XF86DGADirectNotActivated;
 
-    REQUEST_SIZE_MATCH(xXDGAGetViewportStatusReq);
     rep.type = X_Reply;
     rep.length = 0;
     rep.sequenceNumber = client->sequence;
@@ -1680,13 +1684,14 @@ ProcXDGASync(ClientPtr client)
     REQUEST(xXDGASyncReq);
     xXDGASyncReply rep;
 
+    REQUEST_SIZE_MATCH(xXDGASyncReq);
+
     if (stuff->screen >= screenInfo.numScreens)
         return BadValue;
 
     if (DGA_GETCLIENT(stuff->screen) != client)
         return DGAErrorBase + XF86DGADirectNotActivated;
 
-    REQUEST_SIZE_MATCH(xXDGASyncReq);
     rep.type = X_Reply;
     rep.length = 0;
     rep.sequenceNumber = client->sequence;
@@ -1725,13 +1730,14 @@ ProcXDGAChangePixmapMode(ClientPtr client)
     xXDGAChangePixmapModeReply rep;
     int x, y;
 
+    REQUEST_SIZE_MATCH(xXDGAChangePixmapModeReq);
+
     if (stuff->screen >= screenInfo.numScreens)
         return BadValue;
 
     if (DGA_GETCLIENT(stuff->screen) != client)
         return DGAErrorBase + XF86DGADirectNotActivated;
 
-    REQUEST_SIZE_MATCH(xXDGAChangePixmapModeReq);
     rep.type = X_Reply;
     rep.length = 0;
     rep.sequenceNumber = client->sequence;
@@ -1755,14 +1761,14 @@ ProcXDGACreateColormap(ClientPtr client)
     REQUEST(xXDGACreateColormapReq);
     int result;
 
+    REQUEST_SIZE_MATCH(xXDGACreateColormapReq);
+
     if (stuff->screen >= screenInfo.numScreens)
         return BadValue;
 
     if (DGA_GETCLIENT(stuff->screen) != client)
         return DGAErrorBase + XF86DGADirectNotActivated;
 
-    REQUEST_SIZE_MATCH(xXDGACreateColormapReq);
-
     if (!stuff->mode)
         return BadValue;
 
@@ -1791,10 +1797,11 @@ ProcXF86DGAGetVideoLL(ClientPtr client)
     int num, offset, flags;
     char *name;
 
+    REQUEST_SIZE_MATCH(xXF86DGAGetVideoLLReq);
+
     if (stuff->screen >= screenInfo.numScreens)
         return BadValue;
 
-    REQUEST_SIZE_MATCH(xXF86DGAGetVideoLLReq);
     rep.type = X_Reply;
     rep.length = 0;
     rep.sequenceNumber = client->sequence;
@@ -1831,9 +1838,10 @@ ProcXF86DGADirectVideo(ClientPtr client)
 
     REQUEST(xXF86DGADirectVideoReq);
 
+    REQUEST_SIZE_MATCH(xXF86DGADirectVideoReq);
+
     if (stuff->screen >= screenInfo.numScreens)
         return BadValue;
-    REQUEST_SIZE_MATCH(xXF86DGADirectVideoReq);
 
     if (!DGAAvailable(stuff->screen))
         return DGAErrorBase + XF86DGANoDirectVideoMode;
@@ -1889,10 +1897,11 @@ ProcXF86DGAGetViewPortSize(ClientPtr client)
     REQUEST(xXF86DGAGetViewPortSizeReq);
     xXF86DGAGetViewPortSizeReply rep;
 
+    REQUEST_SIZE_MATCH(xXF86DGAGetViewPortSizeReq);
+
     if (stuff->screen >= screenInfo.numScreens)
         return BadValue;
 
-    REQUEST_SIZE_MATCH(xXF86DGAGetViewPortSizeReq);
     rep.type = X_Reply;
     rep.length = 0;
     rep.sequenceNumber = client->sequence;
@@ -1917,14 +1926,14 @@ ProcXF86DGASetViewPort(ClientPtr client)
 {
     REQUEST(xXF86DGASetViewPortReq);
 
+    REQUEST_SIZE_MATCH(xXF86DGASetViewPortReq);
+
     if (stuff->screen >= screenInfo.numScreens)
         return BadValue;
 
     if (DGA_GETCLIENT(stuff->screen) != client)
         return DGAErrorBase + XF86DGADirectNotActivated;
 
-    REQUEST_SIZE_MATCH(xXF86DGASetViewPortReq);
-
     if (!DGAAvailable(stuff->screen))
         return DGAErrorBase + XF86DGANoDirectVideoMode;
 
@@ -1944,10 +1953,11 @@ ProcXF86DGAGetVidPage(ClientPtr client)
     REQUEST(xXF86DGAGetVidPageReq);
     xXF86DGAGetVidPageReply rep;
 
+    REQUEST_SIZE_MATCH(xXF86DGAGetVidPageReq);
+
     if (stuff->screen >= screenInfo.numScreens)
         return BadValue;
 
-    REQUEST_SIZE_MATCH(xXF86DGAGetVidPageReq);
     rep.type = X_Reply;
     rep.length = 0;
     rep.sequenceNumber = client->sequence;
@@ -1962,11 +1972,11 @@ ProcXF86DGASetVidPage(ClientPtr client)
 {
     REQUEST(xXF86DGASetVidPageReq);
 
+    REQUEST_SIZE_MATCH(xXF86DGASetVidPageReq);
+
     if (stuff->screen >= screenInfo.numScreens)
         return BadValue;
 
-    REQUEST_SIZE_MATCH(xXF86DGASetVidPageReq);
-
     /* silently fail */
 
     return Success;
@@ -1980,14 +1990,14 @@ ProcXF86DGAInstallColormap(ClientPtr client)
 
     REQUEST(xXF86DGAInstallColormapReq);
 
+    REQUEST_SIZE_MATCH(xXF86DGAInstallColormapReq);
+
     if (stuff->screen >= screenInfo.numScreens)
         return BadValue;
 
     if (DGA_GETCLIENT(stuff->screen) != client)
         return DGAErrorBase + XF86DGADirectNotActivated;
 
-    REQUEST_SIZE_MATCH(xXF86DGAInstallColormapReq);
-
     if (!DGAActive(stuff->screen))
         return DGAErrorBase + XF86DGADirectNotActivated;
 
@@ -2008,10 +2018,11 @@ ProcXF86DGAQueryDirectVideo(ClientPtr client)
     REQUEST(xXF86DGAQueryDirectVideoReq);
     xXF86DGAQueryDirectVideoReply rep;
 
+    REQUEST_SIZE_MATCH(xXF86DGAQueryDirectVideoReq);
+
     if (stuff->screen >= screenInfo.numScreens)
         return BadValue;
 
-    REQUEST_SIZE_MATCH(xXF86DGAQueryDirectVideoReq);
     rep.type = X_Reply;
     rep.length = 0;
     rep.sequenceNumber = client->sequence;
@@ -2030,14 +2041,14 @@ ProcXF86DGAViewPortChanged(ClientPtr client)
     REQUEST(xXF86DGAViewPortChangedReq);
     xXF86DGAViewPortChangedReply rep;
 
+    REQUEST_SIZE_MATCH(xXF86DGAViewPortChangedReq);
+
     if (stuff->screen >= screenInfo.numScreens)
         return BadValue;
 
     if (DGA_GETCLIENT(stuff->screen) != client)
         return DGAErrorBase + XF86DGADirectNotActivated;
 
-    REQUEST_SIZE_MATCH(xXF86DGAViewPortChangedReq);
-
     if (!DGAActive(stuff->screen))
         return DGAErrorBase + XF86DGADirectNotActivated;
 
diff --git a/hw/xfree86/dri/xf86dri.c b/hw/xfree86/dri/xf86dri.c
index 8f3c2d602..d356db990 100644
--- a/hw/xfree86/dri/xf86dri.c
+++ b/hw/xfree86/dri/xf86dri.c
@@ -570,6 +570,7 @@ static int _X_COLD
 SProcXF86DRIQueryDirectRenderingCapable(register ClientPtr client)
 {
     REQUEST(xXF86DRIQueryDirectRenderingCapableReq);
+    REQUEST_SIZE_MATCH(xXF86DRIQueryDirectRenderingCapableReq);
     swaps(&stuff->length);
     swapl(&stuff->screen);
     return ProcXF86DRIQueryDirectRenderingCapable(client);