From 2eaafbbb62a48ef190e45b3000e0caeb35546eb3 Mon Sep 17 00:00:00 2001
From: "Enrico Weigelt, metux IT consult" <info@metux.net>
Date: Thu, 4 Jul 2024 14:48:43 +0200
Subject: [PATCH] (submit/cleanup-vidmode-dispatch) Xext: vidmode: tidy up
 multi-version request control flow, part 1

Some requests using different structs dependending on which protocol version
(v1 vs. v2) had been selected. That's is handled by coverting v1 structs into v2,
before proceeding with the actual handling.

The code flow of this is very complex and hard to understand. Cleaning this up
in several smaller steps, that are easier to digest.

This moving the request size check into the if-version-X branches, to make it
some bit easier to undertand.

Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
---
 Xext/vidmode.c | 50 ++++++++++++++++++++++++++++++++------------------
 1 file changed, 32 insertions(+), 18 deletions(-)

diff --git a/Xext/vidmode.c b/Xext/vidmode.c
index f32086a74..bef61a0b5 100644
--- a/Xext/vidmode.c
+++ b/Xext/vidmode.c
@@ -464,12 +464,16 @@ ProcVidModeAddModeLine(ClientPtr client)
         len =
             client->req_len -
             bytes_to_int32(sizeof(xXF86OldVidModeAddModeLineReq));
+        if (len != oldstuff->privsize)
+            return BadLength;
     }
     else {
         REQUEST_AT_LEAST_SIZE(xXF86VidModeAddModeLineReq);
         len =
             client->req_len -
             bytes_to_int32(sizeof(xXF86VidModeAddModeLineReq));
+        if (len != stuff->privsize)
+            return BadLength;
     }
 
     if (ver < 2) {
@@ -519,9 +523,6 @@ ProcVidModeAddModeLine(ClientPtr client)
            stuff->after_vsyncend, stuff->after_vtotal,
            (unsigned long) stuff->after_flags);
 
-    if (len != stuff->privsize)
-        return BadLength;
-
     if (stuff->screen >= screenInfo.numScreens)
         return BadValue;
     pScreen = screenInfo.screens[stuff->screen];
@@ -638,12 +639,28 @@ ProcVidModeDeleteModeLine(ClientPtr client)
         len =
             client->req_len -
             bytes_to_int32(sizeof(xXF86OldVidModeDeleteModeLineReq));
+        if (len != oldstuff->privsize) {
+            DebugF("req_len = %ld, sizeof(Req) = %d, privsize = %ld, "
+                   "len = %d, length = %d\n",
+                   (unsigned long) client->req_len,
+                   (int) sizeof(xXF86VidModeDeleteModeLineReq) >> 2,
+                   (unsigned long) stuff->privsize, len, client->req_len);
+            return BadLength;
+        }
     }
     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",
+                   (unsigned long) client->req_len,
+                   (int) sizeof(xXF86VidModeDeleteModeLineReq) >> 2,
+                   (unsigned long) stuff->privsize, len, client->req_len);
+            return BadLength;
+        }
     }
 
     if (ver < 2) {
@@ -673,15 +690,6 @@ ProcVidModeDeleteModeLine(ClientPtr client)
            stuff->vdisplay, stuff->vsyncstart, stuff->vsyncend, stuff->vtotal,
            (unsigned long) stuff->flags);
 
-    if (len != stuff->privsize) {
-        DebugF("req_len = %ld, sizeof(Req) = %d, privsize = %ld, "
-               "len = %d, length = %d\n",
-               (unsigned long) client->req_len,
-               (int) sizeof(xXF86VidModeDeleteModeLineReq) >> 2,
-               (unsigned long) stuff->privsize, len, client->req_len);
-        return BadLength;
-    }
-
     if (stuff->screen >= screenInfo.numScreens)
         return BadValue;
     pScreen = screenInfo.screens[stuff->screen];
@@ -766,12 +774,16 @@ ProcVidModeModModeLine(ClientPtr client)
         len =
             client->req_len -
             bytes_to_int32(sizeof(xXF86OldVidModeModModeLineReq));
+        if (len != oldstuff->privsize)
+            return BadLength;
     }
     else {
         REQUEST_AT_LEAST_SIZE(xXF86VidModeModModeLineReq);
         len =
             client->req_len -
             bytes_to_int32(sizeof(xXF86VidModeModModeLineReq));
+        if (len != stuff->privsize)
+            return BadLength;
     }
 
     if (ver < 2) {
@@ -798,9 +810,6 @@ ProcVidModeModModeLine(ClientPtr client)
            stuff->vdisplay, stuff->vsyncstart, stuff->vsyncend,
            stuff->vtotal, (unsigned long) stuff->flags);
 
-    if (len != stuff->privsize)
-        return BadLength;
-
     if (stuff->hsyncstart < stuff->hdisplay ||
         stuff->hsyncend < stuff->hsyncstart ||
         stuff->htotal < stuff->hsyncend ||
@@ -900,12 +909,16 @@ ProcVidModeValidateModeLine(ClientPtr client)
         REQUEST_AT_LEAST_SIZE(xXF86OldVidModeValidateModeLineReq);
         len = client->req_len -
             bytes_to_int32(sizeof(xXF86OldVidModeValidateModeLineReq));
+        if (len != oldstuff->privsize)
+            return BadLength;
     }
     else {
         REQUEST_AT_LEAST_SIZE(xXF86VidModeValidateModeLineReq);
         len =
             client->req_len -
             bytes_to_int32(sizeof(xXF86VidModeValidateModeLineReq));
+        if (len != stuff->privsize)
+            return BadLength;
     }
 
     if (ver < 2) {
@@ -1061,12 +1074,16 @@ ProcVidModeSwitchToMode(ClientPtr client)
         len =
             client->req_len -
             bytes_to_int32(sizeof(xXF86OldVidModeSwitchToModeReq));
+        if (len != stuff->privsize)
+            return BadLength;
     }
     else {
         REQUEST_AT_LEAST_SIZE(xXF86VidModeSwitchToModeReq);
         len =
             client->req_len -
             bytes_to_int32(sizeof(xXF86VidModeSwitchToModeReq));
+        if (len != stuff->privsize)
+            return BadLength;
     }
 
     if (ver < 2) {
@@ -1097,9 +1114,6 @@ ProcVidModeSwitchToMode(ClientPtr client)
            stuff->vdisplay, stuff->vsyncstart, stuff->vsyncend, stuff->vtotal,
            (unsigned long) stuff->flags);
 
-    if (len != stuff->privsize)
-        return BadLength;
-
     if (stuff->screen >= screenInfo.numScreens)
         return BadValue;
     pScreen = screenInfo.screens[stuff->screen];