Re: [PATCH libXi] SizeClassInfo can return 0 even without an error
Hi Niels,
On Friday, 7 October 2016, Niels Ole Salscheider <
niels_...@salscheider-online.de> wrote:
> Catch the error case separately. This fixes a few crashes on my computer.
>
> Signed-off-by: Niels Ole Salscheider >
> ---
> src/XListDev.c | 21 ++---
> 1 file changed, 10 insertions(+), 11 deletions(-)
>
> diff --git a/src/XListDev.c b/src/XListDev.c
> index f850cd0..d0c6bf2 100644
> --- a/src/XListDev.c
> +++ b/src/XListDev.c
> @@ -73,27 +73,27 @@ static int pad_to_xid(int base_size)
> return ((base_size + padsize - 1)/padsize) * padsize;
> }
>
> -static size_t
> -SizeClassInfo(xAnyClassPtr *any, size_t len, int num_classes)
> +static int
> +SizeClassInfo(xAnyClassPtr *any, size_t len, int num_classes, size_t
> *size)
> {
> -int size = 0;
> int j;
> +*size = 0;
No function should alter the contents of the arguments in case of an error.
For your other libXi patch one might want to fix the callers, if applicable.
If possible please mention a bug report/link or a bit more about how you
hit this. Wondering how it has gone unnoticed for so long.
That aside, nicely spotted !
Emil
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH 3/3] configure.ac: remove --enable-aiglx option
On Friday, 7 October 2016, Jon Turney wrote: > On 06/10/2016 19:15, Emil Velikov wrote: > >> On 6 October 2016 at 19:02, Jon Turney >> wrote: >> >>> On 29/09/2016 18:41, Emil Velikov wrote: >>> Presently the option guards both direct and accelerated indirect GLX. As such when one toggles it off they end up without any acceleration. Remove the option all together until we have the time to split/rework things. Cc: Jon Turney Cc: Adam Jackson Signed-off-by: Emil Velikov --- Jon, I've not checked the Xwin side of things but considering that the option is enabled by default and having it for Xwin only will be confusing I've nuked the guards throughout the tree. >>> >>> Sorry I didn't get around to testing this before it was committed. >>> >>> This breaks my build (See [1]), as the DRI2 loader is now built >>> unconditionally, which fails without drm.h >>> >>> I'm not sure exactly what problem this change is fixing, so I'm not sure >>> how >>> to address that. >>> >>> Is it ok to restore the part which makes building the DRI2 loader >>> conditional? >>> >>> I had a bad feeling about this, fortunately it seems pretty easy to >> handle. >> >> From a quick look nothing in glx/glxdri2.c should require libdrm so we >> can nuke the drm.h and xf86drm.h includes, which will get you back up >> and going. Alternatively we can add those in a ifdef WITH_LIBDRM/endif >> block. >> > > That's not quite enough, as building glxdri2.c also requires dri2proto > headers. > > At the moment, configure.ac only requires dri2proto when --enable-dri2 > turns on. > > So either that needs to be made unconditional, or building glxdri2.c made > conditional on DRI2 (untested patch attached) > > You're correct. Wrapping it in DRI2 conditional is a good idea. Note creating an empty (no sources or static libs) library is likely to cause problems. Just use the form prior to my patch ? Emil ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
[PATCH 1/3] os/connection: Improve abstraction for launchd secure sockets
This changes away from hard-coding the /tmp/launch-* path to now
supporting a generic [.]
format for $DISPLAY.
cf-libxcb: d978a4f69b30b630f28d07f1003cf290284d24d8
Signed-off-by: Jeremy Huddleston Sequoia
CC: Adam Jackson
---
os/connection.c | 31 ++-
1 file changed, 26 insertions(+), 5 deletions(-)
diff --git a/os/connection.c b/os/connection.c
index a901ebf..0d42184 100644
--- a/os/connection.c
+++ b/os/connection.c
@@ -79,6 +79,8 @@ SOFTWARE.
#include
#include
+#include
+
#ifndef WIN32
#include
@@ -1112,15 +1114,34 @@ MakeClientGrabPervious(ClientPtr client)
void
ListenOnOpenFD(int fd, int noxauth)
{
-char port[256];
+char port[PATH_MAX];
XtransConnInfo ciptr;
const char *display_env = getenv("DISPLAY");
-if (display_env && (strncmp(display_env, "/tmp/launch", 11) == 0)) {
-/* Make the path the launchd socket if our DISPLAY is set right */
-strcpy(port, display_env);
+/* First check if display_env matches a [.] scheme (eg: launchd) */
+if (display_env && display_env[0] == '/') {
+struct stat sbuf;
+
+strlcpy(port, display_env, sizeof(port));
+
+/* If the path exists, we don't have do do anything else.
+ * If it doesn't, we need to check for a . to strip off
and recheck.
+ */
+if (0 != stat(port, &sbuf)) {
+char *dot = strrchr(port, '.');
+if (dot) {
+*dot = '\0';
+
+if (0 != stat(port, &sbuf)) {
+display_env = NULL;
+}
+} else {
+display_env = NULL;
+}
+}
}
-else {
+
+if (!display_env) {
/* Just some default so things don't break and die. */
snprintf(port, sizeof(port), ":%d", atoi(display));
}
--
2.9.3
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
[PATCH 2/3] randr: Initialize RandR even if there are currently no screens attached
Failure to do so causes an overvlow in RRClientCallback().
=
==41262==ERROR: AddressSanitizer: global-buffer-overflow on address
0x000103ccfbc8 at pc 0x0001034f32b9 bp 0x735a94c0 sp 0x735a94b8
WRITE of size 4 at 0x000103ccfbc8 thread T6
#0 0x1034f32b8 in RRClientCallback randr.c:72
#1 0x1038c75e3 in _CallCallbacks dixutils.c:737
#2 0x10388f406 in CallCallbacks callback.h:83
#3 0x1038bc49a in NextAvailableClient dispatch.c:3562
#4 0x103ad094c in AllocNewConnection connection.c:777
#5 0x103ad1695 in EstablishNewConnections connection.c:863
#6 0x1038c6630 in ProcessWorkQueue dixutils.c:523
#7 0x103ab2dbf in WaitForSomething WaitFor.c:175
#8 0x103880836 in Dispatch dispatch.c:411
#9 0x1038c2141 in dix_main main.c:301
#10 0x1032ac75a in server_thread quartzStartup.c:66
#11 0x7fffc5f16aaa in _pthread_body (libsystem_pthread.dylib+0x3aaa)
#12 0x7fffc5f169f6 in _pthread_start (libsystem_pthread.dylib+0x39f6)
#13 0x7fffc5f161fc in thread_start (libsystem_pthread.dylib+0x31fc)
Signed-off-by: Jeremy Huddleston Sequoia
---
randr/randr.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/randr/randr.c b/randr/randr.c
index 0138dc1..efd3859 100644
--- a/randr/randr.c
+++ b/randr/randr.c
@@ -387,9 +387,6 @@ RRExtensionInit(void)
{
ExtensionEntry *extEntry;
-if (RRNScreens == 0)
-return;
-
if (!dixRegisterPrivateKey(&RRClientPrivateKeyRec, PRIVATE_CLIENT,
sizeof(RRClientRec) +
screenInfo.numScreens * sizeof(RRTimesRec)))
--
2.9.3
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
[PATCH 3/3] glx: Initialize glx even if there are currently no screens attached
Failure to do so causes an overvlow in glxClientCallback
Application Specific Information:
X.Org X Server 1.18.99.1 Build Date: 20160911
=
==52118==ERROR: AddressSanitizer: SEGV on unknown address 0x000102b27b80 (pc
0x000103433245 bp 0x7de67c20 sp 0x7de67c00 T6)
#0 0x103433244 in __asan::asan_free(void*,
__sanitizer::BufferedStackTrace*, __asan::AllocType)
(libclang_rt.asan_osx_dynamic.dylib+0x3244)
#1 0x10347aeee in wrap_free (libclang_rt.asan_osx_dynamic.dylib+0x4aeee)
#2 0x102e6a5ed in glxClientCallback glxext.c:301
#3 0x102b672a3 in _CallCallbacks dixutils.c:737
#4 0x102b2f0c6 in CallCallbacks callback.h:83
#5 0x102b5c15a in NextAvailableClient dispatch.c:3562
#6 0x102d7060c in AllocNewConnection connection.c:777
#7 0x102d71355 in EstablishNewConnections connection.c:863
#8 0x102b662f0 in ProcessWorkQueue dixutils.c:523
#9 0x102d52a7f in WaitForSomething WaitFor.c:175
#10 0x102b204f6 in Dispatch dispatch.c:411
#11 0x102b61e01 in dix_main main.c:301
#12 0x10254c42a in server_thread quartzStartup.c:66
#13 0x7fffc5f16aaa in _pthread_body (libsystem_pthread.dylib+0x3aaa)
#14 0x7fffc5f169f6 in _pthread_start (libsystem_pthread.dylib+0x39f6)
#15 0x7fffc5f161fc in thread_start (libsystem_pthread.dylib+0x31fc)
Signed-off-by: Jeremy Huddleston Sequoia
---
glx/glxext.c | 21 -
1 file changed, 21 deletions(-)
diff --git a/glx/glxext.c b/glx/glxext.c
index d595a05..d216c9d 100644
--- a/glx/glxext.c
+++ b/glx/glxext.c
@@ -319,23 +319,6 @@ GlxPushProvider(__GLXprovider * provider)
__glXProviderStack = provider;
}
-static Bool
-checkScreenVisuals(void)
-{
-int i, j;
-
-for (i = 0; i < screenInfo.numScreens; i++) {
-ScreenPtr screen = screenInfo.screens[i];
-for (j = 0; j < screen->numVisuals; j++) {
-if (screen->visuals[j].class == TrueColor ||
-screen->visuals[j].class == DirectColor)
-return True;
-}
-}
-
-return False;
-}
-
static void
GetGLXDrawableBytes(void *value, XID id, ResourceSizePtr size)
{
@@ -371,10 +354,6 @@ GlxExtensionInit(void)
*stack = &__glXDRISWRastProvider;
}
-/* Mesa requires at least one True/DirectColor visual */
-if (!checkScreenVisuals())
-return;
-
__glXContextRes = CreateNewResourceType((DeleteType) ContextGone,
"GLXContext");
__glXDrawableRes = CreateNewResourceType((DeleteType) DrawableGone,
--
2.9.3
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH libXi] SizeClassInfo can return 0 even without an error
Hi Emil,
On Sunday, 9 October 2016, 15:34:28 CEST, Emil Velikov wrote:
> Hi Niels,
>
> On Friday, 7 October 2016, Niels Ole Salscheider <
>
> niels_...@salscheider-online.de> wrote:
> > Catch the error case separately. This fixes a few crashes on my computer.
> >
> > Signed-off-by: Niels Ole Salscheider > >
> > ---
> >
> > src/XListDev.c | 21 ++---
> > 1 file changed, 10 insertions(+), 11 deletions(-)
> >
> > diff --git a/src/XListDev.c b/src/XListDev.c
> > index f850cd0..d0c6bf2 100644
> > --- a/src/XListDev.c
> > +++ b/src/XListDev.c
> > @@ -73,27 +73,27 @@ static int pad_to_xid(int base_size)
> >
> > return ((base_size + padsize - 1)/padsize) * padsize;
> >
> > }
> >
> > -static size_t
> > -SizeClassInfo(xAnyClassPtr *any, size_t len, int num_classes)
> > +static int
> > +SizeClassInfo(xAnyClassPtr *any, size_t len, int num_classes, size_t
> > *size)
> >
> > {
> >
> > -int size = 0;
> >
> > int j;
> >
> > +*size = 0;
>
> No function should alter the contents of the arguments in case of an error.
> For your other libXi patch one might want to fix the callers, if applicable.
>
> If possible please mention a bug report/link or a bit more about how you
> hit this. Wondering how it has gone unnoticed for so long.
I encountered the bug in chromium and all users of it, including all
applications that use QtWebEngine. I now hit the error path because of the bug
that is fixed by this patch.
Chromium crashes in the following lines: https://chromium.googlesource.com/
chromium/src/+/master/ui/events/devices/x11/device_data_manager_x11.cc#246
Here, GetXDeviceList calls XListInputDevices:
https://chromium.googlesource.com/chromium/src/+/master/ui/events/devices/x11/
device_list_cache_x11.cc#53
The chromium implementation is only correct if ndevices is set to 0 in the
error case since it does not check if a null pointer is returned. I was not
sure if it is supposed to do the latter since the man page for
XListInputDevices doesn't mention it.
> That aside, nicely spotted !
> Emil
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
