Commit 22376023 authored by Alexander Richardson's avatar Alexander Richardson
Browse files

dix/privates.c: Avoid undefined behaviour after realloc()



Adding the offset between the realloc result and the old allocation to
update pointers into the new allocation is undefined behaviour: the
old pointers are no longer valid after realloc() according to the C
standard. While this works on almost all architectures and compilers,
it causes  problems on architectures that track pointer bounds (e.g.
CHERI or Arm's Morello): the DevPrivateKey pointers will still have the
bounds of the previous allocation and therefore any dereference will
result in a run-time trap.

I found this due to a crash (dereferencing an invalid capability) while
trying to run `XVnc` on a CHERI-RISC-V system. With this commit I can
successfully connect to the XVnc instance running inside a QEMU with a
VNC viewer on my host.
Signed-off-by: Alexander Richardson's avatarAlex Richardson <Alexander.Richardson@cl.cam.ac.uk>
parent f6f2f203
Pipeline #365735 passed with stages
in 4 minutes and 29 seconds
......@@ -155,7 +155,6 @@ dixMovePrivates(PrivatePtr *privates, int new_offset, unsigned bytes)
static Bool
fixupOneScreen(ScreenPtr pScreen, FixupFunc fixup, unsigned bytes)
{
intptr_t dist;
char *old;
char *new;
DevPrivateKey *keyp, key;
......@@ -182,9 +181,7 @@ fixupOneScreen(ScreenPtr pScreen, FixupFunc fixup, unsigned bytes)
if (fixup == dixMovePrivates)
new += bytes;
dist = new - old;
if (dist) {
if (new != old) {
for (type = PRIVATE_XSELINUX; type < PRIVATE_LAST; type++)
/* Walk the privates list, being careful as the
......@@ -201,8 +198,9 @@ fixupOneScreen(ScreenPtr pScreen, FixupFunc fixup, unsigned bytes)
*/
if (old <= (char *) key && (char *) key < old + size)
{
/* Compute new location of key */
key = (DevPrivateKey) ((char *) key + dist);
/* Compute new location of key (deriving from the new
* allocation to avoid UB) */
key = (DevPrivateKey) ((char *) new + ((char *) key - old));
/* Patch the list */
*keyp = key;
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment