From 359d6678930fbb25e612d290a2ff5114e10eda6f Mon Sep 17 00:00:00 2001
From: Ben Noordhuis <info@bnoordhuis.nl>
Date: Tue, 1 Oct 2013 02:37:45 +0200
Subject: [PATCH] unix: sanity-check fds before closing

Ensure that close() system calls don't close stdio file descriptors
because that is almost never the intention.

This is also a partial workaround for a kernel bug that seems to affect
all Linux kernels when stdin is a pipe that gets closed: fd 0 keeps
signalling EPOLLHUP but a subsequent call to epoll_ctl(EPOLL_CTL_DEL)
fails with EBADF.  See joyent/node#6271 for details and a test case.
---
 src/unix/aix.c               | 16 ++++++++--------
 src/unix/async.c             |  4 ++--
 src/unix/core.c              | 26 +++++++++++++++++++++++---
 src/unix/internal.h          |  1 +
 src/unix/kqueue.c            |  2 +-
 src/unix/linux-core.c        |  4 ++--
 src/unix/linux-inotify.c     |  2 +-
 src/unix/loop.c              |  4 ++--
 src/unix/pipe.c              |  4 ++--
 src/unix/process.c           | 22 ++++++++++++----------
 src/unix/signal.c            |  4 ++--
 src/unix/stream.c            | 32 ++++++++++++++++++--------------
 src/unix/sunos.c             |  8 ++++----
 src/unix/tcp.c               |  2 +-
 src/unix/udp.c               |  9 ++++++---
 test/test-stdio-over-pipes.c |  1 -
 16 files changed, 85 insertions(+), 56 deletions(-)

diff --git a/src/unix/aix.c b/src/unix/aix.c
index 604a38d8..3bd6ec08 100644
--- a/src/unix/aix.c
+++ b/src/unix/aix.c
@@ -85,7 +85,7 @@ int uv_exepath(char* buffer, size_t* size) {
     return fd;
 
   res = read(fd, &ps, sizeof(ps));
-  close(fd);
+  uv__close(fd);
   if (res < 0)
     return res;
 
@@ -179,7 +179,7 @@ int uv_resident_set_memory(size_t* rss) {
     *rss = (size_t)psinfo.pr_rssize * 1024;
     err = 0;
   }
-  close(fd);
+  uv__close(fd);
 
   return err;
 }
@@ -291,14 +291,14 @@ int uv_interface_addresses(uv_interface_address_t** addresses,
   }
 
   if (ioctl(sockfd, SIOCGSIZIFCONF, &size) == -1) {
-    close(sockfd);
+    uv__close(sockfd);
     return -ENOSYS;
   }
 
   ifc.ifc_req = (struct ifreq*)malloc(size);
   ifc.ifc_len = size;
   if (ioctl(sockfd, SIOCGIFCONF, &ifc) == -1) {
-    close(sockfd);
+    uv__close(sockfd);
     return -ENOSYS;
   }
 
