From 8ad49a03ebebffbdbf21c4ef9775b7e5847496d9 Mon Sep 17 00:00:00 2001 From: Julien Cristau Date: Sat, 7 Jan 2017 16:20:31 +0100 Subject: Fix wrong Xfree in XListFonts failure path 'ch' gets moved inside the allocated buffer as we're looping through fonts, so keep a reference to the start of the buffer so we can pass that to Xfree in the failure case. Fixes: commit 20a3f99eba5001925b8b313da3accb7900eb1927 "Plug a memory leak" Signed-off-by: Julien Cristau Reviewed-by: Alan Coopersmith Signed-off-by: Peter Hutterer --- nx-X11/lib/X11/FontNames.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/nx-X11/lib/X11/FontNames.c b/nx-X11/lib/X11/FontNames.c index 3e23b5f49..9ffdfd299 100644 --- a/nx-X11/lib/X11/FontNames.c +++ b/nx-X11/lib/X11/FontNames.c @@ -43,6 +43,7 @@ int *actualCount) /* RETURN */ register int length; char **flist = NULL; char *ch = NULL; + char *chstart; char *chend; int count = 0; xListFontsReply rep; @@ -86,6 +87,7 @@ int *actualCount) /* RETURN */ /* * unpack into null terminated strings. */ + chstart = ch; chend = ch + (rlen + 1); length = *(unsigned char *)ch; *ch = 1; /* make sure it is non-zero for XFreeFontNames */ @@ -98,14 +100,14 @@ int *actualCount) /* RETURN */ *ch = '\0'; /* and replace with null-termination */ count++; } else { - Xfree(ch); + Xfree(chstart); Xfree(flist); flist = NULL; count = 0; break; } } else { - Xfree(ch); + Xfree(chstart); Xfree(flist); flist = NULL; count = 0; -- cgit v1.2.3 From 00405b278471b4b5aa65003eb7ae97ac17d56445 Mon Sep 17 00:00:00 2001 From: Arthur Huillet Date: Wed, 1 Feb 2017 15:02:41 +0100 Subject: _XDefaultError: set XlibDisplayIOError flag before calling exit _XReply isn't reentrant, and it can lead to deadlocks when the default error handler is called: _XDefaultError calls exit(1). It is called indirectly by _XReply when a X protocol error comes in that isn't filtered/handled by an extension or the application. This means that if the application (or one of its loaded shared libraries such as the NVIDIA OpenGL driver) has registered any _fini destructor, _fini will get called while still on the call stack of _XReply. If the destructor interacts with the X server and calls _XReply, it will hit a deadlock, looping on the following in _XReply: ConditionWait(dpy, dpy->xcb->reply_notify); It is legal for an application to make Xlib calls during _fini, and that is useful for an OpenGL driver to avoid resource leaks on the X server side, for example in the dlopen/dlclose case. However, the driver can not readily tell whether its _fini is being called because Xlib called exit, or for another reason (dlclose), so it is hard to cleanly work around this issue in the driver. This change makes it so _XReply effectively becomes a no-op when called after _XDefaultError was called, as though an XIOError had happened. The dpy connection isn't broken at that point, but any call to _XReply is going to hang. This is a bit of a kludge, because the more correct solution would be to make _XReply reentrant, maybe by broadcasting the reply_notify condition before calling the default error handler. However, such a change would carry a grater risk of introducing regressions in Xlib. This change will drop some valid requests on the floor, but this should not matter, as it will only do so in the case where the application is dying: X will clean up after it once exit() is done running. There is the case of XSetCloseDownMode(RETAIN_PERMANENT), but an application using that and wishing to clean up resources in _fini would currently be hitting a deadlock, which is hardly a better situation. Signed-off-by: Aaron Plattner Reviewed-by: Jamey Sharp --- nx-X11/lib/X11/XlibInt.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/nx-X11/lib/X11/XlibInt.c b/nx-X11/lib/X11/XlibInt.c index 912c52989..5afc65149 100644 --- a/nx-X11/lib/X11/XlibInt.c +++ b/nx-X11/lib/X11/XlibInt.c @@ -3604,6 +3604,16 @@ int _XDefaultError( XErrorEvent *event) { if (_XPrintDefaultError (dpy, event, stderr) == 0) return 0; + + /* + * Store in dpy flags that the client is exiting on an unhandled XError + * (pretend it is an IOError, since the application is dying anyway it + * does not make a difference). + * This is useful for _XReply not to hang if the application makes Xlib + * calls in _fini as part of process termination. + */ + dpy->flags |= XlibDisplayIOError; + exit(1); /*NOTREACHED*/ } -- cgit v1.2.3