glx: Fix SEGV due to dereferencing a NULL ptr from XCB-GLX.
authorHal Gentz <zegentzy@protonmail.com>
Thu, 25 Jul 2019 21:40:50 +0000 (15:40 -0600)
committerAdam Jackson <ajax@nwnk.net>
Wed, 4 Sep 2019 16:00:10 +0000 (16:00 +0000)
When run in optirun, applications that linked to `libGLX.so` and then
proceeded to querying Mesa for extension strings caused a SEGV in Mesa.

`glXQueryExtensionsString` was calling a chain of functions that
eventually led to `__glXQueryServerString`. This function would call
`xcb_glx_query_server_string` then `xcb_glx_query_server_string_reply`.
The latter for some unknown reason returned `NULL`. Passing this `NULL`
to `xcb_glx_query_server_string_string_length` would cause a SEGV as the
function tried to dereference it.

The reason behind the function returning `NULL` is yet to be determined,
however, simply checking that the ptr is not `NULL` resolves this. A
similar check has been added to `__glXGetString` for completeness sake,
although not immediately necessary.

In addition to that, we stumbled into a similar problem in
`AllocAndFetchScreenConfigs` which tries to access the configs to free
them if `__glXQueryServerString` fails. This, of course, SEGVs, because the
configs are yet to have been allocated. Simply continuing past the configs
if their config ptrs are `NULL` resolves this. We also switch to `calloc`
to make sure that the config ptrs are `NULL` by default, and not some
uninitialized value.

Cc: mesa-stable@lists.freedesktop.org
Fixes: 24b8a8cfe821 "glx: implement __glXGetString, hide __glXGetStringFromServer"
Fixes: cb3610e37c4c "Import the GLX client side library, formerly from xc/lib/GL/glx. Build it "
Reviewed-by: Adam Jackson <ajax@redhat.com>
Signed-off-by: Hal Gentz <zegentzy@protonmail.com>
src/glx/glx_query.c
src/glx/glxext.c

index 7064e7707bfd0b91c53c6249d52a416d553fbc30..14e6f7c1c487966c2d7c17f9c55ab3e2d93ff8e5 100644 (file)
@@ -50,6 +50,9 @@ __glXQueryServerString(Display * dpy, int opcode, CARD32 screen, CARD32 name)
                                                                     name),
                                         NULL);
 
+   if (!reply)
+      return NULL;
+
    /* The spec doesn't mention this, but the Xorg server replies with
     * a string already terminated with '\0'. */
    uint32_t len = xcb_glx_query_server_string_string_length(reply);
@@ -74,6 +77,9 @@ __glXGetString(Display * dpy, int opcode, CARD32 contextTag, CARD32 name)
                                                                  name),
                                                                 NULL);
 
+   if (!reply)
+      return NULL;
+
    /* The spec doesn't mention this, but the Xorg server replies with
     * a string already terminated with '\0'. */
    uint32_t len = xcb_glx_get_string_string_length(reply);
index 459b6c2b07e7e737676b4d0790c0e4d07b20e7dc..6e6525aeeae2e032b0808e701589bcffe879a6d8 100644 (file)
@@ -214,6 +214,8 @@ FreeScreenConfigs(struct glx_display * priv)
    screens = ScreenCount(priv->dpy);
    for (i = 0; i < screens; i++) {
       psc = priv->screens[i];
+      if (!psc)
+         continue;
       glx_screen_cleanup(psc);
 
 #if defined(GLX_DIRECT_RENDERING) && !defined(GLX_USE_APPLEGL)
@@ -792,7 +794,7 @@ AllocAndFetchScreenConfigs(Display * dpy, struct glx_display * priv)
     ** First allocate memory for the array of per screen configs.
     */
    screens = ScreenCount(dpy);
-   priv->screens = malloc(screens * sizeof *priv->screens);
+   priv->screens = calloc(screens, sizeof *priv->screens);
    if (!priv->screens)
       return GL_FALSE;