Skip to content
This repository has been archived by the owner on May 4, 2018. It is now read-only.

Commit

Permalink
unix: sanity-check fds before closing
Browse files Browse the repository at this point in the history
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 nodejs/node-v0.x-archive#6271 for details and a test case.
  • Loading branch information
bnoordhuis committed Oct 1, 2013
1 parent 636f205 commit 359d667
Show file tree
Hide file tree
Showing 16 changed files with 85 additions and 56 deletions.
16 changes: 8 additions & 8 deletions src/unix/aix.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
}

Expand All @@ -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;
}

Expand All @@ -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;
Expand All @@ -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;
}

Expand All @@ -374,7 +374,7 @@ int uv_interface_addresses(uv_interface_address_t** addresses,

#undef ADDR_SIZE

close(sockfd);
uv__close(sockfd);
return 0;
}

Expand Down
4 changes: 2 additions & 2 deletions src/unix/async.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Expand Down
26 changes: 23 additions & 3 deletions src/unix/core.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down Expand Up @@ -397,7 +397,7 @@ int uv__accept(int sockfd) {
err = uv__nonblock(peerfd, 1);

if (err) {
close(peerfd);
uv__close(peerfd);
return err;
}

Expand All @@ -406,6 +406,26 @@ int uv__accept(int sockfd) {
}


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) {
Expand Down Expand Up @@ -514,7 +534,7 @@ int uv__dup(int fd) {

err = uv__cloexec(fd, 1);
if (err) {
close(fd);
uv__close(fd);
return err;
}

Expand Down
1 change: 1 addition & 0 deletions src/unix/internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion src/unix/kqueue.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
4 changes: 2 additions & 2 deletions src/unix/linux-core.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down Expand Up @@ -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';
Expand Down
2 changes: 1 addition & 1 deletion src/unix/linux-inotify.c
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ static int new_inotify_fd(void) {
err = uv__nonblock(fd, 1);

if (err) {
close(fd);
uv__close(fd);
return err;
}

Expand Down
4 changes: 2 additions & 2 deletions src/unix/loop.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
4 changes: 2 additions & 2 deletions src/unix/pipe.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
22 changes: 12 additions & 10 deletions src/unix/process.c
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down Expand Up @@ -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 {
Expand All @@ -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)
Expand Down Expand Up @@ -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;
}

Expand All @@ -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
Expand All @@ -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);
Expand All @@ -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);

Expand Down
4 changes: 2 additions & 2 deletions src/unix/signal.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Expand Down
32 changes: 18 additions & 14 deletions src/unix/stream.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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;
}
Expand All @@ -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;
}
Expand Down Expand Up @@ -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;
Expand All @@ -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;
}

Expand Down
Loading

0 comments on commit 359d667

Please sign in to comment.