Changeset 89947 in vbox

Timestamp:

Jun 29, 2021 10:42:28 AM (4 years ago)

Author:

vboxsync

svn:sync-xref-src-repo-rev:

145405

Message:

X11: Host Services: Shared Clipboard: improve error code path handling, bugref:10038.

This commit improves error code path handling for X11 part of host Shared Clipboard
service. In particular, it adjusts initialization calls to match to its de-init counterpart.
It also adds more resources release on error path. In addition, this commit limits number of
simmultaneous HGCM connections to ShCl host service in order to prevent potential host flooding.

Location:

trunk

Files:

• include/VBox/GuestHost/SharedClipboard-x11.h (modified) (1 diff)
• src/VBox/GuestHost/SharedClipboard/clipboard-x11.cpp (modified) (8 diffs)
• src/VBox/HostServices/SharedClipboard/VBoxSharedClipboardSvc-x11.cpp (modified) (6 diffs)
• src/VBox/HostServices/SharedClipboard/VBoxSharedClipboardSvc.cpp (modified) (3 diffs)

trunk/include/VBox/GuestHost/SharedClipboard-x11.h

@@ -38 +38 @@
 # include <VBox/GuestHost/SharedClipboard-transfers.h>
 #endif
+
+/**
+ * The maximum number of simultaneous connections to shared clipboard service.
+ * This constant limits amount of GUEST -> HOST connections to shared clipboard host service
+ * for X11 host only. Once amount of connections reaches this number, all the
+ * further attempts to CONNECT will be dropped on an early stage. Possibility to connect
+ * is available again after one of existing connections is closed by DISCONNECT call.
+ */
+#define VBOX_SHARED_CLIPBOARD_X11_CONNECTIONS_MAX   (20)
 
 /** Enables the Xt busy / update handling. */

trunk/src/VBox/GuestHost/SharedClipboard/clipboard-x11.cpp

@@ -279 +279 @@
 
 /**
- * The number of simultaneous instances we support.  For all normal purposes
- * we should never need more than one.  For the testcase it is convenient to
- * have a second instance that the first can interact with in order to have
- * a more controlled environment.
- */
-enum { CLIP_MAX_CONTEXTS = 20 };
-
-/**
  * Array of structures for mapping Xt widgets to context pointers.  We
  * need this because the widget clipboard callbacks do not pass user data.
@@ -296 +288 @@
     /** Pointer to X11 context associated with the widget. */
     PSHCLX11CTX pCtx;
-} g_aContexts[CLIP_MAX_CONTEXTS];
+} g_aContexts[VBOX_SHARED_CLIPBOARD_X11_CONNECTIONS_MAX];
 
 /**
@@ -407 +399 @@
 
 #ifndef TESTCASE
+    AssertReturn(pCtx, VERR_INVALID_POINTER);
+    AssertReturn(pCtx->pAppContext, VERR_INVALID_POINTER);
+
     XtAppAddTimeOut(pCtx->pAppContext, 0, (XtTimerCallbackProc)proc,
                     (XtPointer)client_data);
@@ -884 +879 @@
             }
 
+            LogRel(("Shared Clipboard: X11 event thread exiting\n"));
+
             clipUnregisterContext(pCtx);
+        }
+        else
+        {
+            LogRel(("Shared Clipboard: unable to register clip context: %Rrc\n", rc));
         }
 
@@ -1293 +1294 @@
 int ShClX11ThreadStop(PSHCLX11CTX pCtx)
 {
+    int rc;
     /*
      * Immediately return if we are not connected to the X server.
@@ -1302 +1304 @@
 
     /* Write to the "stop" pipe. */
-    clipThreadScheduleCall(pCtx, clipThreadSignalStop, (XtPointer)pCtx);
+    rc = clipThreadScheduleCall(pCtx, clipThreadSignalStop, (XtPointer)pCtx);
+    if (RT_FAILURE(rc))
+    {
+        LogRel(("Shared Clipboard: cannot notify X11 event thread on shutdown with %Rrc\n", rc));
+        return rc;
+    }
 
     LogRel2(("Shared Clipboard: Waiting for X11 event thread to stop ...\n"));
 
     int rcThread;
-    int rc = RTThreadWait(pCtx->Thread, RT_MS_30SEC /* msTimeout */, &rcThread);
+    rc = RTThreadWait(pCtx->Thread, RT_MS_30SEC /* msTimeout */, &rcThread);
     if (RT_SUCCESS(rc))
         rc = rcThread;
@@ -1870 +1877 @@
         rc = clipThreadScheduleCall(pCtx, ShClX11ReportFormatsToX11Worker,
                                     (XtPointer)pFormats);
