xserver: Branch 'server-1.10-branch' - 7 commits

Jeremy Huddleston jeremyhu at kemper.freedesktop.org
Tue Oct 18 09:25:39 PDT 2011


 Xext/shm.c                  |    6 +-----
 Xi/xipassivegrab.c          |   13 ++++++++++---
 dix/dispatch.c              |    1 -
 dix/events.c                |   14 +++++++++++++-
 fb/fbblt.c                  |    9 +++++++--
 hw/xquartz/X11Application.m |    2 +-
 os/utils.c                  |    4 ++--
 7 files changed, 34 insertions(+), 15 deletions(-)

New commits:
commit e4bf3807ae2fa8b4491f0ed336bc42541faf2a55
Author: Matthieu Herrb <matthieu.herrb at laas.fr>
Date:   Mon Oct 17 22:27:35 2011 +0200

    Fix CVE-2011-4029: File permission change vulnerability.
    
    Use fchmod() to change permissions of the lock file instead
    of chmod(), thus avoid the race that can be exploited to set
    a symbolic link to any file or directory in the system.
    
    Signed-off-by: Matthieu Herrb <matthieu.herrb at laas.fr>
    Reviewed-by: Alan Coopersmith <alan.coopersmith at oracle.com>
    (cherry picked from commit b67581cf825940fdf52bf2e0af4330e695d724a4)

diff --git a/os/utils.c b/os/utils.c
index fad72b4..da23589 100644
--- a/os/utils.c
+++ b/os/utils.c
@@ -306,7 +306,7 @@ LockServer(void)
     FatalError("Could not create lock file in %s\n", tmp);
   (void) sprintf(pid_str, "%10ld\n", (long)getpid());
   (void) write(lfd, pid_str, 11);
-  (void) chmod(tmp, 0444);
+  (void) fchmod(lfd, 0444);
   (void) close(lfd);
 
   /*
commit e72b6653628f7aa7bf59c2d42365e967fce685be
Author: Matthieu Herrb <matthieu.herrb at laas.fr>
Date:   Mon Oct 17 22:26:12 2011 +0200

    Fix CVE-2011-4028: File disclosure vulnerability.
    
    use O_NOFOLLOW to open the existing lock file, so symbolic links
    aren't followed, thus avoid revealing if it point to an existing
    file.
    
    Signed-off-by: Matthieu Herrb <matthieu.herrb at laas.fr>
    Reviewed-by: Alan Coopersmith <alan.coopersmith at oracle.com>
    (cherry picked from commit 6ba44b91e37622ef8c146d8f2ac92d708a18ed34)

diff --git a/os/utils.c b/os/utils.c
index fc72787..fad72b4 100644
--- a/os/utils.c
+++ b/os/utils.c
@@ -327,7 +327,7 @@ LockServer(void)
       /*
        * Read the pid from the existing file
        */
-      lfd = open(LockFile, O_RDONLY);
+      lfd = open(LockFile, O_RDONLY|O_NOFOLLOW);
       if (lfd < 0) {
         unlink(tmp);
         FatalError("Can't read lock file %s\n", LockFile);
commit b7b27e9cc1c948bdf4eace5fee8db3667eb4a8d5
Author: Sam Spilsbury <sam.spilsbury at canonical.com>
Date:   Wed Sep 14 09:58:34 2011 +0800

    Remove the SendEvent bit (0x80) before doing range checks on event type.
    
    Some extension libraries may set this bit before converting the event to
    wire protocol and as such range checking the event will cause an invalid
    BadValue error to result. As the documentation suggests the the bit
    should be "forced on", remove it before doing range checks and continue
    to force it on in the server.
    
    Reviewed-by: Jamey Sharp <jamey at minilop.net>
    Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
    (cherry picked from commit 2d2dce558d24eeea0eb011ec9ebaa6c5c2273c39)
    (cherry picked from commit e9ae33316012ffe9acfeeb7303ab3392c2ca2a2b)

diff --git a/dix/events.c b/dix/events.c
index eefcdef..effbb22 100644
--- a/dix/events.c
+++ b/dix/events.c
@@ -4981,6 +4981,8 @@ CloseDownEvents(void)
     InputEventList = NULL;
 }
 
+#define SEND_EVENT_BIT 0x80
+
 /**
  * Server-side protocol handling for SendEvent request.
  *
@@ -4998,6 +5000,16 @@ ProcSendEvent(ClientPtr client)
 
     REQUEST_SIZE_MATCH(xSendEventReq);
 
+    /* libXext and other extension libraries may set the bit indicating
+     * that this event came from a SendEvent request so remove it
+     * since otherwise the event type may fail the range checks
+     * and cause an invalid BadValue error to be returned.
+     *
+     * This is safe to do since we later add the SendEvent bit (0x80)
+     * back in once we send the event to the client */
+
+    stuff->event.u.u.type &= ~(SEND_EVENT_BIT);
+
     /* The client's event type must be a core event type or one defined by an
 	extension. */
 
