Don't unlink socket on close

Otherwise it's possible to close file that does not belong
to the current socket.

Refs zeromq#2764

src/ipc_listener.cpp

@@ -79,12 +79,12 @@ int zmq::ipc_listener_t::create_wildcard_address(std::string& path_,
  // If TMPDIR, TEMPDIR, or TMP are available and are directories, create
  // the socket directory there.
  const char **tmp_env = tmp_env_vars;
- while ( tmp_path.empty() && *tmp_env != 0 ) {
+ while ( tmp_path.empty() && *tmp_env != NULL ) {
  char *tmpdir = getenv(*tmp_env);
  struct stat statbuf;
 
  // Confirm it is actually a directory before trying to use
- if ( tmpdir != 0 && ::stat(tmpdir, &statbuf) == 0 && S_ISDIR(statbuf.st_mode) ) {
+ if ( tmpdir != NULL && ::stat(tmpdir, &statbuf) == 0 && S_ISDIR(statbuf.st_mode) ) {
  tmp_path.assign(tmpdir);
  if ( *(tmp_path.rbegin()) != '/' ) {
  tmp_path.push_back('/');
@@ -125,6 +125,7 @@ int zmq::ipc_listener_t::create_wildcard_address(std::string& path_,
  ::close (fd);
 
  file_.assign (&buffer[0]);
+ ::unlink (file_.c_str ());
 #endif
 
  return 0;
@@ -134,7 +135,7 @@ zmq::ipc_listener_t::ipc_listener_t (io_thread_t *io_thread_,
  socket_base_t *socket_, const options_t &options_) :
  own_t (io_thread_, options_),
  io_object_t (io_thread_),
- has_file (false),
+ is_wildcard (false),
  s (retired_fd),
  socket (socket_)
 {
@@ -210,78 +211,66 @@ int zmq::ipc_listener_t::get_address (std::string &addr_)
 
 int zmq::ipc_listener_t::set_address (const char *addr_)
 {
- // Create addr on stack for auto-cleanup
+ if (options.use_fd != -1) {
+ s = options.use_fd;
+ socket->event_listening (endpoint, s);
+ return 0;
+ }
+
  std::string addr (addr_);
+ ipc_address_t address;
+ int rc = 0;
 
  // Allow wildcard file
- if (options.use_fd == -1 && addr [0] == '*') {
- if ( create_wildcard_address(tmp_socket_dirname, addr) < 0 ) {
- return -1;
+ if (addr.length() == 1 && addr [0] == '*') {
+ if (create_wildcard_address(tmp_socket_dirname, addr) < 0) {
+ goto error;
  }
- }
 
- // Get rid of the file associated with the UNIX domain socket that
- // may have been left behind by the previous run of the application.
- // MUST NOT unlink if the FD is managed by the user, or it will stop
- // working after the first client connects. The user will take care of
- // cleaning up the file after the service is stopped.
- if (options.use_fd == -1) {
- ::unlink (addr.c_str());
+ is_wildcard = true;
  }
+
  filename.clear ();
 
  // Initialise the address structure.
- ipc_address_t address;
- int rc = address.resolve (addr.c_str());
- if (rc != 0) {
- if ( !tmp_socket_dirname.empty() ) {
- // We need to preserve errno to return to the user
- int errno_ = errno;
- ::rmdir(tmp_socket_dirname.c_str ());
- tmp_socket_dirname.clear();
- errno = errno_;
- }
- return -1;
- }
+ rc = address.resolve (addr.c_str());
+ if (rc != 0)
+ goto error;
 
  address.to_string (endpoint);
 
- if (options.use_fd != -1) {
- s = options.use_fd;
- } else {
- // Create a listening socket.
- s = open_socket (AF_UNIX, SOCK_STREAM, 0);
- if (s == -1) {
- if ( !tmp_socket_dirname.empty() ) {
- // We need to preserve errno to return to the user
- int errno_ = errno;
- ::rmdir(tmp_socket_dirname.c_str ());
- tmp_socket_dirname.clear();
- errno = errno_;
- }
- return -1;
- }
+ // Create a listening socket.
+ s = open_socket (AF_UNIX, SOCK_STREAM, 0);
+ if (s == -1) {
+ s = retired_fd;
+ goto error;
+ }
 
- // Bind the socket to the file path.
- rc = bind (s, address.addr (), address.addrlen ());
- if (rc != 0)
- goto error;
+ ::unlink (addr.c_str ());
 
- // Listen for incoming connections.
- rc = listen (s, options.backlog);
- if (rc != 0)
- goto error;
- }
+ // Bind the socket to the file path.
+ rc = bind (s, address.addr (), address.addrlen ());
+ if (rc != 0)
+ goto error;
 
- filename.assign (addr.c_str());
- has_file = true;
+ // Listen for incoming connections.
+ rc = listen (s, options.backlog);
+ if (rc != 0)
+ goto error;
+
+ filename.assign (addr.c_str ());
 
  socket->event_listening (endpoint, s);
  return 0;
 
 error:
  int err = errno;
- close ();
+
+ if (s != retired_fd)
+ close ();
+ else
+ cleanup ();
+
  errno = err;
  return -1;
 }
@@ -292,31 +281,35 @@ void zmq::ipc_listener_t::close ()
  int rc = ::close (s);
  errno_assert (rc == 0);
 
+ rc = cleanup ();
+
+ if (rc == 0)
+ socket->event_closed (endpoint, s);
+ else
+ socket->event_close_failed (endpoint, zmq_errno ());
+
  s = retired_fd;
+}
 
- // If there's an underlying UNIX domain socket, get rid of the file it
- // is associated with.
- // MUST NOT unlink if the FD is managed by the user, or it will stop
- // working after the first client connects. The user will take care of
- // cleaning up the file after the service is stopped.
- if (has_file && options.use_fd == -1) {
- rc = 0;
+int zmq::ipc_listener_t::cleanup ()
+{
+ if (options.use_fd != -1)
+ return 0;
 
- if ( !filename.empty () ) {
- rc = ::unlink(filename.c_str ());
- }
+ int rc = 0;
 
- if ( rc == 0 && !tmp_socket_dirname.empty() ) {
- rc = ::rmdir(tmp_socket_dirname.c_str ());
- tmp_socket_dirname.clear();
- }
+ // It's only safe to unlink a temporary file.
+ if (is_wildcard) {
+ rc = ::unlink (filename.c_str ());
+ filename.clear ();
+ }
 
-  if (rc != 0) {
-  socket->event_close_failed (endpoint, zmq_errno());
- }
+ if (!tmp_socket_dirname.empty ()) {
+ rc = ::rmdir (tmp_socket_dirname.c_str ());
+ tmp_socket_dirname.clear ();
  }
 
- socket->event_closed (endpoint, s);
+ return rc;
 }
 
 #if defined ZMQ_HAVE_SO_PEERCRED

src/ipc_listener.hpp

@@ -71,6 +71,8 @@ namespace zmq
  // Close the listening socket.
  void close ();
 
+ int cleanup ();
+
  // Create wildcard path address
  static int create_wildcard_address(std::string& path_,
  std::string& file_);
@@ -86,8 +88,8 @@ namespace zmq
  // if the connection was dropped while waiting in the listen backlog.
  fd_t accept ();
 
- // True, if the underlying file for UNIX domain socket exists.
- bool has_file;
+ // True, if the underlying file is created for wildcard.
+ bool is_wildcard;
 
  // Name of the temporary directory (if any) that has the
  // the UNIX domain socket

tests/CMakeLists.txt

@@ -91,6 +91,7 @@ if(NOT WIN32)
  test_use_fd_ipc
  test_use_fd_tcp
  test_zmq_poll_fd
+ test_ipc_rebind
  )
  if(HAVE_FORK)
  list(APPEND tests test_fork)

tests/test_ipc_rebind.cpp

@@ -0,0 +1,83 @@
+/*
+ Copyright (c) 2007-2017 Contributors as noted in the AUTHORS file
+
+ This file is part of libzmq, the ZeroMQ core engine in C++.
+
+ libzmq is free software; you can redistribute it and/or modify it under
+ the terms of the GNU Lesser General Public License (LGPL) as published
+ by the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ As a special exception, the Contributors give you permission to link
+ this library with independent modules to produce an executable,
+ regardless of the license terms of these independent modules, and to
+ copy and distribute the resulting executable under terms of your choice,
+ provided that you also meet, for each linked independent module, the
+ terms and conditions of the license of that module. An independent
+ module is a module which is not derived from or based on this library.
+ If you modify this library, you must extend this exception to your
+ version of the library.
+
+ libzmq is distributed in the hope that it will be useful, but WITHOUT
+ ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public
+ License for more details.
+
+ You should have received a copy of the GNU Lesser General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>.
+*/
+
+#include "testutil.hpp"
+
+static const char* SOCKET_ADDR = "ipc:///tmp/tester";
+
+
+int main (void)
+{
+ setup_test_environment();
+
+ void *ctx = zmq_ctx_new ();
+ assert (ctx);
+
+ void *sb = zmq_socket (ctx, ZMQ_PUSH);
+ assert (sb);
+ int rc = zmq_bind (sb, SOCKET_ADDR);
+ assert (rc == 0);
+
+ void *sc = zmq_socket (ctx, ZMQ_PULL);
+ assert (sc);
+ rc = zmq_connect (sc, SOCKET_ADDR);
+ assert (rc == 0);
+
+ rc = zmq_send (sb, "42", 2, 0);
+ assert (rc == 2);
+
+ char buffer [2];
+ rc = zmq_recv(sc, buffer, 2, 0);
+ assert (rc == 2);
+
+ rc = zmq_close (sb);
+ assert (rc == 0);
+
+ sb = zmq_socket (ctx, ZMQ_PUSH);
+ assert (sb);
+ rc = zmq_bind (sb, SOCKET_ADDR);
+ assert (rc == 0);
+
+ rc = zmq_send (sb, "42", 2, 0);
+ assert (rc == 2);
+
+ rc = zmq_recv(sc, buffer, 2, 0);
+ assert (rc == 2);
+
+ rc = zmq_close (sc);
+ assert (rc == 0);
+
+ rc = zmq_close (sb);
+ assert (rc == 0);
+
+ rc = zmq_ctx_term (ctx);
+ assert (rc == 0);
+
+ return 0;
+}