xserver: Branch 'master'

[GitLab Mirror]

 dix/privates.c |   16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

New commits:
commit f9f705bf3cf0d169d54a70f235cc99e106dbda43
Author: Alex Richardson <Alexander.Richardson at cl.cam.ac.uk>
Date:   Fri Jul 23 09:23:45 2021 +0100

    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.
    
    This also changes the check whether the allocation was moved to use
    uintptr_t instead of a pointer since according to the C standard:
    "The value of a pointer becomes indeterminate when the object it
    points to (or just past) reaches the end of its lifetime." Casting to an
    integer type avoids this undefined behaviour.
    
    Signed-off-by: Alex Richardson <Alexander.Richardson at cl.cam.ac.uk>

diff --git a/dix/privates.c b/dix/privates.c
index 384936fbd..71a72fb22 100644
--- a/dix/privates.c
+++ b/dix/privates.c
@@ -155,14 +155,13 @@ dixMovePrivates(PrivatePtr *privates, int new_offset, unsigned bytes)
 static Bool
 fixupOneScreen(ScreenPtr pScreen, FixupFunc fixup, unsigned bytes)
 {
-    intptr_t        dist;
-    char            *old;
+    uintptr_t       old;
     char            *new;
     DevPrivateKey   *keyp, key;
     DevPrivateType  type;
     int             size;
 
-    old = (char *) pScreen->devPrivates;
+    old = (uintptr_t) pScreen->devPrivates;
     size = global_keys[PRIVATE_SCREEN].offset;
     if (!fixup (&pScreen->devPrivates, size, bytes))
         return FALSE;
@@ -182,9 +181,7 @@ fixupOneScreen(ScreenPtr pScreen, FixupFunc fixup, unsigned bytes)
     if (fixup == dixMovePrivates)
         new += bytes;
 
-    dist = new - old;
-
-    if (dist) {
+    if ((uintptr_t) new != old) {
         for (type = PRIVATE_XSELINUX; type < PRIVATE_LAST; type++)
 
             /* Walk the privates list, being careful as the
@@ -199,10 +196,11 @@ fixupOneScreen(ScreenPtr pScreen, FixupFunc fixup, unsigned bytes)
                  * is contained within the allocation. Privates
                  * stored elsewhere will be left alone
                  */
-                if (old <= (char *) key && (char *) key < old + size)
+                if (old <= (uintptr_t) key && (uintptr_t) 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) (new + ((uintptr_t) key - old));
 
                     /* Patch the list */
                     *keyp = key;