@@ -5055,7 +5067,7 @@ ProcSendEvent(ClientPtr client)
 	client->errorValue = stuff->propagate;
 	return BadValue;
     }
-    stuff->event.u.u.type |= 0x80;
+    stuff->event.u.u.type |= SEND_EVENT_BIT;
     if (stuff->propagate)
     {
 	for (;pWin; pWin = pWin->parent)
commit 121a4f23d27a95948ce79b18be4b557db149a9ac
Author: Carlos Garnacho <carlosg at gnome.org>
Date:   Wed Aug 31 00:46:52 2011 +0200

    Xi: Fix passive XI2 ungrabs on XIAll[Master]Devices
    
    The corresponding DeviceIntPtr wasn't being gotten properly,
    resulting in BadDevice from dixLookupDevice().
    
    Signed-off-by: Carlos Garnacho <carlosg at gnome.org>
    Reviewed-by: Peter Hutterer <peter.hutterer at who-t.net>
    Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
    (cherry picked from commit f52d5cd374563544dafe29587411f345e31bbdf8)
    (cherry picked from commit 347f5610ca023fb31485aa19c20607af8bf9c834)

diff --git a/Xi/xipassivegrab.c b/Xi/xipassivegrab.c
index 776d334..762bedb 100644
--- a/Xi/xipassivegrab.c
+++ b/Xi/xipassivegrab.c
@@ -264,9 +264,16 @@ ProcXIPassiveUngrabDevice(ClientPtr client)
     REQUEST(xXIPassiveUngrabDeviceReq);
     REQUEST_AT_LEAST_SIZE(xXIPassiveUngrabDeviceReq);
 
-    rc = dixLookupDevice(&dev, stuff->deviceid, client, DixGrabAccess);
-    if (rc != Success)
-	return rc;
+    if (stuff->deviceid == XIAllDevices)
+        dev = inputInfo.all_devices;
+    else if (stuff->deviceid == XIAllMasterDevices)
+        dev = inputInfo.all_master_devices;
+    else
+    {
+        rc = dixLookupDevice(&dev, stuff->deviceid, client, DixGrabAccess);
+        if (rc != Success)
+	    return rc;
+    }
 
     if (stuff->grab_type != XIGrabtypeButton &&
         stuff->grab_type != XIGrabtypeKeycode &&
commit d8faf80bae3becf21f6bcb1bf5b17e4ab98e1686
Author: Jeremy Huddleston <jeremyhu at apple.com>
Date:   Tue Sep 20 20:39:06 2011 -0700

    XQuartz: Use set_front_process rather than X11ApplicationSetFrontProcess since we're already in the AppKit thread
    
    Signed-off-by: Jeremy Huddleston <jeremyhu at apple.com>
    (cherry picked from commit 38e9e28ba2fbffee52ad9889ef6d4e94c7af3e10)
    (cherry picked from commit 7b74bb67528033bf2d2a1714a801c38e7822642e)

diff --git a/hw/xquartz/X11Application.m b/hw/xquartz/X11Application.m
index 6e9de4b..b64ca1d 100644
--- a/hw/xquartz/X11Application.m
+++ b/hw/xquartz/X11Application.m
@@ -352,7 +352,7 @@ static void message_kit_thread (SEL selector, NSObject *arg) {
                          */
                         _appFlags._active = YES;
 
-                        X11ApplicationSetFrontProcess();
+                        [self set_front_process:nil];
 
                         /* Get the Spaces preference for SwitchOnActivate */
                         (void)CFPreferencesAppSynchronize(CFSTR("com.apple.dock"));
commit c187638e363dca2fa0b2026473b15b01adeef1c1
Author: Jamey Sharp <jamey at minilop.net>
Date:   Tue Sep 14 18:35:21 2010 -0700

    Fix pixmap double-frees on error paths.
    
    If AddResource fails, it will automatically free the object that was
    passed to it by calling the appropriate deleteFunc; and of course
    FreeResource also calls the deleteFunc. In both cases it's wrong to call
    the destroy hook manually.
    
    Commit by Jamey Sharp and Josh Triplett.
    
    Signed-off-by: Jamey Sharp <jamey at minilop.net>
    Signed-off-by: Josh Triplett <josh at joshtriplett.org>
    Reviewed-by: Peter Hutterer <peter.hutterer at who-t.net>
    Reviewed-by: Aaron Plattner <aplattner at nvidia.com>
    (cherry picked from commit 0f380a5005f800572773cd4667ce43c7459cc467)
    (cherry picked from commit bd6ea85209e5ab80375d4ec9994d10a89fd1374a)

diff --git a/Xext/shm.c b/Xext/shm.c
index 23afe6b..d9469ea 100644
--- a/Xext/shm.c
+++ b/Xext/shm.c
@@ -993,7 +993,6 @@ CreatePmap:
 	    pMap->drawable.serialNumber = NEXT_SERIAL_NUMBER;
 	    pMap->drawable.id = newPix->info[j].id;
 	    if (!AddResource(newPix->info[j].id, RT_PIXMAP, (pointer)pMap)) {
-		(*pScreen->DestroyPixmap)(pMap);
 		result = BadAlloc;
 		break;
 	    }
@@ -1004,10 +1003,8 @@ CreatePmap:
     }
 
     if(result == BadAlloc) {
-	while(j--) {
-	    (*pScreen->DestroyPixmap)(pMap);
+	while(j--)
 	    FreeResource(newPix->info[j].id, RT_NONE);
-	}
 	free(newPix);
     } else 
 	AddResource(stuff->pid, XRT_PIXMAP, newPix);
@@ -1112,7 +1109,6 @@ CreatePmap:
 	{
 	    return Success;
 	}
-	pDraw->pScreen->DestroyPixmap(pMap);
     }
     return BadAlloc;
 }
