Changed display number selection to avoid clashes with sshd X forwarding.

diff --git a/framebuffer.cpp b/framebuffer.cpp
index c9dc31f7159b0fd20c7d5e1a634f18fc48792a96..2ae42cde48addaf3f758e77af7d7d652b2c883cd 100644 (file)
--- a/framebuffer.cpp
+++ b/framebuffer.cpp
@@ -11,18 +11,35 @@
 #include <sys/types.h>
 #include <errno.h>
 #include <fcntl.h>
-#include <netinet/in.h>
+#include <netdb.h>
+#include <sys/socket.h>
 #include <sys/stat.h>
 #include <sys/utsname.h>
 #include <unistd.h>
 #include <wait.h>
 
 #include "auto_fd.hpp"
+#include "auto_handle.hpp"
 #include "temp_file.hpp"
 
 namespace
 {
-    int select_display_num()
+    struct addrinfo_factory
+    {
+       addrinfo * operator()() const { return NULL; }
+    };
+    struct addrinfo_closer
+    {
+       void operator()(addrinfo * addr_list) const
+           {
+               if (addr_list)
+                   freeaddrinfo(addr_list);
+           }
+    };
+    typedef auto_handle<addrinfo *, addrinfo_closer, addrinfo_factory>
+        auto_addrinfo;
+
+    int select_display_num(auto_fd & tcp4_socket, auto_fd & tcp6_socket)
     {
        // Minimum and maximum display numbers to use.  Xvnc and ssh's
        // proxies start at 10, so we'll follow that convention.  We
@@ -31,28 +48,103 @@ namespace
        const int min_display_num = 10;
        const int max_display_num = 99;
 
-       // Note that we have to leave it to the X server to create the
-       // lock file for the selected display number, which leaves a
-       // race condition.  We could perhaps read its error stream to
-       // detect the case where another server grabs the display
-       // number before it.
-       char lock_file_name[20];
        for (int display_num = min_display_num;
             display_num <= max_display_num;
             ++display_num)
        {
-           // Check whether a lock file exists for this display.  Really we
-           // should also check for stale locks, but this will probably do.
+           // Check that there's no lock file for the local socket
+           // for this display.  We could also check for stale locks,
+           // but this will probably do.
+           char lock_file_name[20];
            std::sprintf(lock_file_name, "/tmp/.X%d-lock", display_num);
-           if (access(lock_file_name, 0) == -1 && errno == ENOENT)
+           if (!(access(lock_file_name, 0) == -1 && errno == ENOENT))
+               continue;
+
+           // Attempt to create TCP socket(s) and bind them to the
+           // appropriate port number.  We won't set the X server to
+           // listen on a TCP socket but this does ensure that ssh
+           // isn't using and won't use this display number.  This is
+           // roughly based on the x11_create_display_inet function
+           // in OpenSSH.
+
+           auto_addrinfo addr_list;
+
+           {
+               addrinfo hints = {};
+               hints.ai_family = AF_UNSPEC;
+               hints.ai_socktype = SOCK_STREAM;
+               char port_str[5 + 1];
+               std::sprintf(port_str, "%d", 6000 + display_num);
+               addrinfo * addr_list_temp;
+               int error = getaddrinfo(NULL, port_str, &hints,
+                                            &addr_list_temp);
+               if (error != 0)
+                   throw std::runtime_error(
+                       std::string("getaddrinfo: ")
+                       .append(gai_strerror(error)));
+               addr_list.reset(addr_list_temp);
+           }
+
+           const addrinfo * addr;
+
+           for (addr = addr_list.get(); addr != NULL; addr = addr->ai_next)
+           {
+               // We're only interested in TCPv4 and TCPv6.
+               if (addr->ai_family != AF_INET && addr->ai_family != AF_INET6)
+                   continue;
+               auto_fd & tcp_socket =
+                   (addr->ai_family == AF_INET) ? tcp4_socket : tcp6_socket;
+
+               tcp_socket.reset(socket(addr->ai_family,
+                                       SOCK_STREAM,
+                                       addr->ai_protocol));
+               if (tcp_socket.get() < 0)
+               {
+                   // If the family is unsupported, no-one can bind
+                   // to this address, so this is not a problem.
+                   if (errno == EAFNOSUPPORT
+#                      ifdef EPFNOSUPPORT
+                       || errno == EPFNOSUPPORT
+#                      endif
+                       )
+                       continue;
+                   throw std::runtime_error(
+                       std::string("socket: ").append(strerror(errno)));
+               }
+
+               // Don't let TCPv6 sockets interfere with TCPv4 sockets.
+#              ifdef IPV6_V6ONLY
+               if (addr->ai_family == AF_INET6)
+               {
+                   int on = 1;
+                   if (setsockopt(tcp_socket.get(), IPPROTO_IPV6, IPV6_V6ONLY,
+                                  &on, sizeof(on)) != 0)
+                   {
+                       throw std::runtime_error(
+                           std::string("setsockopt IPV6_V6ONLY: ")
+                           .append(strerror(errno)));
+                   }
+               }
+#              endif
+
+               if (bind(tcp_socket.get(), addr->ai_addr, addr->ai_addrlen)
+                   != 0)
+                   break;
+           }
+
+           // If we reached the end of the address list, we've
+           // successfully bound to all appropriate addresses for
+           // this display number, so we can use it.
+           if (addr == NULL)
                return display_num;
        }
 
        throw std::runtime_error("did not find a free X display");
     }
 
-    void get_random_bytes(unsigned char * buf, std::size_t len)
+    void get_random_bytes(unsigned char * buf, int len)
     {
+       assert(len > 0);
        auto_fd random_fd(open("/dev/urandom", O_RDONLY));
        if (random_fd.get() == -1 || read(random_fd.get(), buf, len) != len)
            throw std::runtime_error(std::strerror(errno));
@@ -121,7 +213,9 @@ namespace
            execlp("Xvfb",
                   "Xvfb",
                   "-auth", auth_file_c_str,
+                  "-nolisten", "tcp",
                   "-screen", "0", dimensions,
+                  "-terminate",
                   display,
                   NULL);
            _exit(128 + errno);
@@ -151,7 +245,7 @@ namespace
 }
 
 FrameBuffer::FrameBuffer(int width, int height, int depth)
-       : display_num_(select_display_num()),
+       : display_num_(select_display_num(tcp4_socket_, tcp6_socket_)),
          auth_file_(create_temp_auth_file(display_num_)),
          server_proc_(spawn_x_server(display_num_,
                                      get_x_authority(),

diff --git a/framebuffer.hpp b/framebuffer.hpp
index ee2557b9ff332d5e4b73565abf64967a849ecc93..3e4987564382d22f1b1a30e8ab117eb580296624 100644 (file)
--- a/framebuffer.hpp
+++ b/framebuffer.hpp
@@ -7,6 +7,7 @@
 #include <memory>
 #include <string>
 
+#include "auto_fd.hpp"
 #include "auto_proc.hpp"
 #include "temp_file.hpp"
 
@@ -19,6 +20,7 @@ public:
     std::string get_x_display() const;
 
 private:
+    auto_fd tcp4_socket_, tcp6_socket_;
     int display_num_;
     std::auto_ptr<temp_file> auth_file_;
     auto_kill_proc server_proc_;