From 501e8a2279e547a16e7b4b6ba53e39cf80f77690 Mon Sep 17 00:00:00 2001
From: Xinnan Xie <xiexinnan@bytedance.com>
Date: Mon, 4 Mar 2024 14:39:07 +0800
Subject: [PATCH] libvncserver: Add thread protection for updateBuf

Thread safety for updateBuf in rfbSendFramebufferUpdate. In multi-thread
mode, different threads may modify cl->updateBuf and cl->ublen at same
time. When this happends libvncserver may crash.

Signed-off-by: Xinnan Xie <xiexinnan@bytedance.com>
---
 include/rfb/rfb.h            | 6 ++++++
 src/libvncserver/main.c      | 4 ++++
 src/libvncserver/rfbserver.c | 7 +++++++
 3 files changed, 17 insertions(+)

diff --git a/include/rfb/rfb.h b/include/rfb/rfb.h
index 79a446f1b..86c6a5ffe 100644
--- a/include/rfb/rfb.h
+++ b/include/rfb/rfb.h
@@ -707,6 +707,12 @@ typedef struct _rfbClientRec {
     int tightPngDstDataLen;
 #endif
 #endif
+
+    /** for thread safety for updateBuf in rfbSendFBUpdate() */
+#if defined(LIBVNCSERVER_HAVE_LIBPTHREAD) || defined(LIBVNCSERVER_HAVE_WIN32THREADS)
+    MUTEX(updateBufMutex);
+#endif
+
 } rfbClientRec, *rfbClientPtr;
 
 /**
diff --git a/src/libvncserver/main.c b/src/libvncserver/main.c
index 1efa83879..d8c02684a 100644
--- a/src/libvncserver/main.c
+++ b/src/libvncserver/main.c
@@ -1299,7 +1299,9 @@ rfbUpdateClient(rfbClientPtr cl)
         !sraRgnEmpty(cl->requestedRegion)) {
       result=TRUE;
       if(screen->deferUpdateTime == 0) {
+          LOCK(cl->updateBufMutex);
           rfbSendFramebufferUpdate(cl,cl->modifiedRegion);
+          UNLOCK(cl->updateBufMutex);
       } else if(cl->startDeferring.tv_usec == 0) {
         gettimeofday(&cl->startDeferring,NULL);
         if(cl->startDeferring.tv_usec == 0)
@@ -1311,7 +1313,9 @@ rfbUpdateClient(rfbClientPtr cl)
                +(tv.tv_usec-cl->startDeferring.tv_usec)/1000)
              > screen->deferUpdateTime) {
           cl->startDeferring.tv_usec = 0;
+          LOCK(cl->updateBufMutex);
           rfbSendFramebufferUpdate(cl,cl->modifiedRegion);
+          UNLOCK(cl->updateBufMutex);
         }
       }
     }
diff --git a/src/libvncserver/rfbserver.c b/src/libvncserver/rfbserver.c
index a80fd1aee..6a404cf32 100644
--- a/src/libvncserver/rfbserver.c
+++ b/src/libvncserver/rfbserver.c
@@ -378,6 +378,8 @@ rfbNewTCPOrUDPClient(rfbScreenInfoPtr rfbScreen,
       INIT_MUTEX(cl->sendMutex);
       INIT_COND(cl->deleteCond);
 
+      INIT_MUTEX(cl->updateBufMutex);
+
       cl->state = RFB_PROTOCOL_VERSION;
 
       cl->reverseConnection = FALSE;
@@ -650,6 +652,11 @@ rfbClientConnectionGone(rfbClientPtr cl)
     UNLOCK(cl->sendMutex);
     TINI_MUTEX(cl->sendMutex);
 
+    /* make sure updateBufMutex is unlocked before destroying */
+    LOCK(cl->updateBufMutex);
+    UNLOCK(cl->updateBufMutex);
+    TINI_MUTEX(cl->updateBufMutex);
+
 #ifdef LIBVNCSERVER_HAVE_LIBPTHREAD
     if (cl->screen->backgroundLoop) {
         close(cl->pipe_notify_client_thread[0]);