From: Oliver Kurth <okurth@vmware.com>
Date: Fri, 6 Jul 2018 21:31:55 +0000 (-0700)
Subject: [tools-plugin] Potential solution for reported desktopEvent crash
X-Git-Tag: stable-10.3.0~3
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=c80bb3fc7960bc78a6d39c89b6952218a401b0cf;p=thirdparty%2Fopen-vm-tools.git

[tools-plugin] Potential solution for reported desktopEvent crash

On several distros like RHEL, CentOS and Fedora, plugin desktopEvents.so
crashes have been reported.  However, neither development or QE can
reproduce this issue in local environments.

The XFetchName document states that it may generate BadWindow error.  That
means that the value for a Window argument does not name a defined Window.
X FAQ also suggests handling badwindow error after finding it.  Since
the window does not exist when XFetchName is run on it, the name is not
needed.  Suppress this error to avoid a crash.

After grabbing the X server, use XSetErrorHandler to temporarily define
a local error handler to intercept and cleanly handle any BadWindow
errors.  XSetErrorHandler returns the current error handler, so it is
straightforward to reinstate GDK's original handler.
---

diff --git a/open-vm-tools/services/plugins/desktopEvents/x11Lock.c b/open-vm-tools/services/plugins/desktopEvents/x11Lock.c
index 613b69463..811f4bf7e 100644
--- a/open-vm-tools/services/plugins/desktopEvents/x11Lock.c
+++ b/open-vm-tools/services/plugins/desktopEvents/x11Lock.c
@@ -161,6 +161,43 @@ QueryX11Lock(Display *dpy,
 }
 
 
+/*
+ *-----------------------------------------------------------------------------
+ *
+ * FetchNameErrorHandler --
+ *
+ *      According to XFetchName document, XFetchName may generate a BadWindow
+ *      error. In this case, the pointer we pass to XFetchName doesn't name a
+ *      defined window. X is asynchronous, there isn't a gurantee that the
+ *      window is still alive between the time the window is obtained and
+ *      the time a event is sent to the window. So, for this scenario, since
+ *      the window doesn't exist, we can ignore checking its name. This can
+ *      avoid this plugin from crashing. Refer to PR 1871141 for details.
+ *
+ * Results:
+ *      Logs error.
+ *
+ * Side effects:
+ *      None.
+ *
+ *-----------------------------------------------------------------------------
+ */
+
+static int
+FetchNameErrorHandler(Display *display,
+                      XErrorEvent *error)
+{
+   /* 256 is enough for the error description. */
+   char msg[256];
+   XGetErrorText(display, error->error_code, msg, sizeof(msg));
+
+   g_warning("X Error %d (%s): request %d.%d\n",
+             error->error_code, msg, error->request_code, error->minor_code);
+
+   return 0;
+}
+
+
 /*
  ******************************************************************************
  * AcquireDisplayLock --                                                */ /**
@@ -281,15 +318,24 @@ AcquireDisplayLock(void)
     */
    for (index = 0; (index < nchildren) && !alreadyLocked; index++) {
       char *name = NULL;
+      /* Use customized error handler to suppress BadWindow error. */
+      int (*oldXErrorHandler)(Display*, XErrorEvent*) =
+         XSetErrorHandler(FetchNameErrorHandler);
 
       /* Skip unless window is named vmware-user. */
       if ((XFetchName(defaultDisplay, children[index], &name) == 0) ||
           (name == NULL) ||
           strcmp(name, VMUSER_TITLE)) {
          XFree(name);
+
+         /* Restore default error handler. */
+         XSetErrorHandler(oldXErrorHandler);
          continue;
       }
 
+      /* Restore default error handler. */
+      XSetErrorHandler(oldXErrorHandler);
+
       /*
        * Query the window for the "vmware-user-lock" property.
        */