Merge pull request #238 from tetrane/pr-fix-use-after-free

Fix use-after-free and concurrent access segmentation fault

libvncserver/main.c

@@ -33,6 +33,7 @@
 #include <sys/socket.h>
 #include <netinet/in.h>
 #include <unistd.h>
+#include <fcntl.h>
 #endif
 
 #include <signal.h>
@@ -533,6 +534,7 @@ clientInput(void *data)
 
 	FD_ZERO(&rfds);
 	FD_SET(cl->sock, &rfds);
+	FD_SET(cl->pipe_notify_client_thread[0], &rfds);
 	FD_ZERO(&efds);
 	FD_SET(cl->sock, &efds);
 
@@ -541,9 +543,13 @@ clientInput(void *data)
 	if ((cl->fileTransfer.fd!=-1) && (cl->fileTransfer.sending==1))
 	    FD_SET(cl->sock, &wfds);
 
+	int nfds = cl->pipe_notify_client_thread[0] > cl->sock ? cl->pipe_notify_client_thread[0] : cl->sock;
+	
 	tv.tv_sec = 60; /* 1 minute */
 	tv.tv_usec = 0;
-	n = select(cl->sock + 1, &rfds, &wfds, &efds, &tv);
+
+	n = select(nfds + 1, &rfds, &wfds, &efds, &tv);
+
 	if (n < 0) {
 	    rfbLogPerror("ReadExact: select");
 	    break;
@@ -558,6 +564,13 @@ clientInput(void *data)
         if (FD_ISSET(cl->sock, &wfds))
             rfbSendFileTransferChunk(cl);
 
+	if (FD_ISSET(cl->pipe_notify_client_thread[0], &rfds))
+	{
+	    // Reset the pipe
+	    char buf;
+	    while (read(cl->pipe_notify_client_thread[0], &buf, sizeof(buf)) == sizeof(buf));
+	}
+
         if (FD_ISSET(cl->sock, &rfds) || FD_ISSET(cl->sock, &efds))
         {
 #ifdef LIBVNCSERVER_WITH_WEBSOCKETS
@@ -628,8 +641,15 @@ rfbStartOnHoldClient(rfbClientPtr cl)
 {
     cl->onHold = FALSE;
 #ifdef LIBVNCSERVER_HAVE_LIBPTHREAD
-    if(cl->screen->backgroundLoop)
-	pthread_create(&cl->client_thread, NULL, clientInput, (void *)cl);
+    if(cl->screen->backgroundLoop) {
+        if (pipe(cl->pipe_notify_client_thread) == -1) {
+            cl->pipe_notify_client_thread[0] = -1;
+            cl->pipe_notify_client_thread[1] = -1;
+        }
+        fcntl(cl->pipe_notify_client_thread[0], F_SETFL, O_NONBLOCK);
+
+        pthread_create(&cl->client_thread, NULL, clientInput, (void *)cl);
+    }
 #endif
 }
 
@@ -1081,15 +1101,27 @@ void rfbInitServer(rfbScreenInfoPtr screen)
 
 void rfbShutdownServer(rfbScreenInfoPtr screen,rfbBool disconnectClients) {
   if(disconnectClients) {
-    rfbClientPtr cl;
     rfbClientIteratorPtr iter = rfbGetClientIterator(screen);
-    while( (cl = rfbClientIteratorNext(iter)) ) {
-      if (cl->sock > -1) {
-       /* we don't care about maxfd here, because the server goes away */
-       rfbCloseClient(cl);
-       rfbClientConnectionGone(cl);
+    rfbClientPtr nextCl, currentCl = rfbClientIteratorNext(iter);
+
+    while(currentCl) {
+      nextCl = rfbClientIteratorNext(iter);
+      if (currentCl->sock > -1) {
+        /* we don't care about maxfd here, because the server goes away */
+        rfbCloseClient(currentCl);
       }
+
+#ifdef LIBVNCSERVER_HAVE_LIBPTHREAD
+      // Notify the thread and join it
+      write(currentCl->pipe_notify_client_thread[1], "\x00", 1);
+      pthread_join(currentCl->client_thread, NULL);
+#else
+      rfbClientConnectionGone(currentCl);
+#endif
+
+      currentCl = nextCl;
     }
+
     rfbReleaseClientIterator(iter);
   }
 

libvncserver/rfbserver.c

@@ -619,6 +619,11 @@ rfbClientConnectionGone(rfbClientPtr cl)
     UNLOCK(cl->sendMutex);
     TINI_MUTEX(cl->sendMutex);
 
+#ifdef LIBVNCSERVER_HAVE_LIBPTHREAD
+    close(cl->pipe_notify_client_thread[0]);
+    close(cl->pipe_notify_client_thread[1]);
+#endif
+
     rfbPrintStats(cl);
     rfbResetStats(cl);
 

rfb/rfb.h

@@ -465,6 +465,7 @@ typedef struct _rfbClientRec {
     int protocolMinorVersion;
 
 #ifdef LIBVNCSERVER_HAVE_LIBPTHREAD
+    int pipe_notify_client_thread[2];
     pthread_t client_thread;
 #endif