[PATCH xserver 15/15] dri3: correctly handle failed get_supported_modifiers requests

[Emil Velikov]

From: Emil Velikov <emil.velikov at collabora.com>

Currently depending on the code path hit, the helper will set some of
the output values and not others.

It could also leak memory ;-)

At the same time the caller was:
 - working around the broken behaviour - by initialising the variables
 - outright ignoring if the helper fails

Fix all that by consistently setting the output variables and returning
a protocol error if we fail to get the data required.

Bonus points: current code does not attribute if we cannot get the
modifiers info for certain format. A smaller format array is available,
yet the original length is stored.

Fixes: cef12efc15c ("glamor: Implement GetSupportedModifiers")
Cc: Louis-Francis Ratté-Boulianne <lfrb at collabora.com>
Cc: Daniel Stone <daniels at collabora.com>
Signed-off-by: Emil Velikov <emil.velikov at collabora.com>
---
We could still return "success" to the user - I've opted for this
solution since we already bail on dixLookupWindow failure.
---
 dri3/dri3_request.c | 18 ++++++++++--------
 dri3/dri3_screen.c  | 38 +++++++++++++++++++++-----------------
 2 files changed, 31 insertions(+), 25 deletions(-)

diff --git a/dri3/dri3_request.c b/dri3/dri3_request.c
index edcd0e782..9b7d81ea3 100644
--- a/dri3/dri3_request.c
+++ b/dri3/dri3_request.c
@@ -335,10 +335,10 @@ proc_dri3_get_supported_modifiers(ClientPtr client)
     };
     WindowPtr window;
     ScreenPtr pScreen;
-    CARD64 *window_modifiers = NULL;
-    CARD64 *screen_modifiers = NULL;
-    CARD32 nwindowmodifiers = 0;
-    CARD32 nscreenmodifiers = 0;
+    CARD64 *window_modifiers;
+    CARD64 *screen_modifiers;
+    CARD32 nwindowmodifiers;
+    CARD32 nscreenmodifiers;
     int status;
     int i;
 
@@ -349,10 +349,12 @@ proc_dri3_get_supported_modifiers(ClientPtr client)
         return status;
     pScreen = window->drawable.pScreen;
 
-    dri3_get_supported_modifiers(pScreen, &window->drawable,
-				 stuff->depth, stuff->bpp,
-                                 &nwindowmodifiers, &window_modifiers,
-                                 &nscreenmodifiers, &screen_modifiers);
+    status = dri3_get_supported_modifiers(pScreen, &window->drawable,
+                                          stuff->depth, stuff->bpp,
+                                          &nwindowmodifiers, &window_modifiers,
+                                          &nscreenmodifiers, &screen_modifiers);
+    if (status != Success)
+        return status;
 
     rep.numWindowModifiers = nwindowmodifiers;
     rep.numScreenModifiers = nscreenmodifiers;
diff --git a/dri3/dri3_screen.c b/dri3/dri3_screen.c
index 2b0e139fa..de97d55bc 100644
--- a/dri3/dri3_screen.c
+++ b/dri3/dri3_screen.c
@@ -178,7 +178,9 @@ dri3_get_supported_modifiers(ScreenPtr screen, DrawablePtr drawable,
     int                         ret;
     CARD32                      num_drawable_mods;
     CARD64                     *drawable_mods;
+    CARD32                      num_window_mods = 0;
     CARD64                     *window_mods = NULL;
+    CARD32                      num_screen_mods = 0;
     CARD64                     *screen_mods = NULL;
     CARD32                      format;
     dri3_dmabuf_format_ptr      screen_format = NULL;
@@ -202,11 +204,8 @@ dri3_get_supported_modifiers(ScreenPtr screen, DrawablePtr drawable,
     if (screen_format == NULL)
         return BadMatch;
 
-    if (screen_format->num_modifiers == 0) {
-        *num_screen_modifiers = 0;
-        *num_window_modifiers = 0;
-        return Success;
-    }
+    if (screen_format->num_modifiers == 0)
+        goto done;
 
     if (!info->get_drawable_modifiers ||
         !info->get_drawable_modifiers(drawable, format,
@@ -221,17 +220,13 @@ dri3_get_supported_modifiers(ScreenPtr screen, DrawablePtr drawable,
      */
     screen_mods = malloc(screen_format->num_modifiers * sizeof(CARD64));
     if (!screen_mods)
-        return BadAlloc;
+        goto bad_alloc;
     if (num_drawable_mods > 0) {
         window_mods = malloc(screen_format->num_modifiers * sizeof(CARD64));
-        if (!window_mods) {
-            free(screen_mods);
-            return BadAlloc;
-        }
+        if (!window_mods)
+            goto bad_alloc;
     }
 
-    *num_screen_modifiers = 0;
-    *num_window_modifiers = 0;
     for (i = 0; i < screen_format->num_modifiers; i++) {
         CARD64 modifier = screen_format->modifiers[i];
         Bool window_mod = FALSE;
@@ -245,18 +240,27 @@ dri3_get_supported_modifiers(ScreenPtr screen, DrawablePtr drawable,
 
         if (window_mod) {
             window_mods[*num_window_modifiers] = modifier;
-            *num_window_modifiers += 1;
+            num_window_mods++;
         } else {
             screen_mods[*num_screen_modifiers] = modifier;
-            *num_screen_modifiers += 1;
+            num_screen_mods++;
         }
     }
 
-    assert(*num_window_modifiers + *num_screen_modifiers == screen_format->num_modifiers);
+    assert(num_window_mods + num_screen_mods == screen_format->num_modifiers);
 
-    *window_modifiers = window_mods;
-    *screen_modifiers = screen_mods;
+done:
     free(drawable_mods);
 
+    *num_window_modifiers = num_window_mods;
+    *window_modifiers = window_mods;
+    *num_screen_modifiers = num_screen_mods;
+    *screen_modifiers = screen_mods;
     return Success;
+
+bad_alloc:
+    free(drawable_mods);
+    free(screen_mods);
+    free(window_mods);
+    return BadAlloc;
 }
-- 
2.16.0