diff --git a/dix/dispatch.c b/dix/dispatch.c
index 7b2132d..d3b280a 100644
--- a/dix/dispatch.c
+++ b/dix/dispatch.c
@@ -1418,7 +1418,6 @@ CreatePmap:
 	}
 	if (AddResource(stuff->pid, RT_PIXMAP, (pointer)pMap))
 	    return Success;
-	(*pDraw->pScreen->DestroyPixmap)(pMap);
     }
     return BadAlloc;
 }
commit ef42bc7f67aba60f59dd9d047fb00df2943e3029
Author: Adam Jackson <ajax at redhat.com>
Date:   Thu Apr 21 16:37:11 2011 -0400

    fb: Fix memcpy abuse
    
    The memcpy fast path implicitly assumes that the copy walks
    left-to-right.  That's not something memcpy guarantees, and newer glibc
    on some processors will indeed break that assumption.  Since we walk a
    line at a time, check the source and destination against the width of
    the blit to determine whether we can be sloppy enough to allow memcpy.
    (Having done this, we can remove the check for !reverse as well.)
    
    On an Intel Core i7-2630QM with an NVIDIA GeForce GTX 460M running in
    NoAccel, the broken code and various fixes for -copywinwin{10,100,500}
    gives (edited to fit in 80 columns):
    
    1: Disable the fastpath entirely
    2: Replace memcpy with memmove
    3: This fix
    4: The code before this fix
    
      1            2                 3                 4           Operation
    ------   ---------------   ---------------   ---------------   ------------
    258000   269000 (  1.04)   544000 (  2.11)   552000 (  2.14)   Copy 10x10
     21300    23000 (  1.08)    43700 (  2.05)    47100 (  2.21)   Copy 100x100
       960      962 (  1.00)     1990 (  2.09)     1990 (  2.07)   Copy 500x500
    
    So it's a modest performance hit, but correctness demands it, and it's
    probably worth keeping the 2x speedup from having the fast path in the
    first place.
    
    Signed-off-by: Adam Jackson <ajax at redhat.com>
    Signed-off-by: Keith Packard <keithp at keithp.com>
    (cherry picked from commit e32cc0b4c85c78cd8743a6e1680dcc79054b57ce)

diff --git a/fb/fbblt.c b/fb/fbblt.c
index 38271c0..23ba3ae 100644
--- a/fb/fbblt.c
+++ b/fb/fbblt.c
@@ -65,6 +65,7 @@ fbBlt (FbBits   *srcLine,
     int	    n, nmiddle;
     Bool    destInvarient;
     int	    startbyte, endbyte;
+    int     careful;
     FbDeclareMergeRop ();
 
 #ifdef FB_24BIT
@@ -76,12 +77,16 @@ fbBlt (FbBits   *srcLine,
     }
 #endif
 
-    if (alu == GXcopy && pm == FB_ALLONES && !reverse &&
+    careful = !((srcLine < dstLine && srcLine + width * (bpp>>3) > dstLine) ||
+                (dstLine < srcLine && dstLine + width * (bpp>>3) > srcLine)) ||
+              (bpp & 7);
+
+    if (alu == GXcopy && pm == FB_ALLONES && !careful &&
             !(srcX & 7) && !(dstX & 7) && !(width & 7)) {
         int i;
         CARD8 *src = (CARD8 *) srcLine;
         CARD8 *dst = (CARD8 *) dstLine;
-        
+
         srcStride *= sizeof(FbBits);
         dstStride *= sizeof(FbBits);
         width >>= 3;


More information about the xorg-commit mailing list