An XCB test application will always crash because of heap corruption if it's running xcb_connect/xcb_disconnect continuously from multiple threads. The problem can also happen in real applications if XOpenDisplay and xcb_connect are called simultaneously.
This commit fixes only the heap corruption and sporadic crashes. It's still possible that XauFileName returns a badly formed filename string if called from multiple threads. For example, changing contents of HOME environment variable could make the returned string to be malformed. However, there shouldn't be crashes. Signed-off-by: Rami Ylimäki <[email protected]> Signed-off-by: Erkki Seppälä <[email protected]> --- AuFileName.c | 34 +++++++++++++++++++++------------- 1 files changed, 21 insertions(+), 13 deletions(-) diff --git a/AuFileName.c b/AuFileName.c index b21b048..fdf03e0 100644 --- a/AuFileName.c +++ b/AuFileName.c @@ -33,14 +33,14 @@ in this Software without prior written authorization from The Open Group. #include <X11/Xauth.h> #include <X11/Xos.h> #include <stdlib.h> +#include <limits.h> char * XauFileName (void) { const char *slashDotXauthority = "/.Xauthority"; char *name; - static char *buf; - static int bsize; + static char buf[PATH_MAX] = {0}; #ifdef WIN32 char dir[128]; #endif @@ -48,6 +48,9 @@ XauFileName (void) if ((name = getenv ("XAUTHORITY"))) return name; + /* This function assumes that the HOME environment variable + * doesn't change between multiple threads. If it does change, the + * returned string may not contain a valid file name. */ name = getenv ("HOME"); if (!name) { #ifdef WIN32 @@ -60,16 +63,21 @@ XauFileName (void) #endif return NULL; } - size = strlen (name) + strlen(&slashDotXauthority[1]) + 2; - if (size > bsize) { - if (buf) - free (buf); - buf = malloc ((unsigned) size); - if (!buf) - return NULL; - bsize = size; - } - strcpy (buf, name); - strcat (buf, slashDotXauthority + (name[1] == '\0' ? 1 : 0)); + /* Length of home directory location. */ + size = strlen (name); + /* Check whether "/home/user" + "/.Xauthority" + "\0" fits into + * "buf". */ + if (size + strlen (slashDotXauthority) + 1 > sizeof(buf)) + return NULL; + /* Construct "/home/user/.Xauthority". Avoid writing null + * character when the first part of the string is copied. That + * could temporarily split the contents of "buf" and cause a + * problem with multiple threads. We are assuming here that the + * contents of "name" stay the same if this function is called + * simultaneously from multiple threads. + */ + memcpy (buf, name, size); + strcpy (buf + size, slashDotXauthority + ((size <= 1) ? 1 : 0)); + return buf; } -- 1.6.3.3 _______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
