xserver: Branch 'master' - 2 commits

[GitLab Mirror]

 glx/indirect_util.c |   22 ++++++++--------------
 glx/singlesize.c    |    4 ++--
 2 files changed, 10 insertions(+), 16 deletions(-)

New commits:
commit a4df68688871e2ba4cd268633d220bd27adb8374
Author: Nathan Kidd <nkidd at rocketsoftware.com>
Date:   Mon Aug 12 18:05:58 2024 -0400

    glx: Don't blindly write 8 bytes in GLX single replies
    
    Previously we leaked stack when invalid enum parameters were
    specified and caused __glGet*_size functions to return a 0 size.
    
    Further, we read out-of-bounds (and leaked) when the input data was less
    than 8 bytes (__glXDispSwap_GetFramebufferAttachmentParameteriv and
    __glXDisp_GetRenderbufferParameteriv).
    
    Now we only write a single element in the reply padding, and only when there
    is a single element. This is what the Mesa client-side libGL expects, and
    restores original GLX server behaviour, matching both pre-public (1996) SGI GLX
    and XFree86 4.
    
    The main risk of this change is if we have any error in element count or size;
    previously it may not have mattered but now it does.
    
    There are no piglit result changes from this modification using either mesa
    libGLX or NVIDIA libGLX.
    
    For performance considerations, an extra conditional and variable-length
    memcpy has no meaningful impact on the indirect rendering pipeline cost.
    
    There is still the possiblity to leak if our size checks allow an enum that
    the GL implemention does not. Guarding against that requires zero-initializing
    all temp storage, which wants re-evaluation of the blind 200-byte buffers
    used for many calls and thus is a much bigger change.
    
    Signed-off-by: Nathan Kidd <nkidd at rocketsoftware.com>
    Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1647>

diff --git a/glx/indirect_util.c b/glx/indirect_util.c
index 83579c529..140b3b21f 100644
--- a/glx/indirect_util.c
+++ b/glx/indirect_util.c
@@ -128,13 +128,10 @@ __glXSendReply(ClientPtr client, const void *data, size_t elements,
         .retval = retval,
     };
 
-    /* It is faster on almost always every architecture to just copy the 8
-     * bytes, even when not necessary, than check to see of the value of
-     * elements requires it.  Copying the data when not needed will do no
-     * harm.
-     */
-
-    (void) memcpy(&reply.pad3, data, 8);
+    /* Single element goes in reply padding; don't leak uninitialized data. */
+    if (elements == 1) {
+        (void) memcpy(&reply.pad3, data, element_size);
+    }
     WriteToClient(client, sizeof(xGLXSingleReply), &reply);
 
     if (reply_ints != 0) {
@@ -176,13 +173,10 @@ __glXSendReplySwap(ClientPtr client, const void *data, size_t elements,
         .retval = bswap_32(retval),
     };
 
-    /* It is faster on almost always every architecture to just copy the 8
-     * bytes, even when not necessary, than check to see of the value of
-     * elements requires it.  Copying the data when not needed will do no
-     * harm.
-     */
-
-    (void) memcpy(&reply.pad3, data, 8);
+    /* Single element goes in reply padding; don't leak uninitialized data. */
+    if (elements == 1) {
+        (void) memcpy(&reply.pad3, data, element_size);
+    }
     WriteToClient(client, sizeof(xGLXSingleReply), &reply);
 
     if (reply_ints != 0) {
commit 29cfcf52599c9984cc7352ce4116337f15d7f20f
Author: Nathan Kidd <nkidd at rocketsoftware.com>
Date:   Wed Aug 7 18:17:32 2024 -0400

    glx: Fix out-of-bounds reads from negative return
    
    The callers of these functions were casting -1 to unsigned and then
    using 4GB indexes. By returning 0 we match all the other size functions.
    
    GLX size functions return -1 to indicate error, but GL size functions return 0.
    
    Signed-off-by: Nathan Kidd <nkidd at rocketsoftware.com>
    Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1647>

diff --git a/glx/singlesize.c b/glx/singlesize.c
index 344ffef7f..d2db6945a 100644
--- a/glx/singlesize.c
+++ b/glx/singlesize.c
@@ -105,7 +105,7 @@ __glGetMap_size(GLenum target, GLenum query)
         }
         break;
     }
-    return -1;
+    return 0;
 }
 
 GLint
@@ -164,7 +164,7 @@ __glGetPixelMap_size(GLenum map)
         query = GL_PIXEL_MAP_A_TO_A_SIZE;
         break;
     default:
-        return -1;
+        return 0;
     }
     glGetIntegerv(query, &size);
     return size;