@@ -317,7 +317,7 @@ int uv_interface_addresses(uv_interface_address_t** addresses,
 
     memcpy(flg.ifr_name, p->ifr_name, sizeof(flg.ifr_name));
     if (ioctl(sockfd, SIOCGIFFLAGS, &flg) == -1) {
-      close(sockfd);
+      uv__close(sockfd);
       return -ENOSYS;
     }
 
@@ -331,7 +331,7 @@ int uv_interface_addresses(uv_interface_address_t** addresses,
   *addresses = (uv_interface_address_t*)
     malloc(*count * sizeof(uv_interface_address_t));
   if (!(*addresses)) {
-    close(sockfd);
+    uv__close(sockfd);
     return -ENOMEM;
   }
   address = *addresses;
@@ -348,7 +348,7 @@ int uv_interface_addresses(uv_interface_address_t** addresses,
 
     memcpy(flg.ifr_name, p->ifr_name, sizeof(flg.ifr_name));
     if (ioctl(sockfd, SIOCGIFFLAGS, &flg) == -1) {
-      close(sockfd);
+      uv__close(sockfd);
       return -ENOSYS;
     }
 
@@ -374,7 +374,7 @@ int uv_interface_addresses(uv_interface_address_t** addresses,
 
 #undef ADDR_SIZE
 
-  close(sockfd);
+  uv__close(sockfd);
   return 0;
 }
 
diff --git a/src/unix/async.c b/src/unix/async.c
index 5ced3cbe..3c23e1d7 100644
--- a/src/unix/async.c
+++ b/src/unix/async.c
@@ -237,11 +237,11 @@ void uv__async_stop(uv_loop_t* loop, struct uv__async* wa) {
     return;
 
   uv__io_stop(loop, &wa->io_watcher, UV__POLLIN);
-  close(wa->io_watcher.fd);
+  uv__close(wa->io_watcher.fd);
   wa->io_watcher.fd = -1;
 
   if (wa->wfd != -1) {
-    close(wa->wfd);
+    uv__close(wa->wfd);
     wa->wfd = -1;
   }
 }
diff --git a/src/unix/core.c b/src/unix/core.c
index 7de85bf6..b23f6aed 100644
--- a/src/unix/core.c
+++ b/src/unix/core.c
@@ -340,7 +340,7 @@ int uv__socket(int domain, int type, int protocol) {
     err = uv__cloexec(sockfd, 1);
 
   if (err) {
-    close(sockfd);
+    uv__close(sockfd);
     return err;
   }
 
@@ -397,7 +397,7 @@ skip:
       err = uv__nonblock(peerfd, 1);
 
     if (err) {
-      close(peerfd);
+      uv__close(peerfd);
       return err;
     }
 
@@ -406,6 +406,26 @@ skip:
 }
 
 
+int uv__close(int fd) {
+  int saved_errno;
+  int rc;
+
+  assert(fd > -1);  /* Catch uninitialized io_watcher.fd bugs. */
+  assert(fd > STDERR_FILENO);  /* Catch stdio close bugs. */
+
+  saved_errno = errno;
+  rc = close(fd);
+  if (rc == -1) {
+    rc = -errno;
+    if (rc == -EINTR)
+      rc = -EINPROGRESS;  /* For platform/libc consistency. */
+    errno = saved_errno;
+  }
+
+  return rc;
+}
+
+
 #if defined(__linux__) || defined(__FreeBSD__) || defined(__APPLE__)
 
 int uv__nonblock(int fd, int set) {
@@ -514,7 +534,7 @@ int uv__dup(int fd) {
 
   err = uv__cloexec(fd, 1);
   if (err) {
-    close(fd);
+    uv__close(fd);
     return err;
   }
 
diff --git a/src/unix/internal.h b/src/unix/internal.h
index 67c847ba..c050c522 100644
--- a/src/unix/internal.h
+++ b/src/unix/internal.h
@@ -130,6 +130,7 @@ enum {
 
 /* core */
 int uv__nonblock(int fd, int set);
+int uv__close(int fd);
 int uv__cloexec(int fd, int set);
 int uv__socket(int domain, int type, int protocol);
 int uv__dup(int fd);
diff --git a/src/unix/kqueue.c b/src/unix/kqueue.c
index 391ab615..f94c7e8c 100644
--- a/src/unix/kqueue.c
+++ b/src/unix/kqueue.c
@@ -354,6 +354,6 @@ void uv__fs_event_close(uv_fs_event_t* handle) {
   free(handle->filename);
   handle->filename = NULL;
 
-  close(handle->event_watcher.fd);
+  uv__close(handle->event_watcher.fd);
   handle->event_watcher.fd = -1;
 }
diff --git a/src/unix/linux-core.c b/src/unix/linux-core.c
index fa5dec19..29a80d87 100644
--- a/src/unix/linux-core.c
+++ b/src/unix/linux-core.c
@@ -98,7 +98,7 @@ int uv__platform_loop_init(uv_loop_t* loop, int default_loop) {
 void uv__platform_loop_delete(uv_loop_t* loop) {
   if (loop->inotify_fd == -1) return;
   uv__io_stop(loop, &loop->inotify_read_watcher, UV__POLLIN);
-  close(loop->inotify_fd);
+  uv__close(loop->inotify_fd);
   loop->inotify_fd = -1;
 }
 
@@ -309,7 +309,7 @@ int uv_resident_set_memory(size_t* rss) {
     n = read(fd, buf, sizeof(buf) - 1);
   while (n == -1 && errno == EINTR);
 
-  SAVE_ERRNO(close(fd));
+  uv__close(fd);
   if (n == -1)
     return -errno;
   buf[n] = '\0';
diff --git a/src/unix/linux-inotify.c b/src/unix/linux-inotify.c
index 4bf30dbf..76419985 100644
--- a/src/unix/linux-inotify.c
+++ b/src/unix/linux-inotify.c
@@ -81,7 +81,7 @@ static int new_inotify_fd(void) {
     err = uv__nonblock(fd, 1);
 
   if (err) {
-    close(fd);
+    uv__close(fd);
     return err;
   }
 
diff --git a/src/unix/loop.c b/src/unix/loop.c
index 4c10f0f3..cbe93171 100644
--- a/src/unix/loop.c
+++ b/src/unix/loop.c
@@ -136,12 +136,12 @@ static void uv__loop_delete(uv_loop_t* loop) {
   uv__async_stop(loop, &loop->async_watcher);
 
   if (loop->emfile_fd != -1) {
-    close(loop->emfile_fd);
+    uv__close(loop->emfile_fd);
     loop->emfile_fd = -1;
   }
 
   if (loop->backend_fd != -1) {
-    close(loop->backend_fd);
+    uv__close(loop->backend_fd);
     loop->backend_fd = -1;
   }
 
diff --git a/src/unix/pipe.c b/src/unix/pipe.c
index d7c4794b..2ff47669 100644
--- a/src/unix/pipe.c
+++ b/src/unix/pipe.c
@@ -93,11 +93,11 @@ int uv_pipe_bind(uv_pipe_t* handle, const char* name) {
 
 out:
   if (bound) {
-    /* unlink() before close() to avoid races. */
+    /* unlink() before uv__close() to avoid races. */
     assert(pipe_fname != NULL);
     unlink(pipe_fname);
   }
-  close(sockfd);
+  uv__close(sockfd);
   free((void*)pipe_fname);
   return err;
 }
diff --git a/src/unix/process.c b/src/unix/process.c
index 990ea157..cd369c7d 100644
--- a/src/unix/process.c
+++ b/src/unix/process.c
@@ -231,7 +231,7 @@ static int uv__process_open_stream(uv_stdio_container_t* container,
   if (!(container->flags & UV_CREATE_PIPE) || pipefds[0] < 0)
     return 0;
 
-  if (close(pipefds[1]))
+  if (uv__close(pipefds[1]))
     if (errno != EINTR && errno != EINPROGRESS)
       abort();
 
@@ -285,8 +285,10 @@ static void uv__process_child_init(const uv_process_options_t* options,
     close_fd = pipes[fd][0];
     use_fd = pipes[fd][1];
 
-    if (use_fd >= 0)
-      close(close_fd);
+    if (use_fd >= 0) {
+      if (close_fd != -1)
+        uv__close(close_fd);
+    }
     else if (fd >= 3)
       continue;
     else {
@@ -306,7 +308,7 @@ static void uv__process_child_init(const uv_process_options_t* options,
       uv__cloexec(use_fd, 0);
     else {
       dup2(use_fd, fd);
-      close(use_fd);
+      uv__close(use_fd);
     }
 
     if (fd <= 2)
@@ -414,8 +416,8 @@ int uv_spawn(uv_loop_t* loop,
 
   if (pid == -1) {
     err = -errno;
-    close(signal_pipe[0]);
-    close(signal_pipe[1]);
+    uv__close(signal_pipe[0]);
+    uv__close(signal_pipe[1]);
     goto error;
   }
 
@@ -424,7 +426,7 @@ int uv_spawn(uv_loop_t* loop,
     abort();
   }
 
-  close(signal_pipe[1]);
+  uv__close(signal_pipe[1]);
 
   process->errorno = 0;
   do
@@ -440,7 +442,7 @@ int uv_spawn(uv_loop_t* loop,
   else
     abort();
 
-  close(signal_pipe[0]);
+  uv__close(signal_pipe[0]);
 
   for (i = 0; i < options->stdio_count; i++) {
     err = uv__process_open_stream(options->stdio + i, pipes[i], i == 0);
@@ -465,8 +467,8 @@ int uv_spawn(uv_loop_t* loop,
 
 error:
   for (i = 0; i < stdio_count; i++) {
-    close(pipes[i][0]);
-    close(pipes[i][1]);
+    uv__close(pipes[i][0]);
+    uv__close(pipes[i][1]);
   }
   free(pipes);
 
diff --git a/src/unix/signal.c b/src/unix/signal.c
index b19d9365..d2cf54a9 100644
--- a/src/unix/signal.c
+++ b/src/unix/signal.c
@@ -240,12 +240,12 @@ void uv__signal_loop_cleanup(uv_loop_t* loop) {
   }
 
   if (loop->signal_pipefd[0] != -1) {
-    close(loop->signal_pipefd[0]);
+    uv__close(loop->signal_pipefd[0]);
     loop->signal_pipefd[0] = -1;
   }
 
   if (loop->signal_pipefd[1] != -1) {
-    close(loop->signal_pipefd[1]);
+    uv__close(loop->signal_pipefd[1]);
     loop->signal_pipefd[1] = -1;
   }
 }
diff --git a/src/unix/stream.c b/src/unix/stream.c
index b7029bc0..0c406209 100644
--- a/src/unix/stream.c
+++ b/src/unix/stream.c
@@ -85,7 +85,7 @@ static int uv__open_cloexec(const char* path, int flags) {
 
   err = uv__cloexec(fd, 1);
   if (err) {
-    close(fd);
+    uv__close(fd);
     return err;
   }
 
@@ -309,7 +309,7 @@ int uv__stream_try_select(uv_stream_t* stream, int* fd) {
   timeout.tv_nsec = 1;
 
   ret = kevent(kq, filter, 1, events, 1, &timeout);
-  SAVE_ERRNO(close(kq));
+  uv__close(kq);
 
   if (ret == -1)
     return -errno;
@@ -357,8 +357,8 @@ int uv__stream_try_select(uv_stream_t* stream, int* fd) {
   return 0;
 
 fatal4:
-  close(s->fake_fd);
-  close(s->int_fd);
+  uv__close(s->fake_fd);
+  uv__close(s->int_fd);
   s->fake_fd = -1;
   s->int_fd = -1;
 fatal3:
@@ -467,13 +467,13 @@ static int uv__emfile_trick(uv_loop_t* loop, int accept_fd) {
   if (loop->emfile_fd == -1)
     return -EMFILE;
 
-  close(loop->emfile_fd);
+  uv__close(loop->emfile_fd);
 
   for (;;) {
     fd = uv__accept(accept_fd);
 
     if (fd != -1) {
-      close(fd);
+      uv__close(fd);
       continue;
     }
 
@@ -572,7 +572,7 @@ int uv_accept(uv_stream_t* server, uv_stream_t* client) {
                             UV_STREAM_READABLE | UV_STREAM_WRITABLE);
       if (err) {
         /* TODO handle error */
-        close(server->accepted_fd);
+        uv__close(server->accepted_fd);
         server->accepted_fd = -1;
         return err;
       }
@@ -581,7 +581,7 @@ int uv_accept(uv_stream_t* server, uv_stream_t* client) {
     case UV_UDP:
       err = uv_udp_open((uv_udp_t*) client, server->accepted_fd);
       if (err) {
-        close(server->accepted_fd);
+        uv__close(server->accepted_fd);
         server->accepted_fd = -1;
         return err;
       }
@@ -1424,8 +1424,8 @@ void uv__stream_close(uv_stream_t* handle) {
     uv_thread_join(&s->thread);
     uv_sem_destroy(&s->close_sem);
     uv_sem_destroy(&s->async_sem);
-    close(s->fake_fd);
-    close(s->int_fd);
+    uv__close(s->fake_fd);
+    uv__close(s->int_fd);
     uv_close((uv_handle_t*) &s->async, uv__stream_osx_cb_close);
 
     handle->select = NULL;
@@ -1436,11 +1436,15 @@ void uv__stream_close(uv_stream_t* handle) {
   uv_read_stop(handle);
   uv__handle_stop(handle);
 
-  close(handle->io_watcher.fd);
-  handle->io_watcher.fd = -1;
+  if (handle->io_watcher.fd != -1) {
+    /* Don't close stdio file descriptors.  Nothing good comes from it. */
+    if (handle->io_watcher.fd > STDERR_FILENO)
+      uv__close(handle->io_watcher.fd);
+    handle->io_watcher.fd = -1;
+  }
 
-  if (handle->accepted_fd >= 0) {
-    close(handle->accepted_fd);
+  if (handle->accepted_fd != -1) {
+    uv__close(handle->accepted_fd);
     handle->accepted_fd = -1;
   }
 
diff --git a/src/unix/sunos.c b/src/unix/sunos.c
index 0306e036..cc578d96 100644
--- a/src/unix/sunos.c
+++ b/src/unix/sunos.c
@@ -75,7 +75,7 @@ int uv__platform_loop_init(uv_loop_t* loop, int default_loop) {
 
   err = uv__cloexec(fd, 1);
   if (err) {
-    close(fd);
+    uv__close(fd);
     return err;
   }
   loop->backend_fd = fd;
@@ -86,12 +86,12 @@ int uv__platform_loop_init(uv_loop_t* loop, int default_loop) {
 
 void uv__platform_loop_delete(uv_loop_t* loop) {
   if (loop->fs_fd != -1) {
-    close(loop->fs_fd);
+    uv__close(loop->fs_fd);
     loop->fs_fd = -1;
   }
 
   if (loop->backend_fd != -1) {
-    close(loop->backend_fd);
+    uv__close(loop->backend_fd);
     loop->backend_fd = -1;
   }
 }
@@ -451,7 +451,7 @@ int uv_resident_set_memory(size_t* rss) {
     *rss = (size_t)psinfo.pr_rssize * 1024;
     err = 0;
   }
-  close(fd);
+  uv__close(fd);
 
   return err;
 }
diff --git a/src/unix/tcp.c b/src/unix/tcp.c
index 8ff26dcb..e5f226c1 100644
--- a/src/unix/tcp.c
+++ b/src/unix/tcp.c
@@ -48,7 +48,7 @@ static int maybe_new_socket(uv_tcp_t* handle, int domain, int flags) {
 
   err = uv__stream_open((uv_stream_t*) handle, sockfd, flags);
   if (err) {
-    close(sockfd);
+    uv__close(sockfd);
     return err;
   }
 
diff --git a/src/unix/udp.c b/src/unix/udp.c
index 47e8b035..a2b3dc32 100644
--- a/src/unix/udp.c
+++ b/src/unix/udp.c
@@ -40,8 +40,11 @@ static int uv__udp_maybe_deferred_bind(uv_udp_t* handle, int domain);
 void uv__udp_close(uv_udp_t* handle) {
   uv__io_close(handle->loop, &handle->io_watcher);
   uv__handle_stop(handle);
-  close(handle->io_watcher.fd);
-  handle->io_watcher.fd = -1;
+
+  if (handle->io_watcher.fd != -1) {
+    uv__close(handle->io_watcher.fd);
+    handle->io_watcher.fd = -1;
+  }
 }
 
 
@@ -337,7 +340,7 @@ int uv__udp_bind(uv_udp_t* handle,
   return 0;
 
 out:
-  close(handle->io_watcher.fd);
+  uv__close(handle->io_watcher.fd);
   handle->io_watcher.fd = -1;
   return err;
 }
diff --git a/test/test-stdio-over-pipes.c b/test/test-stdio-over-pipes.c
index 5f2226cd..15744761 100644
--- a/test/test-stdio-over-pipes.c
+++ b/test/test-stdio-over-pipes.c
@@ -43,7 +43,6 @@ static int output_used;
 
 
 static void close_cb(uv_handle_t* handle) {
-  printf("close_cb\n");
   close_cb_called++;
 }