From 118a24dadc369815afd31a69dfbc6da2d959a756 Mon Sep 17 00:00:00 2001
From: "Enrico Weigelt, metux IT consult" <info@metux.net>
Date: Thu, 15 Aug 2024 10:58:18 +0200
Subject: [PATCH] Xnest: fix user specified color depth and memleak

Retrieving visuals offered by upstream Xserver is broken in several ways:

a) duplicate elimination breaks out too fast: when a duplicate is found,
   it doesn't just skips that one, it completely breaks out the loop,
   so subsequent upstream visuals aren't considered anymore. that's leading
   to (unpredictable) limit on available color depths (depending on the order
   reported by upstream sever)

b) buffer overflow when user specificed different depth/class than default one:
   xnestOpenScreen() looks into the wrong table: it's local visuals[] array,
   instead of the global (non-dedup'ed) list fetched by xlib. The visuals[]
   array is *much* smaller (deduplicated) than the xnestVisuals[] array,
   and xnestDefaultVisualIndex is likely to point outside of visual[]'s bounds.

To make it actually work against an Xorg upstream server, the upstream server
needs fix for another bug in the DIX:

    https://gitlab.freedesktop.org/xorg/xserver/-/issues/1741
    https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1644

Closes: https://gitlab.freedesktop.org/xorg/xserver/-/issues/1742
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
---
 hw/xnest/Display.c | 25 -------------------------
 hw/xnest/Display.h |  1 -
 hw/xnest/Screen.c  | 29 ++++++++++++++++++++++++++---
 3 files changed, 26 insertions(+), 29 deletions(-)

diff --git a/hw/xnest/Display.c b/hw/xnest/Display.c
index 0735de772..574effd98 100644
--- a/hw/xnest/Display.c
+++ b/hw/xnest/Display.c
@@ -42,7 +42,6 @@ is" without express or implied warranty.
 Display *xnestDisplay = NULL;
 XVisualInfo *xnestVisuals;
 int xnestNumVisuals;
-int xnestDefaultVisualIndex;
 Colormap *xnestDefaultColormaps;
 static uint16_t xnestNumDefaultColormaps;
 int *xnestDepths;
@@ -94,30 +93,6 @@ xnestOpenDisplay(int argc, char *argv[])
     if (xnestNumVisuals == 0 || xnestVisuals == NULL)
         FatalError("Unable to find any visuals.\n");
 
-    if (xnestUserDefaultClass || xnestUserDefaultDepth) {
-        xnestDefaultVisualIndex = UNDEFINED;
-        for (i = 0; i < xnestNumVisuals; i++)
-            if ((!xnestUserDefaultClass ||
-                 xnestVisuals[i].class == xnestDefaultClass)
-                &&
-                (!xnestUserDefaultDepth ||
-                 xnestVisuals[i].depth == xnestDefaultDepth)) {
-                xnestDefaultVisualIndex = i;
-                break;
-            }
-        if (xnestDefaultVisualIndex == UNDEFINED)
-            FatalError("Unable to find desired default visual.\n");
-    }
-    else {
-        vi.visualid = XVisualIDFromVisual(DefaultVisual(xnestDisplay,
-                                                        DefaultScreen
-                                                        (xnestDisplay)));
-        xnestDefaultVisualIndex = 0;
-        for (i = 0; i < xnestNumVisuals; i++)
-            if (vi.visualid == xnestVisuals[i].visualid)
-                xnestDefaultVisualIndex = i;
-    }
-
     xnestNumDefaultColormaps = xnestNumVisuals;
     xnestDefaultColormaps = xallocarray(xnestNumDefaultColormaps,
                                         sizeof(Colormap));
diff --git a/hw/xnest/Display.h b/hw/xnest/Display.h
index 4e9dbf463..b7fbad877 100644
--- a/hw/xnest/Display.h
+++ b/hw/xnest/Display.h
@@ -23,7 +23,6 @@ is" without express or implied warranty.
 extern Display *xnestDisplay;
 extern XVisualInfo *xnestVisuals;
 extern int xnestNumVisuals;
-extern int xnestDefaultVisualIndex;
 extern Colormap *xnestDefaultColormaps;
 extern int xnestNumDefaultClormaps;
 extern int *xnestDepths;
diff --git a/hw/xnest/Screen.c b/hw/xnest/Screen.c
index 6eb767233..a8d8a66f7 100644
--- a/hw/xnest/Screen.c
+++ b/hw/xnest/Screen.c
@@ -176,6 +176,8 @@ xnestOpenScreen(ScreenPtr pScreen, int argc, char *argv[])
     depths[0].vids = (VisualID *) malloc(MAXVISUALSPERDEPTH * sizeof(VisualID));
     numDepths = 1;
 
+    int found_default_visual = 0;
+
     for (i = 0; i < xnestNumVisuals; i++) {
         visuals[numVisuals].class = xnestVisuals[i].class;
         visuals[numVisuals].bitsPerRGBValue = xnestVisuals[i].bits_per_rgb;
@@ -205,7 +207,7 @@ xnestOpenScreen(ScreenPtr pScreen, int argc, char *argv[])
                 break;
         }
         if (j < numVisuals)
-            break;
+            continue;
 
         visuals[numVisuals].vid = FakeClientID(0);
 
@@ -231,12 +233,33 @@ xnestOpenScreen(ScreenPtr pScreen, int argc, char *argv[])
             visuals[numVisuals].vid;
         depths[depthIndex].numVids++;
 
+        if (xnestUserDefaultClass || xnestUserDefaultDepth) {
+            if ((!xnestDefaultClass || visuals[numVisuals].class == xnestDefaultClass) &&
+                (!xnestDefaultDepth || visuals[numVisuals].nplanes == xnestDefaultDepth))
+            {
+                defaultVisual = visuals[numVisuals].vid;
+                rootDepth = visuals[numVisuals].nplanes;
+                found_default_visual = 1;
+            }
+        }
+        else
+        {
+            VisualID visual_id = XVisualIDFromVisual(DefaultVisual(xnestDisplay, DefaultScreen(xnestDisplay)));
+            if (visual_id == xnestVisuals[i].visualid) {
+                defaultVisual = visuals[numVisuals].vid;
+                rootDepth = visuals[numVisuals].nplanes;
+                found_default_visual = 1;
+            }
+        }
         numVisuals++;
     }
     visuals = reallocarray(visuals, numVisuals, sizeof(VisualRec));
 
-    defaultVisual = visuals[xnestDefaultVisualIndex].vid;
-    rootDepth = visuals[xnestDefaultVisualIndex].nplanes;
+    if (!found_default_visual) {
+        ErrorF("Xnest: can't find matching visual for user specified depth %d\n", xnestDefaultDepth);
+        defaultVisual = visuals[0].vid;
+        rootDepth = visuals[0].nplanes;
+    }
 
     if (xnestParentWindow != 0) {
         XGetWindowAttributes(xnestDisplay, xnestParentWindow, &gattributes);