From 005ab41986b0bb6a4e626aee7a7a542247f422e7 Mon Sep 17 00:00:00 2001 From: Dave Airlie Date: Thu, 27 Oct 2011 08:38:45 +1000 Subject: [PATCH 1/4] 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 Reviewed-by: Peter Hutterer --- test/xi2/protocol-common.c | 1 + test/xi2/protocol-eventconvert.c | 4 +--- test/xi2/protocol-xiquerydevice.c | 6 +++--- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/test/xi2/protocol-common.c b/test/xi2/protocol-common.c index 56d6bd268..27edfe516 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 ba2d96ad1..dce1c50c4 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 63d725f28..569aea93a 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)); } } From 63e87b8639eb8e0b4e32e5d3a09099d31a03bbcd Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Tue, 25 Oct 2011 11:49:26 +1000 Subject: [PATCH 2/4] xfree86: reduce calls to input_option_get_key/value No functional changes. Signed-off-by: Peter Hutterer Reviewed-by: Dan Nicholson --- hw/xfree86/common/xf86Xinput.c | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/hw/xfree86/common/xf86Xinput.c b/hw/xfree86/common/xf86Xinput.c index 425b35957..ee705a4a8 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; From 820d9040f50a8440741b3aefbc069a3ad81e824e Mon Sep 17 00:00:00 2001 From: Servaas Vandenberghe Date: Wed, 31 Aug 2011 07:06:49 +0200 Subject: [PATCH 3/4] 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 Reviewed-by: Peter Hutterer [whot: added buffer overflow test case] Signed-off-by: Peter Hutterer --- hw/xfree86/parser/scan.c | 17 +++++++++++++---- test/xfree86.c | 26 ++++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 4 deletions(-) diff --git a/hw/xfree86/parser/scan.c b/hw/xfree86/parser/scan.c index 1cff3bc5c..99b325717 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 7012e90c3..448aa915e 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; } From d7c44a7c9760449bef263413ad3b20f19b1dc95a Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Mon, 24 Oct 2011 12:00:32 +1000 Subject: [PATCH 4/4] 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 Reviewed-by: Julien Cristau --- dix/devices.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/dix/devices.c b/dix/devices.c index 7c196e077..673a360fb 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(); } /**