xserver: Branch 'master' - 5 commits

Keith Packard keithp at kemper.freedesktop.org
Sun Oct 30 16:58:02 PDT 2011


 dix/devices.c                     |    4 ++++
 hw/xfree86/common/xf86Xinput.c    |   21 ++++++++++++---------
 hw/xfree86/parser/scan.c          |   17 +++++++++++++----
 test/xfree86.c                    |   26 ++++++++++++++++++++++++++
 test/xi2/protocol-common.c        |    1 +
 test/xi2/protocol-eventconvert.c  |    4 +---
 test/xi2/protocol-xiquerydevice.c |    6 +++---
 7 files changed, 60 insertions(+), 19 deletions(-)

New commits:
commit 132545ff576cc69ed63f5a08127151fe550de4c3
Merge: d0c6732... d7c44a7...
Author: Keith Packard <keithp at keithp.com>
Date:   Sun Oct 30 16:57:58 2011 -0700

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

commit d7c44a7c9760449bef263413ad3b20f19b1dc95a
Author: Peter Hutterer <peter.hutterer at who-t.net>
Date:   Mon Oct 24 12:00:32 2011 +1000

    dix: block signals when closing all devices
    
    When closing down all devices, we manually unset master for all attached
    devices, but the device's sprite info still points to the master's sprite
    info. This leaves us a window where the master is freed already but the
    device isn't yet. A signal during that window causes dereference of the
    already freed spriteInfo in mieqEnqueue's EnqueueScreen macro.
    
    Simply block signals when removing all devices. It's not like we're really
    worrying about high-responsive input at this stage.
    
    https://bugzilla.redhat.com/show_bug.cgi?id=737031
    
    Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
    Reviewed-by: Julien Cristau <jcristau at debian.org>

diff --git a/dix/devices.c b/dix/devices.c
index 7c196e0..673a360 100644
--- a/dix/devices.c
+++ b/dix/devices.c
@@ -985,6 +985,8 @@ CloseDownDevices(void)
 {
     DeviceIntPtr dev;
 
+    OsBlockSignals();
+
     /* Float all SDs before closing them. Note that at this point resources
      * (e.g. cursors) have been freed already, so we can't just call
      * AttachDevice(NULL, dev, NULL). Instead, we have to forcibly set master
@@ -1007,6 +1009,8 @@ CloseDownDevices(void)
     inputInfo.keyboard = NULL;
     inputInfo.pointer = NULL;
     XkbDeleteRulesDflts();
+
+    OsReleaseSignals();
 }
 
 /**
commit 820d9040f50a8440741b3aefbc069a3ad81e824e
Author: Servaas Vandenberghe <vdb at picaros.org>
Date:   Wed Aug 31 07:06:49 2011 +0200

    xfree86: fix potential buffer overflow
    
    The patch below fixes a potential buffer overflow in xf86addComment().
    This occurs if  curlen > 0 && eol_seen == 0 && iscomment == 0 , as
    follows from the code:
    
    char *xf86addComment(char *cur, char *add)
    
    <...>
    
            len = strlen(add);
            endnewline = add[len - 1] == '\n';
            len +=  1 + iscomment + (!hasnewline) + (!endnewline) + eol_seen;
    
            if ((str = realloc(cur, len + curlen)) == NULL)
                    return cur;
    
            cur = str;
    
            if (eol_seen || (curlen && !hasnewline))
                    cur[curlen++] = '\n';
            if (!iscomment)
                    cur[curlen++] = '#';
            strcpy(cur + curlen, add);
            if (!endnewline)
                    strcat(cur, "\n");
    
    Signed-off-by: Servaas Vandenberghe <vdb at picaros.org>
    Reviewed-by: Peter Hutterer <peter.hutterer at who-t.net>
    
    [whot: added buffer overflow test case]
    
    Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>

diff --git a/hw/xfree86/parser/scan.c b/hw/xfree86/parser/scan.c
index 1cff3bc..99b3257 100644
--- a/hw/xfree86/parser/scan.c
+++ b/hw/xfree86/parser/scan.c
@@ -1093,7 +1093,7 @@ char *
 xf86addComment(char *cur, char *add)
 {
 	char *str;
-	int len, curlen, iscomment, hasnewline = 0, endnewline;
+	int len, curlen, iscomment, hasnewline = 0, insnewline, endnewline;
 
 	if (add == NULL || add[0] == '\0')
 		return cur;
@@ -1118,14 +1118,23 @@ xf86addComment(char *cur, char *add)
 
 	len = strlen(add);
 	endnewline = add[len - 1] == '\n';
-	len +=  1 + iscomment + (!hasnewline) + (!endnewline) + eol_seen;
 
-	if ((str = realloc(cur, len + curlen)) == NULL)
+	insnewline = eol_seen || (curlen && !hasnewline);
+	if (insnewline)
+		len++;
+	if (!iscomment)
+		len++;
+	if (!endnewline)
+		len++;
+
+	/* Allocate + 1 char for '\0' terminator. */
+	str = realloc(cur, curlen + len + 1);
+	if (!str)
 		return cur;
 
 	cur = str;
 
-	if (eol_seen || (curlen && !hasnewline))
+	if (insnewline)
 		cur[curlen++] = '\n';
 	if (!iscomment)
 		cur[curlen++] = '#';
diff --git a/test/xfree86.c b/test/xfree86.c
index 7012e90..448aa91 100644
--- a/test/xfree86.c
+++ b/test/xfree86.c
@@ -29,6 +29,7 @@
 
 
 #include "xf86.h"
+#include "xf86Parser.h"
 
 static void
 xfree86_option_list_duplicate(void)
@@ -73,9 +74,34 @@ xfree86_option_list_duplicate(void)
     assert(a && b);
 }
 
