xserver: Branch 'master' - 7 commits

Keith Packard keithp at kemper.freedesktop.org
Wed Jan 22 11:34:22 PST 2014


 Xi/exevents.c           |   19 ++++++++++++++++++-
 dix/events.c            |    6 +++++-
 dix/grabs.c             |   11 +++++------
 hw/kdrive/ephyr/ephyr.c |   27 ++++++++++++++++++++++++++-
 os/utils.c              |   27 +++++++++++++++++++++++++++
 5 files changed, 81 insertions(+), 9 deletions(-)

New commits:
commit 25ebb9dbc9df659dec2bf6c27654a5bad2d11f94
Merge: 409e8e2 71baa46
Author: Keith Packard <keithp at keithp.com>
Date:   Wed Jan 22 11:33:53 2014 -0800

    Merge remote-tracking branch 'whot/for-keith'

commit 71baa466b1f6b02fe503f9a3089b7b9d61aa0f80
Author: Peter Hutterer <peter.hutterer at who-t.net>
Date:   Mon Jan 13 17:00:23 2014 +1000

    os: restrict display names to digits
    
    We call atoi() on the server's display to get the socket but otherwise use the
    unmodified display for log file name, xkb paths, etc. This results in
    Xorg :banana being the equivalent of Xorg :0, except for the log files being
    in /var/log/Xorg.banana.log. I'm not sure there's a good use-case for this
    behaviour.
    
    Check the display for something that looks reasonable, i.e. digits only, but
    do allow for :0.0 (i.e. digits, followed by a period, followed by one or two
    digits).
    
    Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
    Reviewed-by: Keith Packard <keithp at keithp.com>

diff --git a/os/utils.c b/os/utils.c
index 608ee6a..3b20a5c 100644
--- a/os/utils.c
+++ b/os/utils.c
@@ -600,6 +600,10 @@ UseMsg(void)
 static int
 VerifyDisplayName(const char *d)
 {
+    int i;
+    int period_found = FALSE;
+    int after_period = 0;
+
     if (d == (char *) 0)
         return 0;               /*  null  */
     if (*d == '\0')
@@ -610,6 +614,29 @@ VerifyDisplayName(const char *d)
         return 0;               /*  must not equal "." or ".."  */
     if (strchr(d, '/') != (char *) 0)
         return 0;               /*  very important!!!  */
+
+    /* Since we run atoi() on the display later, only allow
+       for digits, or exception of :0.0 and similar (two decimal points max)
+       */
+    for (i = 0; i < strlen(d); i++) {
+        if (!isdigit(d[i])) {
+            if (d[i] != '.' || period_found)
+                return 0;
+            period_found = TRUE;
+        } else if (period_found)
+            after_period++;
+
+        if (after_period > 2)
+            return 0;
+    }
+
+    /* don't allow for :0. */
+    if (period_found && after_period == 0)
+        return 0;
+
+    if (atol(d) > INT_MAX)
+        return 0;
+
     return 1;
 }
 
commit e81902872176fa9848211fcd7a5eafa4f861a1b7
Author: Peter Hutterer <peter.hutterer at who-t.net>
Date:   Thu Jan 9 15:29:23 2014 +1000

    ephyr: don't allow a shift+ctrl keygrab if mod1 was enabled
    
    Xephyr wants ctrl+shift to grab the window, but that conflicts with
    ctrl+alt+shift key combos. Remember the modifier state on key presses and
    releases, if mod1 is pressed, we need ctrl, shift and mod1 released
    before we allow a shift-ctrl grab activation.
    
    Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>

diff --git a/hw/kdrive/ephyr/ephyr.c b/hw/kdrive/ephyr/ephyr.c
index b2a7985..94b9397 100644
--- a/hw/kdrive/ephyr/ephyr.c
+++ b/hw/kdrive/ephyr/ephyr.c
@@ -1008,6 +1008,29 @@ ephyrProcessButtonRelease(xcb_generic_event_t *xev)
     KdEnqueuePointerEvent(ephyrMouse, mouseState | KD_MOUSE_DELTA, 0, 0, 0);
 }
 
