From 667f95e39e5fd4e13500cb030a1a8ac4a27243ee Mon Sep 17 00:00:00 2001 From: Ben Hutchings Date: Fri, 9 Dec 2005 00:46:20 +0000 Subject: [PATCH] Changed display number selection to avoid clashes with sshd X forwarding. --- framebuffer.cpp | 120 ++++++++++++++++++++++++++++++++++++++++++------ framebuffer.hpp | 2 + 2 files changed, 109 insertions(+), 13 deletions(-) diff --git a/framebuffer.cpp b/framebuffer.cpp index c9dc31f..2ae42cd 100644 --- a/framebuffer.cpp +++ b/framebuffer.cpp @@ -11,18 +11,35 @@ #include #include #include -#include +#include +#include #include #include #include #include #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 + 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 ee2557b..3e49875 100644 --- a/framebuffer.hpp +++ b/framebuffer.hpp @@ -7,6 +7,7 @@ #include #include +#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 auth_file_; auto_kill_proc server_proc_; -- 2.39.5