+static void
+xfree86_add_comment(void)
+{
+    char *current = NULL, *comment;
+    char compare[1024] =  {0};
+
+    comment = "# foo";
+    current =  xf86addComment(current, comment);
+    strcpy(compare, comment);
+    strcat(compare, "\n");
+
+    assert(!strcmp(current, compare));
+
+    /* this used to overflow */
+    strcpy(current, "\n");
+    comment = "foobar\n";
+    current =  xf86addComment(current, comment);
+    strcpy(compare, "\n#");
+    strcat(compare, comment);
+    assert(!strcmp(current, compare));
+
+    free(current);
+}
+
 int main(int argc, char** argv)
 {
     xfree86_option_list_duplicate();
+    xfree86_add_comment();
 
     return 0;
 }
commit 63e87b8639eb8e0b4e32e5d3a09099d31a03bbcd
Author: Peter Hutterer <peter.hutterer at who-t.net>
Date:   Tue Oct 25 11:49:26 2011 +1000

    xfree86: reduce calls to input_option_get_key/value
    
    No functional changes.
    
    Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
    Reviewed-by: Dan Nicholson <dbn.lists at gmail.com>

diff --git a/hw/xfree86/common/xf86Xinput.c b/hw/xfree86/common/xf86Xinput.c
index 425b359..ee705a4 100644
--- a/hw/xfree86/common/xf86Xinput.c
+++ b/hw/xfree86/common/xf86Xinput.c
@@ -910,35 +910,38 @@ NewInputDeviceRequest (InputOption *options, InputAttributes *attrs,
         return BadAlloc;
 
     nt_list_for_each_entry(option, options, list.next) {
-        if (strcasecmp(input_option_get_key(option), "driver") == 0) {
+        const char *key = input_option_get_key(option);
+        const char *value = input_option_get_value(option);
+
+        if (strcasecmp(key, "driver") == 0) {
             if (pInfo->driver) {
                 rval = BadRequest;
                 goto unwind;
             }
-            pInfo->driver = xstrdup(input_option_get_value(option));
+            pInfo->driver = xstrdup(value);
             if (!pInfo->driver) {
                 rval = BadAlloc;
                 goto unwind;
             }
         }
 
-        if (strcasecmp(input_option_get_key(option), "name") == 0 ||
-            strcasecmp(input_option_get_key(option), "identifier") == 0) {
+        if (strcasecmp(key, "name") == 0 ||
+            strcasecmp(key, "identifier") == 0) {
             if (pInfo->name) {
                 rval = BadRequest;
                 goto unwind;
             }
-            pInfo->name = xstrdup(input_option_get_value(option));
+            pInfo->name = xstrdup(value);
             if (!pInfo->name) {
                 rval = BadAlloc;
                 goto unwind;
             }
         }
 
-        if (strcmp(input_option_get_key(option), "_source") == 0 &&
-            (strcmp(input_option_get_value(option), "server/hal") == 0 ||
-             strcmp(input_option_get_value(option), "server/udev") == 0 ||
-             strcmp(input_option_get_value(option), "server/wscons") == 0)) {
+        if (strcmp(key, "_source") == 0 &&
+            (strcmp(value, "server/hal") == 0 ||
+             strcmp(value, "server/udev") == 0 ||
+             strcmp(value, "server/wscons") == 0)) {
             is_auto = 1;
             if (!xf86Info.autoAddDevices) {
                 rval = BadMatch;
commit 005ab41986b0bb6a4e626aee7a7a542247f422e7
Author: Dave Airlie <airlied at redhat.com>
Date:   Thu Oct 27 08:38:45 2011 +1000

    test: fix two more failing FP3232 tests
    
    And put a comment in to explain why we're testing for a frac between .3 and
    .6. We can't directly compare the frac since the floating/fixed point
    conversion loses precision.
    
    Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
    Reviewed-by: Peter Hutterer <peter.hutterer at who-t.net>

diff --git a/test/xi2/protocol-common.c b/test/xi2/protocol-common.c
index 56d6bd2..27edfe5 100644
--- a/test/xi2/protocol-common.c
+++ b/test/xi2/protocol-common.c
@@ -108,6 +108,7 @@ TestPointerProc(DeviceIntPtr pDev, int what)
         pDev->valuator->axisVal[1] = screenInfo.screens[0]->height / 2;
         pDev->last.valuators[1] = pDev->valuator->axisVal[1];
 
+        /* protocol-xiquerydevice.c relies on these increment */
         SetScrollValuator(pDev, 2, SCROLL_TYPE_VERTICAL, 2.4, SCROLL_FLAG_NONE);
         SetScrollValuator(pDev, 3, SCROLL_TYPE_HORIZONTAL, 3.5, SCROLL_FLAG_PREFERRED);
         break;
diff --git a/test/xi2/protocol-eventconvert.c b/test/xi2/protocol-eventconvert.c
index ba2d96a..dce1c50 100644
--- a/test/xi2/protocol-eventconvert.c
+++ b/test/xi2/protocol-eventconvert.c
@@ -389,9 +389,7 @@ static void test_values_XIDeviceEvent(DeviceEvent *in, xXIDeviceEvent *out,
             {
                 FP3232 vi, vo;
 
-                vi.integral = trunc(in->valuators.data[i]);
-                vi.frac = (in->valuators.data[i] - vi.integral) * (1UL << 32);
-
+                vi = double_to_fp3232(in->valuators.data[i]);
                 vo = *values;
 
                 if (swap)
diff --git a/test/xi2/protocol-xiquerydevice.c b/test/xi2/protocol-xiquerydevice.c
index 63d725f..569aea9 100644
--- a/test/xi2/protocol-xiquerydevice.c
+++ b/test/xi2/protocol-xiquerydevice.c
@@ -213,9 +213,9 @@ static void reply_XIQueryDevice_data(ClientPtr client, int len, char *data, void
                             }
 
                             assert(si->increment.integral == si->number);
-                            /* FIXME: frac testing with float/FP issues? */
-                            assert(si->increment.frac > 0.3  * (1UL << 32));
-                            assert(si->increment.frac < 0.6  * (1UL << 32));
+                            /* protocol-common.c sets up increments of 2.4 and 3.5 */
+                            assert(si->increment.frac > 0.3  * (1ULL << 32));
+                            assert(si->increment.frac < 0.6  * (1ULL << 32));
                         }
 
                     }


More information about the xorg-commit mailing list