+/* Xephyr wants ctrl+shift to grab the window, but that conflicts with
+   ctrl+alt+shift key combos. Remember the modifier state on key presses and
+   releases, if mod1 is pressed, we need ctrl, shift and mod1 released
+   before we allow a shift-ctrl grab activation.
+
+   note: a key event contains the mask _before_ the current key takes
+   effect, so mod1_was_down will be reset on the first key press after all
+   three were released, not on the last release. That'd require some more
+   effort.
+ */
+static int
+ephyrUpdateGrabModifierState(int state)
+{
+    static int mod1_was_down = 0;
+
+    if ((state & (XCB_MOD_MASK_CONTROL|XCB_MOD_MASK_SHIFT|XCB_MOD_MASK_1)) == 0)
+        mod1_was_down = 0;
+    else if (state & XCB_MOD_MASK_1)
+        mod1_was_down = 1;
+
+    return mod1_was_down;
+}
+
 static void
 ephyrProcessKeyPress(xcb_generic_event_t *xev)
 {
@@ -1018,6 +1041,7 @@ ephyrProcessKeyPress(xcb_generic_event_t *xev)
         return;
     }
 
+    ephyrUpdateGrabModifierState(key->state);
     ephyrUpdateModifierState(key->state);
     KdEnqueueKeyboardEvent(ephyrKbd, key->detail, FALSE);
 }
@@ -1029,6 +1053,7 @@ ephyrProcessKeyRelease(xcb_generic_event_t *xev)
     xcb_key_release_event_t *key = (xcb_key_release_event_t *)xev;
     static xcb_key_symbols_t *keysyms;
     static int grabbed_screen = -1;
+    int mod1_down = ephyrUpdateGrabModifierState(key->state);
 
     if (!keysyms)
         keysyms = xcb_key_symbols_alloc(conn);
@@ -1049,7 +1074,7 @@ ephyrProcessKeyRelease(xcb_generic_event_t *xev)
             hostx_set_win_title(screen,
                                 "(ctrl+shift grabs mouse and keyboard)");
         }