+        if (RT_FAILURE(rc))
+            RTMemFree(pFormats);
     }
     else
@@ -2274 +2283 @@
         /* We use this to schedule a worker function on the event thread. */
         rc = clipThreadScheduleCall(pCtx, ShClX11ReadDataFromX11Worker, (XtPointer)pX11Req);
+        if (RT_FAILURE(rc))
+            RTMemFree(pX11Req);
     }
     else

trunk/src/VBox/HostServices/SharedClipboard/VBoxSharedClipboardSvc-x11.cpp

@@ -27 +27 @@
 #include <iprt/semaphore.h>
 #include <iprt/string.h>
+#include <iprt/asm.h>
 
 #include <VBox/GuestHost/SharedClipboard.h>
@@ -35 +36 @@
 #include "VBoxSharedClipboardSvc-internal.h"
 
+/* Number of currently extablished connections. */
+static volatile uint32_t g_cShClConnections;
 
 /*********************************************************************************************************************************
@@ -76 +79 @@
     int rc;
 
+    /* Check if maximum allowed connections count has reached. */
+    if (ASMAtomicIncU32(&g_cShClConnections) > VBOX_SHARED_CLIPBOARD_X11_CONNECTIONS_MAX)
+    {
+        ASMAtomicDecU32(&g_cShClConnections);
+        LogRel(("Shared Clipboard: maximum amount for client connections reached\n"));
+        return VERR_OUT_OF_RESOURCES;
+    }
+
     PSHCLCONTEXT pCtx = (PSHCLCONTEXT)RTMemAllocZ(sizeof(SHCLCONTEXT));
     if (pCtx)
@@ -96 +107 @@
                 RTCritSectDelete(&pCtx->CritSect);
         }
-        else
+
+        if (RT_FAILURE(rc))
+        {
+            pClient->State.pCtx = NULL;
             RTMemFree(pCtx);
+        }
     }
     else
         rc = VERR_NO_MEMORY;
+
+    if (RT_FAILURE(rc))
+    {
+        /* Restore active connections count. */
+        ASMAtomicDecU32(&g_cShClConnections);
+    }
 
     LogFlowFuncLeaveRC(rc);
@@ -141 +162 @@
 
     RTMemFree(pCtx);
+
+    /* Decrease active connections count. */
+    ASMAtomicDecU32(&g_cShClConnections);
 
     LogFlowFuncLeaveRC(rc);
@@ -253 +277 @@
     LogFlowFunc(("pCtx=%p, fFormats=%#x\n", pCtx, fFormats));
 
+    int rc = VINF_SUCCESS;
     PSHCLCLIENT pClient = pCtx->pClient;
     AssertPtr(pClient);
 
-    RTCritSectEnter(&pClient->CritSect);
-
-    /** @todo r=bird: BUGBUG: Revisit this   */
-    if (fFormats == VBOX_SHCL_FMT_NONE) /* No formats to report? Bail out early. */
-        return;
-
-    int rc = ShClSvcHostReportFormats(pCtx->pClient, fFormats);
-    RT_NOREF(rc);
-
-    RTCritSectLeave(&pClient->CritSect);
+    rc = RTCritSectEnter(&pClient->CritSect);
+    if (RT_SUCCESS(rc))
+    {
+        /** @todo r=bird: BUGBUG: Revisit this   */
+        if (fFormats != VBOX_SHCL_FMT_NONE) /* No formats to report? */
+        {
+            rc = ShClSvcHostReportFormats(pCtx->pClient, fFormats);
+        }
+
+        RTCritSectLeave(&pClient->CritSect);
+    }
 
     LogFlowFuncLeaveRC(rc);

trunk/src/VBox/HostServices/SharedClipboard/VBoxSharedClipboardSvc.cpp

@@ -1995 +1995 @@
     if (RT_SUCCESS(rc))
     {
+        /* Assign weak pointer to client map .*/
+        g_mapClients[u32ClientID] = pClient; /** @todo Handle OOM / collisions? */
+
         rc = ShClBackendConnect(pClient, ShClSvcGetHeadless());
         if (RT_SUCCESS(rc))
@@ -2014 +2017 @@
             if (RT_SUCCESS(rc))
             {
-                /* Assign weak pointer to client map .*/
-                g_mapClients[u32ClientID] = pClient; /** @todo Handle OOM / collisions? */
-
                 /* For now we ASSUME that the first client ever connected is in charge for
                  * communicating withe the service extension.
@@ -2025 +2025 @@
             }
         }
+
+        if (RT_FAILURE(rc))
+        {
+            shClSvcClientDestroy(pClient);
+        }
+
     }