frederik
/
xserver
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

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@rocketsoftware.com>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1647>
This commit is contained in:
Nathan Kidd 2024-08-12 18:05:58 -04:00 committed by Enrico Weigelt
parent 5b810bac5e
commit 7ca8b37ab1
1 changed files with 8 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

22
glx/indirect_util.c
View File

@ -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) {