-        else {
+        else if (!mod1_down) {
             /* Attempt grab */
             xcb_grab_keyboard_cookie_t kbgrabc =
                 xcb_grab_keyboard(conn,
commit b2d5ee2e3684951b611fd2068d57cc65fd8305a3
Author: Carlos Garnacho <carlosg at gnome.org>
Date:   Thu Jan 2 21:33:30 2014 +0100

    Xi: Ensure DeviceChanged is emitted after grabs are deactivated
    
    When a grab on a slave device is deactivated, the master device must
    be checked, just in case there were events from other devices while
    the slave device was stolen away by the passive grab. This may
    introduce misbehaviors on mismatching valuators and device features
    later on UpdateDeviceState().
    
    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>

diff --git a/Xi/exevents.c b/Xi/exevents.c
index dff0a92..528e105 100644
--- a/Xi/exevents.c
+++ b/Xi/exevents.c
@@ -1783,8 +1783,25 @@ ProcessDeviceEvent(InternalEvent *ev, DeviceIntPtr device)
         DeliverDeviceEvents(GetSpriteWindow(device), (InternalEvent *) event,
                             NullGrab, NullWindow, device);
 
-    if (deactivateDeviceGrab == TRUE)
+    if (deactivateDeviceGrab == TRUE) {
         (*device->deviceGrab.DeactivateGrab) (device);
+
+        if (!IsMaster (device) && !IsFloating (device)) {
+            int flags, num_events = 0;
+            InternalEvent dce;
+
+            flags = (IsPointerDevice (device)) ?
+                DEVCHANGE_POINTER_EVENT : DEVCHANGE_KEYBOARD_EVENT;
+            UpdateFromMaster (&dce, device, flags, &num_events);
+            BUG_WARN(num_events > 1);
+
+            if (num_events == 1)
+                ChangeMasterDeviceClasses(GetMaster (device, MASTER_ATTACHED),
+                                          &dce.changed_event);
+        }
+
+    }
+
     event->detail.key = key;
 }
 
commit 863d2ad5c02cccde9a4d1a392a7cae78d001c8a9
Author: Alan Coopersmith <alan.coopersmith at oracle.com>
Date:   Mon Jan 6 17:10:40 2014 -0800

    CheckPassiveGrabsOnWindow() needs to handle NULL return value from AllocGrab()
    
    CheckPassiveGrabsOnWindow() calls AllocGrab() which can fail and return NULL.
    This return value is not checked, and can cause NULL pointer dereferences.
    
    Signed-off-by: Alan Coopersmith <alan.coopersmith at oracle.com>
    Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
    Reviewed-by: Peter Hutterer <peter.hutterer at who-t.net>

diff --git a/dix/events.c b/dix/events.c
index 2f0605e..acf97cc 100644
--- a/dix/events.c
+++ b/dix/events.c
@@ -3956,6 +3956,8 @@ CheckPassiveGrabsOnWindow(WindowPtr pWin,
         return NULL;
 
     tempGrab = AllocGrab(NULL);
+    if (tempGrab == NULL)
+        return NULL;
 
     /* Fill out the grab details, but leave the type for later before
      * comparing */
commit 5493a67ec256d22a8a41597a345d8e1c54d6e335
Author: Alan Coopersmith <alan.coopersmith at oracle.com>
Date:   Mon Jan 6 17:10:39 2014 -0800

    GrabDevice() needs to handle NULL return value from AllocGrab()
    
    GrabDevice() calls AllocGrab() which can fail and return NULL.
    This return value is not checked, and can cause NULL pointer dereferences.
    
    Reported-by: Ilja Van Sprundel <ivansprundel at ioactive.com>
    Signed-off-by: Alan Coopersmith <alan.coopersmith at oracle.com>
    Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
    Reviewed-by: Peter Hutterer <peter.hutterer at who-t.net>

diff --git a/dix/events.c b/dix/events.c
index 4aaa54c..2f0605e 100644
--- a/dix/events.c
+++ b/dix/events.c
@@ -5051,7 +5051,7 @@ ProcUngrabPointer(ClientPtr client)
  * @param other_mode GrabModeSync or GrabModeAsync
  * @param status Return code to be returned to the caller.
  *
- * @returns Success or BadValue.
+ * @returns Success or BadValue or BadAlloc.
  */
 int
 GrabDevice(ClientPtr client, DeviceIntPtr dev,
@@ -5132,6 +5132,8 @@ GrabDevice(ClientPtr client, DeviceIntPtr dev,
         GrabPtr tempGrab;
 
         tempGrab = AllocGrab(NULL);
+        if (tempGrab == NULL)
+            return BadAlloc;
 
         tempGrab->next = NULL;
         tempGrab->window = pWin;
commit 3a113815a0cc86d64789494e905da9778174f738
Author: Alan Coopersmith <alan.coopersmith at oracle.com>
Date:   Mon Jan 6 17:10:38 2014 -0800

    If AllocGrab() fails to set up grab, don't copy to a NULL grab
    
    If either the initial calloc or the xi2mask_new fails, grab is NULL,
    but if a src grab is passed in, it was always being written to by
    CopyGrab (and if that failed, dereferenced again in teardown).
    
    Signed-off-by: Alan Coopersmith <alan.coopersmith at oracle.com>
    Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
    Reviewed-by: Peter Hutterer <peter.hutterer at who-t.net>

diff --git a/dix/grabs.c b/dix/grabs.c
index a03897a..7f4c871 100644
--- a/dix/grabs.c
+++ b/dix/grabs.c
@@ -199,12 +199,11 @@ AllocGrab(const GrabPtr src)
             free(grab);
             grab = NULL;
         }
-    }
-
-    if (src && !CopyGrab(grab, src)) {
-        free(grab->xi2mask);
-        free(grab);
-        grab = NULL;
+        else if (src && !CopyGrab(grab, src)) {
+            free(grab->xi2mask);
+            free(grab);
+            grab = NULL;
+        }
     }
 
     return grab;


More information about the xorg-commit mailing list